-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Expand file tree
/
Copy pathInnerClassCouldBeStatic.ql
More file actions
160 lines (151 loc) · 5.77 KB
/
InnerClassCouldBeStatic.ql
File metadata and controls
160 lines (151 loc) · 5.77 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
/**
* @name Inner class could be static
* @description A non-static nested class keeps a reference to the enclosing object,
* which makes the nested class bigger and may cause a memory leak.
* @kind problem
* @problem.severity recommendation
* @precision high
* @id java/non-static-nested-class
* @tags quality
* maintainability
* readability
*/
import java
/**
* Is the field `f` inherited by the class `c`? This is a slightly imprecise,
* since package-protected fields are not inherited by classes in different
* packages, but it's enough for the purposes of this check.
*/
pragma[nomagic]
predicate inherits(Class c, Field f) {
f = c.getAField()
or
not f.isPrivate() and c.getAStrictAncestor().getAField() = f
}
/**
* An access to a method or field that uses an enclosing instance
* of the type containing it.
*/
class EnclosingInstanceAccess extends Expr {
EnclosingInstanceAccess() { exists(enclosingInstanceAccess(this)) }
RefType getAnAccessedType() { result = enclosingInstanceAccess(this) }
}
RefType enclosingInstanceAccess(Expr expr) {
exists(RefType enclosing | enclosing = expr.getEnclosingCallable().getDeclaringType() |
// A direct qualified `this` access that doesn't refer to the containing
// class must refer to an enclosing instance instead.
result = expr.(ThisAccess).getType() and result != enclosing
or
// A qualified `super` access qualified with a type that isn't the enclosing type.
result = expr.(SuperAccess).getQualifier().(TypeAccess).getType() and result != enclosing
or
// An unqualified `new` expression constructing a
// non-static type that needs an enclosing instance.
exists(ClassInstanceExpr new, InnerClass t |
new = expr and t = new.getType().(RefType).getSourceDeclaration()
|
result = t and
not exists(new.getQualifier()) and
not t.getEnclosingType*() = enclosing
)
or
// An unqualified `new` expression constructing another instance of the
// class it is itself located in, calling a constructor that uses an
// enclosing instance.
exists(ClassInstanceExpr new, Constructor ctor, Expr e2 |
new = expr and
not exists(new.getQualifier()) and
ctor = new.getConstructor() and
enclosing.getEnclosingType*().(InnerClass) = ctor.getDeclaringType() and
ctor = e2.getEnclosingCallable() and
result = enclosingInstanceAccess(e2)
)
or
// An unqualified method or field access to a member that isn't inherited
// must refer to an enclosing instance.
exists(FieldAccess fa | fa = expr |
result = fa.getField().getDeclaringType() and
not exists(fa.getQualifier()) and
not fa.getVariable().(Field).isStatic() and
not inherits(enclosing, fa.getVariable())
)
or
exists(MethodCall ma | ma = expr |
result = ma.getMethod().getDeclaringType() and
not exists(ma.getQualifier()) and
not ma.getMethod().isStatic() and
not enclosing.inherits(ma.getMethod())
)
)
}
/**
* A nested class `c` could be static precisely when
*
* - it only accesses members of enclosing instances in its constructor
* (this includes field initializers);
* - it is not anonymous;
* - it is not a local class;
* - if its supertype or enclosing type is also nested, that type could be made static;
* - any classes nested within `c` only access members of enclosing instances of `c` in their constructors,
* and only extend classes that could be made static.
*
* Note that classes that are already static clearly "could" be static.
*/
predicate potentiallyStatic(InnerClass c) {
not exists(EnclosingInstanceAccess a, Method m |
m = a.getEnclosingCallable() and
m.getDeclaringType() = c
) and
c instanceof MemberType and
forall(
InnerClass other // If nested and non-static, ...
|
// ... all supertypes (which are from source), ...
other = c.getASourceSupertype() and other.fromSource()
or
// ... and the enclosing type, ...
other = c.getEnclosingType()
|
// ... must be (potentially) static.
potentiallyStatic(other)
) and
// No nested classes of `c` access an enclosing instance of `c` except in their constructors, i.e.
// for all accesses to a non-static member of an enclosing instance ...
forall(EnclosingInstanceAccess a, Method m |
// ... that occur in a method of a nested class of `c` ...
m = a.getEnclosingCallable() and m.getDeclaringType().getEnclosingType+() = c
|
// ... the access must be to a member of a type enclosed in `c` or `c` itself.
a.getAnAccessedType().getEnclosingType*() = c
) and
// Any supertype of a class nested in `c` must be potentially static.
forall(InnerClass nested | nested.getEnclosingType+() = c |
forall(InnerClass superOfNested | superOfNested = nested.getASourceSupertype+() |
potentiallyStatic(superOfNested)
)
) and
// JUnit Nested test classes are required to be non-static.
not c.hasAnnotation("org.junit.jupiter.api", "Nested") and
// There's no `static` in kotlin:
not c.getLocation().getFile().isKotlinSourceFile()
}
/**
* A problematic class, meaning a class that could be static but isn't.
*/
class ProblematicClass extends InnerClass {
ProblematicClass() { potentiallyStatic(this) }
/**
* Check for accesses to the enclosing instance in a constructor or field
* initializer.
*/
predicate usesEnclosingInstanceInConstructor() {
exists(EnclosingInstanceAccess a | a.getEnclosingCallable() = this.getAConstructor())
}
}
from ProblematicClass c, string msg
where
c.fromSource() and
if c.usesEnclosingInstanceInConstructor()
then msg = " could be made static, since the enclosing instance is used only in its constructor."
else msg = " should be made static, since the enclosing instance is not used."
select c, c.getName() + msg