Skip to content

Commit 99c049c

Browse files
hvitvedhvitved
authored
Merge pull request #10102 from hvitved/ql/redundant-override-refined
QL: Refine 'redundant override' query
2 parents 93fc952 + c86c9ec commit 99c049c

File tree

4 files changed

+232
-53
lines changed

4 files changed

+232
-53
lines changed

‎ql/ql/src/codeql_ql/style/RedundantOverrideQuery.qll

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import ql
2+
3+
/** Holds if `pred` overrides super predicate `sup` by forwarding via `mc`. */
4+
private predicate forwardingOverride(ClassPredicate pred, MemberCall mc, ClassPredicate sup) {
5+
pred.overrides(sup) and
6+
mc.getBase() instanceof Super and
7+
mc.getTarget() = sup and
8+
not exists(pred.getQLDoc()) and
9+
forall(int i, VarDecl p | p = pred.getParameter(i) | mc.getArgument(i) = p.getAnAccess()) and
10+
(
11+
pred.getBody() =
12+
any(ComparisonFormula comp |
13+
comp.getOperator() = "=" and
14+
comp.getAnOperand() instanceof ResultAccess and
15+
comp.getAnOperand() = mc and
16+
pred.getReturnType() = sup.getReturnType()
17+
)
18+
or
19+
pred.getBody() = mc
20+
)
21+
}
22+
23+
private predicate forwardingOverrideProj(ClassPredicate pred, ClassPredicate sup) {
24+
forwardingOverride(pred, _, sup)
25+
}
26+
27+
private ClassPredicate getUltimateDef(ClassPredicate p) {
28+
forwardingOverrideProj*(p, result) and
29+
not forwardingOverrideProj(result, _)
30+
}
31+
32+
predicate redundantOverride(ClassPredicate pred, ClassPredicate sup) {
33+
exists(MemberCall mc |
34+
forwardingOverride(pred, mc, sup) and
35+
// overridden to provide more precise QL doc
36+
not exists(pred.getQLDoc()) and
37+
// overridden to disambiguate
38+
not exists(ClassPredicate other |
39+
getUltimateDef(sup) != getUltimateDef(other) and
40+
pred.getDeclaringType().getASuperType+() = other.getDeclaringType() and
41+
not sup.overrides*(other) and
42+
other.getName() = pred.getName() and
43+
other.getArity() = pred.getArity()
44+
)
45+
)
46+
}

‎ql/ql/src/queries/style/RedundantOverride.ql

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,27 +9,7 @@
99
*/
1010

1111
import ql
12-
13-
private predicate redundantOverride(ClassPredicate pred, ClassPredicate sup) {
14-
pred.overrides(sup) and
15-
// Can be made more precise, but rules out overrides needed for disambiguation
16-
count(pred.getDeclaringType().getASuperType()) <= 1 and
17-
exists(MemberCall mc |
18-
mc.getBase() instanceof Super and
19-
mc.getTarget() = sup and
20-
not exists(pred.getQLDoc())
21-
|
22-
pred.getBody() =
23-
any(ComparisonFormula comp |
24-
comp.getOperator() = "=" and
25-
comp.getAnOperand() instanceof ResultAccess and
26-
comp.getAnOperand() = mc and
27-
pred.getReturnType() = sup.getReturnType()
28-
)
29-
or
30-
pred.getBody() = mc
31-
)
32-
}
12+
import codeql_ql.style.RedundantOverrideQuery
3313

3414
from ClassPredicate pred, ClassPredicate sup
3515
where redundantOverride(pred, sup)

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
| RedundantOverride.qll:12:16:12:19 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:4:7:4:10 | ClassPredicate pred | this |
2-
| RedundantOverride.qll:16:16:16:19 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:4:7:4:10 | ClassPredicate pred | this |
3-
| RedundantOverride.qll:47:22:47:26 | ClassPredicate pred3 | Redundant override of $@ predicate | RedundantOverride.qll:8:13:8:17 | ClassPredicate pred3 | this |
4-
| RedundantOverride.qll:51:16:51:19 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:4:7:4:10 | ClassPredicate pred | this |
1+
| RedundantOverride.qll:9:18:9:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:5:9:5:12 | ClassPredicate pred | this |
2+
| RedundantOverride.qll:21:18:21:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:17:9:17:12 | ClassPredicate pred | this |
3+
| RedundantOverride.qll:110:24:110:27 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:106:15:106:18 | ClassPredicate pred | this |
4+
| RedundantOverride.qll:124:18:124:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:118:9:118:12 | ClassPredicate pred | this |
5+
| RedundantOverride.qll:128:18:128:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:118:9:118:12 | ClassPredicate pred | this |
6+
| RedundantOverride.qll:132:18:132:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:128:18:128:21 | ClassPredicate pred | this |
7+
| RedundantOverride.qll:150:18:150:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:140:9:140:12 | ClassPredicate pred | this |
8+
| RedundantOverride.qll:164:18:164:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:158:9:158:12 | ClassPredicate pred | this |
9+
| RedundantOverride.qll:168:18:168:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:158:9:158:12 | ClassPredicate pred | this |
10+
| RedundantOverride.qll:172:18:172:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:168:18:168:21 | ClassPredicate pred | this |
11+
| RedundantOverride.qll:176:18:176:21 | ClassPredicate pred | Redundant override of $@ predicate | RedundantOverride.qll:168:18:168:21 | ClassPredicate pred | this |

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

Lines changed: 174 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,198 @@
1-
class Foo extends string {
2-
Foo() { this = "Foo" }
1+
module Test1 {
2+
class Foo extends int {
3+
Foo() { this = 1 }
34

4-
Foo pred() { none() }
5+
Foo pred() { result = this }
6+
}
57

6-
Foo pred2() { none() }
7-
8-
predicate pred3() { none() }
8+
class Bar extends Foo {
9+
override Foo pred() { result = Foo.super.pred() } // BAD
10+
}
911
}
1012

11-
class Bar1 extends Foo {
12-
override Foo pred() { result = Foo.super.pred() } // BAD
13+
module Test2 {
14+
class Foo extends int {
15+
Foo() { this = 1 }
16+
17+
Foo pred() { result = this }
18+
}
19+
20+
class Bar extends Foo {
21+
override Foo pred() { result = super.pred() } // BAD
22+
}
1323
}
1424

15-
class Bar2 extends Foo {
16-
override Foo pred() { result = super.pred() } // BAD
25+
module Test3 {
26+
class Foo extends int {
27+
Foo() { this = [1 .. 3] }
28+
29+
Foo pred() { any() }
30+
}
31+
32+
class Bar extends Foo {
33+
Bar() { this = 2 }
34+
}
35+
36+
class Baz extends Foo {
37+
Baz() { this = 3 }
38+
39+
override Bar pred() { result = super.pred() } // GOOD (refined return type)
40+
}
1741
}
1842

19-
class Bar3 extends Foo {
20-
override Bar3 pred() { result = super.pred() } // GOOD (refined return type)
43+
module Test4 {
44+
class Foo extends int {
45+
Foo() { this = [1, 2] }
46+
47+
Foo pred() { result = 2 }
48+
}
49+
50+
class Bar extends Foo {
51+
Bar() { this = 1 }
52+
53+
override Foo pred() { result = this } // GOOD
54+
}
2155
}
2256

23-
class Bar4 extends Foo {
24-
override Foo pred() { any() } // GOOD
57+
module Test5 {
58+
class Foo extends int {
59+
Foo() { this = 1 }
60+
61+
Foo pred() { result = this }
62+
}
63+
64+
class Bar extends Foo {
65+
/** My own overriding QL doc. */
66+
override Foo pred() { result = super.pred() } // GOOD
67+
}
2568
}
2669

27-
class Bar5 extends Foo {
28-
/** My own overriding QL doc. */
29-
override Foo pred() { result = super.pred() } // GOOD
70+
module Test6 {
71+
class Foo extends int {
72+
Foo() { this = [1, 2] }
73+
74+
Foo pred() { result = 1 }
75+
76+
Foo pred2() { result = 2 }
77+
}
78+
79+
class Bar extends Foo {
80+
override Foo pred() { result = super.pred2() } // GOOD
81+
}
3082
}
3183

32-
class Bar6 extends Foo {
33-
override Foo pred() { result = super.pred2() } // GOOD
84+
module Test7 {
85+
class Foo extends int {
86+
Foo() { this = [1, 2] }
87+
88+
Foo pred() { result = 2 }
89+
}
90+
91+
class Bar extends int {
92+
Bar() { this = 1 }
93+
94+
Foo pred() { result = this }
95+
}
96+
97+
class Baz extends Foo, Bar {
98+
override Foo pred() { result = Foo.super.pred() } // GOOD (disambiguate)
99+
}
34100
}
35101

36-
class Bar7 extends string {
37-
Bar7() { this = "Bar7" }
102+
module Test8 {
103+
class Foo extends int {
104+
Foo() { this = 1 }
38105

39-
Foo pred() { any() }
106+
predicate pred(Foo f) { f = this }
107+
}
108+
109+
class Bar extends Foo {
110+
override predicate pred(Foo f) { super.pred(f) } // BAD
111+
}
40112
}
41113

42-
class Bar8 extends Foo, Bar7 {
43-
override Foo pred() { result = Foo.super.pred() } // GOOD
114+
module Test9 {
115+
class Foo extends int {
116+
Foo() { this = [1, 2] }
117+
118+
Foo pred() { result = this }
119+
}
120+
121+
class Bar extends Foo {
122+
Bar() { this = 1 }
123+
124+
override Foo pred() { Foo.super.pred() = result } // BAD
125+
}
126+
127+
class Baz1 extends Foo, Bar {
128+
override Foo pred() { Foo.super.pred() = result } // BAD
129+
}
130+
131+
class Baz2 extends Foo, Baz1 {
132+
override Foo pred() { Baz1.super.pred() = result } // BAD
133+
}
44134
}
45135

46-
class Bar9 extends Foo {
47-
override predicate pred3() { super.pred3() } // BAD
136+
module Test10 {
137+
class Foo extends int {
138+
Foo() { this = [1, 2] }
139+
140+
Foo pred() { result = 1 }
141+
}
142+
143+
class Bar extends int {
144+
Bar() { this = 1 }
145+
146+
Foo pred(int i) { none() }
147+
}
148+
149+
class Baz1 extends Foo, Bar {
150+
override Foo pred() { result = Foo.super.pred() } // BAD
151+
}
48152
}
49153

50-
class Bar10 extends Foo {
51-
override Foo pred() { Foo.super.pred() = result } // BAD
154+
module Test11 {
155+
class Foo extends int {
156+
Foo() { this = [1 .. 4] }
157+
158+
Foo pred() { result = 1 }
159+
}
160+
161+
class Bar1 extends Foo {
162+
Bar1() { this = [1 .. 3] }
163+
164+
override Foo pred() { Foo.super.pred() = result } // BAD
165+
}
166+
167+
class Bar2 extends Foo, Bar1 {
168+
override Foo pred() { Foo.super.pred() = result } // BAD
169+
}
170+
171+
class Bar3 extends Foo, Bar2 {
172+
override Foo pred() { Bar2.super.pred() = result } // BAD
173+
}
174+
175+
class Bar4 extends Bar2, Bar3 {
176+
override Foo pred() { result = Bar2.super.pred() } // BAD
177+
}
178+
179+
class Bar5 extends Foo {
180+
Bar5() { this = [1 .. 2] }
181+
182+
override Foo pred() { result = this } // GOOD
183+
}
184+
185+
class Bar6 extends Bar4, Bar5 {
186+
override Foo pred() { result = Bar4.super.pred() } // GOOD (dismambiguate)
187+
}
188+
189+
class Bar7 extends Bar6 {
190+
Bar7() { this = 1 }
191+
192+
override Foo pred() { result = 2 } // GOOD
193+
}
194+
195+
class Bar8 extends Bar6, Bar7 {
196+
override Foo pred() { result = Bar6.super.pred() } // GOOD (specialize)
197+
}
52198
}

0 commit comments

Comments
 (0)