diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql new file mode 100644 index 000000000000..a79e4f69569e --- /dev/null +++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql @@ -0,0 +1,37 @@ +/** + * @name Predicates starting with "get" or "as" should return a value + * @description Checks if predicates that start with "get" or "as" actually return a value. + * @kind problem + * @problem.severity warning + * @id ql/predicates-get-should-return-value + * @tags correctness + * maintainability + * @precision high + */ + +import ql +import codeql_ql.ast.Ast + +/** + * Identifies predicates whose names start with "get", "as" followed by an uppercase letter. + * This ensures that only predicates like "getValue" are matched, excluding names like "getter". + */ +predicate isGetPredicate(Predicate pred, string prefix) { + prefix = pred.getName().regexpCapture("(get|as)[A-Z].*", 1) +} + +/** + * Checks if a predicate has a return type. This is phrased negatively to not flag unresolved aliases. + */ +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 + 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 new file mode 100644 index 000000000000..4b36d2c5c328 --- /dev/null +++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected @@ -0,0 +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/ValidatePredicateGetReturns.qlref b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref new file mode 100644 index 000000000000..e116f69d6b22 --- /dev/null +++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref @@ -0,0 +1 @@ +queries/style/ValidatePredicateGetReturns.ql diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll new file mode 100644 index 000000000000..2cc4dec64d20 --- /dev/null +++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll @@ -0,0 +1,67 @@ +import ql + +// NOT OK -- Predicate starts with "get" but does not return a value +predicate getValue() { none() } + +// OK -- starts with get and returns a value +string getData() { result = "data" } + +// OK -- starts with get but followed by a lowercase letter, probably should be ignored +predicate getterFunction() { none() } + +// OK -- starts with get and returns a value +string getImplementation() { result = "implementation" } + +// OK -- is an alias +predicate getAlias = getImplementation/0; + +// OK -- Starts with "get" but followed by a lowercase letter, probably be ignored +predicate getvalue() { none() } + +// OK -- Does not start with "get", should be ignored +predicate retrieveValue() { none() } + +// NOT OK -- starts with get and does not return value +predicate getImplementation2() { none() } + +// 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 +predicate asValue() { none() } + +// OK -- starts with as but followed by a lowercase letter, probably should be ignored +predicate assessment() { none() } + +// OK -- starts with as and returns a value +string asString() { result = "string" } + +// OK -- starts with get and returns a value +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;