Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add query to ensure predicates starting with 'get' return a value #18164

Merged
37 changes: 37 additions & 0 deletions ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
Original file line number Diff line number Diff line change
@@ -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."
Original file line number Diff line number Diff line change
@@ -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. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
queries/style/ValidatePredicateGetReturns.ql
67 changes: 67 additions & 0 deletions ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
Original file line number Diff line number Diff line change
@@ -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;
Loading