Skip to content

Commit ed87d77

Browse files
QL: Merge pull request #70 from github/dont-mention-this
Query to find member predicates that don't depend on `this`
2 parents dd6abdc + c3493d6 commit ed87d77

1 file changed

Lines changed: 86 additions & 0 deletions

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* @name Class predicate doesn't mention `this`
3+
* @description A class predicate that doesn't use `this` (or a field) could instead be a classless predicate, and may cause a cartesian product.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @id ql/class-predicate-doesnt-use-this
7+
* @tags performance
8+
* @precision medium
9+
*/
10+
11+
import ql
12+
13+
predicate usesThis(ClassPredicate pred) {
14+
exists(ThisAccess th | th.getEnclosingPredicate() = pred)
15+
or
16+
exists(Super sup | sup.getEnclosingPredicate() = pred)
17+
or
18+
exists(FieldAccess f | f.getEnclosingPredicate() = pred)
19+
or
20+
// implicit this
21+
exists(PredicateCall pc | pc.getEnclosingPredicate() = pred |
22+
pc.getTarget().getDeclaration() instanceof ClassPredicate
23+
)
24+
}
25+
26+
predicate isLiteralComparison(ComparisonFormula eq) {
27+
exists(Expr lhs, Expr rhs |
28+
eq.getSymbol() = "=" and
29+
eq.getAnOperand() = lhs and
30+
eq.getAnOperand() = rhs and
31+
(
32+
lhs instanceof ResultAccess
33+
or
34+
lhs instanceof ThisAccess
35+
or
36+
lhs instanceof VarAccess
37+
) and
38+
(
39+
rhs instanceof Literal
40+
or
41+
exists(NewTypeBranch nt |
42+
rhs.(Call).getTarget().getDeclaration() = nt and
43+
count(nt.getField(_)) = 0
44+
)
45+
)
46+
)
47+
}
48+
49+
predicate conjParent(Formula par, Formula child) { child = par.(Conjunction).getAnOperand() }
50+
51+
predicate isLiteralComparisons(Formula f) {
52+
forex(ComparisonFormula child | conjParent*(f, child) | isLiteralComparison(child))
53+
}
54+
55+
predicate isTrivialImplementation(Predicate pred) {
56+
not exists(pred.getBody())
57+
or
58+
exists(Formula bod | bod = pred.getBody() |
59+
bod instanceof AnyCall
60+
or
61+
bod instanceof NoneCall
62+
or
63+
isLiteralComparisons(bod)
64+
)
65+
}
66+
67+
predicate isSingleton(Type ty) {
68+
isTrivialImplementation(ty.(ClassType).getDeclaration().getCharPred())
69+
or
70+
isSingleton(ty.getASuperType())
71+
or
72+
exists(NewTypeBranch br | count(br.getField(_)) = 0 |
73+
ty.(NewTypeBranchType).getDeclaration() = br
74+
or
75+
br = unique(NewTypeBranch br2 | br2 = ty.(NewTypeType).getDeclaration().getABranch())
76+
)
77+
}
78+
79+
from ClassPredicate pred
80+
where
81+
not usesThis(pred) and
82+
not isTrivialImplementation(pred) and
83+
not isSingleton(pred.getDeclaringType()) and
84+
not exists(ClassPredicate other | pred.overrides(other) or other.overrides(pred)) and
85+
not pred.isOverride()
86+
select pred, "This predicate could be a classless predicate, as it doesn't depend on `this`."

0 commit comments

Comments
 (0)