Skip to content

Commit 8e6bc11

Browse files
MathiasVPMathiasVP
authored
QL: Merge pull request #86 from github/use-set-literal
New query: Use set literal
2 parents f29457f + e510593 commit 8e6bc11

4 files changed

Lines changed: 273 additions & 0 deletions

File tree

‎ql/src/queries/style/UseSetLiteral.ql‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* @name Use a set literal in place of `or`
3+
* @description A chain of `or`s can be replaced with a set literal, improving readability.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id ql/use-set-literal
7+
* @tags maintainability
8+
* @precision high
9+
*/
10+
11+
import ql
12+
13+
/**
14+
* A chain of disjunctions treated as one object. For example the following is
15+
* a chain of disjunctions with three operands:
16+
* ```
17+
* a or b or c
18+
* ```
19+
*/
20+
class DisjunctionChain extends Disjunction {
21+
DisjunctionChain() { not exists(Disjunction parent | parent.getAnOperand() = this) }
22+
23+
/**
24+
* Gets any operand of the chain.
25+
*/
26+
Formula getAnOperandRec() {
27+
result = getAnOperand*() and
28+
not result instanceof Disjunction
29+
}
30+
}
31+
32+
/**
33+
* An equality comparison with a `Literal`. For example:
34+
* ```
35+
* x = 4
36+
* ```
37+
*/
38+
class EqualsLiteral extends ComparisonFormula {
39+
EqualsLiteral() {
40+
getSymbol() = "=" and
41+
getAnOperand() instanceof Literal
42+
}
43+
}
44+
45+
/**
46+
* A chain of disjunctions where each operand is an equality comparison between
47+
* the same thing and various `Literal`s. For example:
48+
* ```
49+
* x = 4 or
50+
* x = 5 or
51+
* x = 6
52+
* ```
53+
*/
54+
class DisjunctionEqualsLiteral extends DisjunctionChain {
55+
DisjunctionEqualsLiteral() {
56+
// VarAccess on the same variable
57+
exists(VarDef v |
58+
forex(Formula f | f = getAnOperandRec() |
59+
f.(EqualsLiteral).getAnOperand().(VarAccess).getDeclaration() = v
60+
)
61+
)
62+
or
63+
// FieldAccess on the same variable
64+
exists(VarDecl v |
65+
forex(Formula f | f = getAnOperandRec() |
66+
f.(EqualsLiteral).getAnOperand().(FieldAccess).getDeclaration() = v
67+
)
68+
)
69+
or
70+
// ThisAccess
71+
forex(Formula f | f = getAnOperandRec() |
72+
f.(EqualsLiteral).getAnOperand() instanceof ThisAccess
73+
)
74+
or
75+
// ResultAccess
76+
forex(Formula f | f = getAnOperandRec() |
77+
f.(EqualsLiteral).getAnOperand() instanceof ResultAccess
78+
)
79+
// (in principle something like GlobalValueNumbering could be used to generalize this)
80+
}
81+
}
82+
83+
/**
84+
* A call with a single `Literal` argument. For example:
85+
* ```
86+
* myPredicate(4)
87+
* ```
88+
*/
89+
class CallLiteral extends Call {
90+
CallLiteral() {
91+
getNumberOfArguments() = 1 and
92+
getArgument(0) instanceof Literal
93+
}
94+
}
95+
96+
/**
97+
* A chain of disjunctions where each operand is a call to the same predicate
98+
* using various `Literal`s. For example:
99+
* ```
100+
* myPredicate(4) or
101+
* myPredicate(5) or
102+
* myPredicate(6)
103+
* ```
104+
*/
105+
class DisjunctionPredicateLiteral extends DisjunctionChain {
106+
DisjunctionPredicateLiteral() {
107+
// Call to the same target
108+
exists(PredicateOrBuiltin target |
109+
forex(Formula f | f = getAnOperandRec() | f.(CallLiteral).getTarget() = target)
110+
)
111+
}
112+
}
113+
114+
from DisjunctionChain d, string msg, int c
115+
where
116+
(
117+
d instanceof DisjunctionEqualsLiteral and
118+
msg =
119+
"This formula of " + c.toString() +
120+
" comparisons can be replaced with a single equality on a set literal, improving readability."
121+
or
122+
d instanceof DisjunctionPredicateLiteral and
123+
msg =
124+
"This formula of " + c.toString() +
125+
" predicate calls can be replaced with a single call on a set literal, improving readability."
126+
) and
127+
c = count(d.getAnOperandRec()) and
128+
c >= 4
129+
select d, msg

‎ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
| test.qll:4:3:7:7 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
2+
| test.qll:30:3:33:10 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. |
3+
| test.qll:44:3:47:12 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
4+
| test.qll:62:7:65:14 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
5+
| test.qll:68:7:71:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
6+
| test.qll:74:7:77:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
7+
| test.qll:87:3:90:9 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. |
8+
| test.qll:128:3:134:3 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |

‎ql/test/queries/style/UseSetLiteral/UseSetLiteral.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/style/UseSetLiteral.ql

‎ql/test/queries/style/UseSetLiteral/test.qll‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import ql
2+
3+
predicate test1(int a) {
4+
a = 1 or // BAD
5+
a = 2 or
6+
a = 3 or
7+
a = 4
8+
}
9+
10+
predicate test2(int a) {
11+
a = [1, 2, 3, 4] // GOOD
12+
}
13+
14+
predicate test3(int a) {
15+
a = 1 and // GOOD (for the purposes of this query)
16+
a = 2 and
17+
a = 3 and
18+
a = 4
19+
}
20+
21+
bindingset[a]
22+
predicate test4(int a) {
23+
a < 1 or // GOOD (for the purposes of this query)
24+
a = 2 or
25+
a >= 3 or
26+
a > 4
27+
}
28+
29+
predicate test5() {
30+
test1(1) or // BAD
31+
test1(2) or
32+
test1(3) or
33+
test1(4)
34+
}
35+
36+
predicate test6() {
37+
test1(1) or // GOOD
38+
test2(2) or
39+
test3(3) or
40+
test4(4)
41+
}
42+
43+
int test7() {
44+
1 = result or // BAD
45+
2 = result or
46+
3 = result or
47+
4 = result
48+
}
49+
50+
predicate test8() {
51+
test7() = 1 or // BAD [NOT DETECTED]
52+
test7() = 2 or
53+
test7() = 3 or
54+
test7() = 4
55+
}
56+
57+
class MyTest8Class extends int {
58+
string s;
59+
60+
MyTest8Class() {
61+
(
62+
this = 1 or // BAD
63+
this = 2 or
64+
this = 3 or
65+
this = 4
66+
) and
67+
(
68+
s = "1" or // BAD
69+
s = "2" or
70+
s = "3" or
71+
s = "4"
72+
) and
73+
exists(float f |
74+
f = 1.0 or // BAD
75+
f = 1.5 or
76+
f = 2.0 or
77+
f = 2.5
78+
)
79+
}
80+
81+
predicate is(int x) { x = this }
82+
83+
int get() { result = this }
84+
}
85+
86+
predicate test9(MyTest8Class c) {
87+
c.is(1) or // BAD
88+
c.is(2) or
89+
c.is(3) or
90+
c.is(4)
91+
}
92+
93+
predicate test10(MyTest8Class c) {
94+
c.get() = 1 or // BAD [NOT DETECTED]
95+
c.get() = 2 or
96+
c.get() = 3 or
97+
c.get() = 4
98+
}
99+
100+
bindingset[a, b, c, d]
101+
predicate test11(int a, int b, int c, int d) {
102+
a = 1 or // GOOD
103+
b = 2 or
104+
c = 3 or
105+
d = 4
106+
}
107+
108+
bindingset[a, b]
109+
predicate test12(int a, int b) {
110+
a = 1 or // BAD [NOT DETECTED]
111+
a = 2 or
112+
a = 3 or
113+
a = 4 or
114+
b = 0
115+
}
116+
117+
predicate test13(int a, int b) {
118+
a = 1 and b = 1 // GOOD
119+
or
120+
a = 2 and b = 4
121+
or
122+
a = 3 and b = 9
123+
or
124+
a = 4 and b = 16
125+
}
126+
127+
predicate test14(int a) {
128+
a = 1 // BAD
129+
or
130+
(
131+
(a = 2 or a = 3)
132+
or
133+
a = 4
134+
)
135+
}

0 commit comments

Comments
 (0)