From 7db9b7d758859042797625363fc6c0a2fef9b2c1 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Mon, 2 Dec 2024 12:43:52 +0100
Subject: [PATCH] Now flag aliases with the 'get' or 'as' prefix that resolve
 to predicates lacking a return type.

Co-authored-by: asgerf <asgerf@github.com>
---
 .../style/ValidatePredicateGetReturns.ql      | 17 ++++++------
 .../ValidatePredicateGetReturns.expected      |  3 +++
 .../ValidatePredicateGetReturns/test.qll      | 27 ++++++++++++++++++-
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index 4d0196509c33..a79e4f69569e 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -21,18 +21,17 @@ predicate isGetPredicate(Predicate pred, string prefix) {
 }
 
 /**
- * Checks if a predicate has a return type.
+ * Checks if a predicate has a return type. This is phrased negatively to not flag unresolved aliases.
  */
-predicate hasReturnType(Predicate pred) { exists(pred.getReturnTypeExpr()) }
-
-/**
- * Checks if a predicate is an alias using getAlias().
- */
-predicate isAlias(Predicate pred) { exists(pred.(ClasslessPredicate).getAlias()) }
+predicate hasNoReturnType(Predicate pred) {
+  not exists(pred.getReturnTypeExpr()) and
+  not pred.(ClasslessPredicate).getAlias() instanceof PredicateExpr
+  or
+  hasNoReturnType(pred.(ClasslessPredicate).getAlias().(PredicateExpr).getResolvedPredicate())
+}
 
 from Predicate pred, string prefix
 where
   isGetPredicate(pred, prefix) and
-  not hasReturnType(pred) and
-  not isAlias(pred)
+  hasNoReturnType(pred)
 select pred, "This predicate starts with '" + prefix + "' but does not return a value."
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
index 57469246ae82..4b36d2c5c328 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
@@ -1,3 +1,6 @@
 | test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
 | test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
+| test.qll:28:11:28:19 | ClasslessPredicate getAlias2 | This predicate starts with 'get' but does not return a value. |
 | test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'as' but does not return a value. |
+| test.qll:48:11:48:19 | ClasslessPredicate getAlias4 | This predicate starts with 'get' but does not return a value. |
+| test.qll:61:11:61:22 | ClasslessPredicate getDistance2 | This predicate starts with 'get' but does not return a value. |
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
index b68aab08fd78..2cc4dec64d20 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
@@ -24,7 +24,7 @@ predicate retrieveValue() { none() }
 // NOT OK -- starts with get and does not return value
 predicate getImplementation2() { none() }
 
-// OK -- is an alias
+// NOT OK -- is an alias for a predicate which does not have a return value
 predicate getAlias2 = getImplementation2/0;
 
 // NOT OK -- starts with as and does not return value
@@ -40,3 +40,28 @@ string asString() { result = "string" }
 HiddenType getInjectableCompositeActionNode() {
   exists(HiddenType hidden | result = hidden.toString())
 }
+
+// OK
+predicate implementation4() { none() }
+
+// NOT OK -- is an alias
+predicate getAlias4 = implementation4/0;
+
+// OK -- is an alias
+predicate alias5 = implementation4/0;
+
+int root() { none() }
+
+predicate edge(int x, int y) { none() }
+
+// OK -- Higher-order predicate
+int getDistance(int x) = shortestDistances(root/0, edge/2)(_, x, result)
+
+// NOT OK -- Higher-order predicate that does not return a value even though has 'get' in the name
+predicate getDistance2(int x, int y) = shortestDistances(root/0, edge/2)(_, x, y)
+
+// OK
+predicate unresolvedAlias = unresolved/0;
+
+// OK -- unresolved alias
+predicate getUnresolvedAlias = unresolved/0;