Skip to content

Commit f79c5ca

Browse files
committed
Add super-field-always-reassigned.ql
1 parent 2eb2dd3 commit f79c5ca

File tree

1 file changed

+82
-0
lines changed

1 file changed

+82
-0
lines changed
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* Finds fields which are overwritten in the constructors of all subclasses.
3+
* It might be less error-prone to instead add an additional parameter to the superclass
4+
* constructor and initialize the field there (and optionally reduce the visibility of
5+
* the field). This way it can be ensured that no subclass forgets to initialize the field.
6+
*
7+
* For example:
8+
* ```java
9+
* class Superclass {
10+
* protected int f;
11+
*
12+
* protected Superclass() {
13+
* }
14+
* }
15+
*
16+
* class Subclass {
17+
* public Subclass() {
18+
* f = 1;
19+
* }
20+
* }
21+
* ```
22+
*
23+
* Should be changed to:
24+
* ```java
25+
* class Superclass {
26+
* private final int f;
27+
*
28+
* protected Superclass(int f) {
29+
* this.f = f;
30+
* }
31+
* }
32+
*
33+
* class Subclass {
34+
* public Subclass() {
35+
* super(1);
36+
* }
37+
* }
38+
* ```
39+
*
40+
* This query might yield incorrect results in case external code can create instances
41+
* of the superclass, or create custom subclasses, and is not required to overwrite the
42+
* field value.
43+
*
44+
* @kind problem
45+
* @id TODO
46+
*/
47+
48+
import java
49+
50+
from Field f, Class declaringType
51+
where
52+
not f.isStatic() and
53+
f.getDeclaringType() = declaringType and
54+
// Class must be part of the project, since this query suggests to modify it
55+
declaringType.fromSource() and
56+
(
57+
declaringType.isAbstract()
58+
or
59+
// Class is not directly instantiated
60+
not exists(ClassInstanceExpr newExpr |
61+
newExpr.getConstructedType().getSourceDeclaration() = declaringType
62+
)
63+
) and
64+
// Make sure there are multiple subclasses, so that the query is actually relevant
65+
count(Class subtype |
66+
subtype.fromSource() and subtype.getASupertype().getSourceDeclaration() = declaringType
67+
) >= 2 and
68+
// Use `forex` here to make sure there are actually subclasses, with constructors
69+
forex(Class subtype |
70+
subtype.fromSource() and subtype.getASupertype().getSourceDeclaration() = declaringType
71+
|
72+
forex(Constructor c | c = subtype.getAConstructor() |
73+
exists(AssignExpr fieldWrite, FieldAccess fieldAccess |
74+
fieldWrite.getEnclosingCallable() = c and
75+
fieldWrite.getDest() = fieldAccess and
76+
fieldAccess.getField() = f and
77+
fieldAccess.isOwnFieldAccess()
78+
)
79+
)
80+
)
81+
select f,
82+
"All subclasses reassign this field in their constructor; consider initializing this field in the constructor of this class"

0 commit comments

Comments
 (0)