Skip to content

Commit 7267722

Browse files
authored
Merge pull request #10191 from smowton/smowton/admin/java-implicit-this-type-tests
Java: Add test regarding the type of an implicit `this` expression
2 parents d8b000f + 88644b6 commit 7267722

File tree

19 files changed

+262
-9
lines changed

19 files changed

+262
-9
lines changed

java/ql/src/Advisory/Deprecated Code/AvoidDeprecatedCallableAccess.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ private predicate isDeprecatedCallable(Callable c) {
2020

2121
from Call ca, Callable c
2222
where
23-
ca.getCallee() = c and
23+
ca.getCallee().getSourceDeclaration() = c and
2424
isDeprecatedCallable(c) and
2525
// Exclude deprecated calls from within deprecated code.
2626
not isDeprecatedCallable(ca.getCaller())

java/ql/src/Likely Bugs/Collections/IteratorRemoveMayFail.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ predicate containsSpecialCollection(Expr e, SpecialCollectionCreation origin) {
3636
or
3737
exists(Call c, int i |
3838
containsSpecialCollection(c.getArgument(i), origin) and
39-
e = c.getCallee().getParameter(i).getAnAccess()
39+
e = c.getCallee().getSourceDeclaration().getParameter(i).getAnAccess()
4040
)
4141
or
4242
exists(Call c, ReturnStmt r | e = c |
43-
r.getEnclosingCallable() = c.getCallee() and
43+
r.getEnclosingCallable() = c.getCallee().getSourceDeclaration() and
4444
containsSpecialCollection(r.getResult(), origin)
4545
)
4646
}
@@ -58,11 +58,11 @@ predicate iterOfSpecialCollection(Expr e, SpecialCollectionCreation origin) {
5858
or
5959
exists(Call c, int i |
6060
iterOfSpecialCollection(c.getArgument(i), origin) and
61-
e = c.getCallee().getParameter(i).getAnAccess()
61+
e = c.getCallee().getSourceDeclaration().getParameter(i).getAnAccess()
6262
)
6363
or
6464
exists(Call c, ReturnStmt r | e = c |
65-
r.getEnclosingCallable() = c.getCallee() and
65+
r.getEnclosingCallable() = c.getCallee().getSourceDeclaration() and
6666
iterOfSpecialCollection(r.getResult(), origin)
6767
)
6868
}

java/ql/src/Performance/InnerClassCouldBeStatic.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ RefType enclosingInstanceAccess(Expr expr) {
7878
result = ma.getMethod().getDeclaringType() and
7979
not exists(ma.getQualifier()) and
8080
not ma.getMethod().isStatic() and
81-
not exists(Method m | m.getSourceDeclaration() = ma.getMethod() | enclosing.inherits(m))
81+
not enclosing.inherits(ma.getMethod())
8282
)
8383
)
8484
}

java/ql/src/Violations of Best Practice/Implementation Hiding/ExposeRepresentation.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ predicate mayWriteToArray(Expr modified) {
7272
// return __array__; ... method()[1] = 0
7373
exists(ReturnStmt rs | modified = rs.getResult() and relevantType(modified.getType()) |
7474
exists(Callable enclosing, MethodAccess ma |
75-
enclosing = rs.getEnclosingCallable() and ma.getMethod() = enclosing
75+
enclosing = rs.getEnclosingCallable() and ma.getMethod().getSourceDeclaration() = enclosing
7676
|
7777
mayWriteToArray(ma)
7878
)
@@ -100,7 +100,7 @@ VarAccess varPassedInto(Callable c, int i) {
100100
predicate exposesByReturn(Callable c, Field f, Expr why, string whyText) {
101101
returnsArray(c, f) and
102102
exists(MethodAccess ma |
103-
ma.getMethod() = c and ma.getCompilationUnit() != c.getCompilationUnit()
103+
ma.getMethod().getSourceDeclaration() = c and ma.getCompilationUnit() != c.getCompilationUnit()
104104
|
105105
mayWriteToArray(ma) and
106106
why = ma and

java/ql/src/Violations of Best Practice/Naming Conventions/AmbiguousOuterSuper.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ predicate callToInheritedMethod(RefType lexicalScope, MethodAccess ma, string si
2727
not ma.getMethod().isStatic() and
2828
not ma.hasQualifier() and
2929
ma.getEnclosingCallable().getDeclaringType() = lexicalScope and
30-
nestedSupertypePlus(lexicalScope).getAMethod() = ma.getMethod() and
30+
nestedSupertypePlus(lexicalScope).getAMethod() = ma.getMethod().getSourceDeclaration() and
3131
signature = ma.getMethod().getSignature()
3232
}
3333

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The Java extractor now populates the `Method` relating to a `MethodAccess` consistently for calls using an explicit and implicit `this` qualifier. Previously if the method `foo` was inherited from a specialised generic type `ParentType<String>`, then an explicit call `this.foo()` would yield a `MethodAccess` whose `getMethod()` accessor returned the bound method `ParentType<String>.foo`, whereas an implicitly-qualified `foo()` `MethodAccess`'s `getMethod()` would return the unbound method `ParentType.foo`. Now both scenarios produce a bound method. This means that all data-flow queries may return more results where a relevant path transits a call to such an implicitly-qualified call to a member method with a bound generic type, while queries that inspect the result of `MethodAccess.getMethod()` may need to tolerate bound generic methods in more circumstances. The queries `java/iterator-remove-failure`, `java/non-static-nested-class`, `java/internal-representation-exposure`, `java/subtle-inherited-call` and `java/deprecated-call` have been amended to properly handle calls to bound generic methods, and in some instances may now produce more results in the explicit-`this` case as well.
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
class Gen<T> {
2+
void m(T t) { }
3+
}
4+
5+
class SubSpec extends Gen<String> {
6+
void foo() {
7+
m("direct implicit this");
8+
this.m("direct explicit this");
9+
}
10+
11+
class Inner {
12+
void bar() {
13+
m("direct implicit this from inner");
14+
SubSpec.this.m("direct explicit this from inner");
15+
}
16+
}
17+
18+
void hasLocal() {
19+
class Local {
20+
void baz() {
21+
m("direct implicit this from local");
22+
SubSpec.this.m("direct explicit this from local");
23+
}
24+
}
25+
}
26+
}
27+
28+
class SubGen<S> extends Gen<S> {
29+
void foo() {
30+
m((S)"direct implicit this (generic sub)");
31+
this.m((S)"direct explicit this (generic sub)");
32+
}
33+
34+
class Inner {
35+
void bar() {
36+
m((S)"direct implicit this from inner (generic sub)");
37+
SubGen.this.m((S)"direct explicit this from inner (generic sub)");
38+
}
39+
}
40+
}
41+
42+
class Intermediate<S> extends Gen<S> { }
43+
44+
class GrandchildSpec extends Intermediate<String> {
45+
void foo() {
46+
m("indirect implicit this");
47+
this.m("indirect explicit this");
48+
}
49+
50+
class Inner {
51+
void bar() {
52+
m("indirect implicit this from inner");
53+
GrandchildSpec.this.m("indirect explicit this from inner");
54+
}
55+
}
56+
}
57+
58+
class GrandchildGen<R> extends Intermediate<R> {
59+
void foo() {
60+
m((R)"indirect implicit this (generic sub)");
61+
this.m((R)"indirect explicit this (generic sub)");
62+
}
63+
64+
class Inner {
65+
void bar() {
66+
m((R)"indirect implicit this from inner (generic sub)");
67+
GrandchildGen.this.m((R)"indirect explicit this from inner (generic sub)");
68+
}
69+
}
70+
}
71+
72+
class UninvolvedOuter {
73+
class InnerGen<T> {
74+
void m(T t) { }
75+
}
76+
77+
class InnerSpec extends InnerGen<String> {
78+
void foo() {
79+
m("direct implicit this, from inner to inner");
80+
this.m("direct explicit this, from inner to inner");
81+
}
82+
}
83+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
| Test.java:7:5:7:29 | m(...) | Test.java:7:7:7:28 | "direct implicit this" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
2+
| Test.java:8:5:8:34 | m(...) | Test.java:8:12:8:33 | "direct explicit this" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
3+
| Test.java:13:7:13:42 | m(...) | Test.java:13:9:13:41 | "direct implicit this from inner" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
4+
| Test.java:14:7:14:55 | m(...) | Test.java:14:22:14:54 | "direct explicit this from inner" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
5+
| Test.java:21:9:21:44 | m(...) | Test.java:21:11:21:43 | "direct implicit this from local" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
6+
| Test.java:22:9:22:57 | m(...) | Test.java:22:24:22:56 | "direct explicit this from local" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
7+
| Test.java:30:5:30:46 | m(...) | Test.java:30:10:30:45 | "direct implicit this (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<S> |
8+
| Test.java:31:5:31:51 | m(...) | Test.java:31:15:31:50 | "direct explicit this (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<S> |
9+
| Test.java:36:7:36:59 | m(...) | Test.java:36:12:36:58 | "direct implicit this from inner (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<S> |
10+
| Test.java:37:7:37:71 | m(...) | Test.java:37:24:37:70 | "direct explicit this from inner (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<S> |
11+
| Test.java:46:5:46:31 | m(...) | Test.java:46:7:46:30 | "indirect implicit this" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
12+
| Test.java:47:5:47:36 | m(...) | Test.java:47:12:47:35 | "indirect explicit this" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
13+
| Test.java:52:7:52:44 | m(...) | Test.java:52:9:52:43 | "indirect implicit this from inner" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
14+
| Test.java:53:7:53:64 | m(...) | Test.java:53:29:53:63 | "indirect explicit this from inner" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<String> |
15+
| Test.java:60:5:60:48 | m(...) | Test.java:60:10:60:47 | "indirect implicit this (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<R> |
16+
| Test.java:61:5:61:53 | m(...) | Test.java:61:15:61:52 | "indirect explicit this (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<R> |
17+
| Test.java:66:7:66:61 | m(...) | Test.java:66:12:66:60 | "indirect implicit this from inner (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<R> |
18+
| Test.java:67:7:67:80 | m(...) | Test.java:67:31:67:79 | "indirect explicit this from inner (generic sub)" | Gen.class:0:0:0:0 | m | Gen.class:0:0:0:0 | Gen<R> |
19+
| Test.java:79:7:79:52 | m(...) | Test.java:79:9:79:51 | "direct implicit this, from inner to inner" | UninvolvedOuter$InnerGen.class:0:0:0:0 | m | UninvolvedOuter$InnerGen.class:0:0:0:0 | InnerGen<String> |
20+
| Test.java:80:7:80:57 | m(...) | Test.java:80:14:80:56 | "direct explicit this, from inner to inner" | UninvolvedOuter$InnerGen.class:0:0:0:0 | m | UninvolvedOuter$InnerGen.class:0:0:0:0 | InnerGen<String> |
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import java
2+
3+
from MethodAccess ma
4+
select ma, ma.getAnArgument().getAChildExpr*().(StringLiteral), ma.getCallee(),
5+
ma.getCallee().getDeclaringType()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| GenericTest.java:14:7:14:9 | f(...) | A $@ is called instead of a $@. | GenericTest.class:0:0:0:0 | f | method declared in a superclass | GenericTest.java:9:8:9:8 | f | method with the same signature in an enclosing class |
2+
| Test.java:14:7:14:9 | f(...) | A $@ is called instead of a $@. | Test.java:3:8:3:8 | f | method declared in a superclass | Test.java:9:8:9:8 | f | method with the same signature in an enclosing class |

0 commit comments

Comments
 (0)