Skip to content

Commit 38f517e

Browse files
committed
Java: Add lambda-aware test detection to VisibleForTesting query
1 parent 4149968 commit 38f517e

File tree

3 files changed

+18
-6
lines changed

3 files changed

+18
-6
lines changed

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,20 @@ predicate isWithinVisibleForTestingContext(Callable c) {
3939
isWithinVisibleForTestingContext(c.getEnclosingCallable())
4040
}
4141

42+
/**
43+
* Holds if `e` is within a test method context, including lambda expressions
44+
* within test methods and nested lambdas.
45+
*/
46+
private predicate isWithinTest(Expr e) {
47+
e.getEnclosingCallable() instanceof LikelyTestMethod
48+
or
49+
exists(Method lambda, LambdaExpr lambdaExpr |
50+
lambda = lambdaExpr.asMethod() and
51+
lambda.getEnclosingCallable*() instanceof LikelyTestMethod and
52+
e.getEnclosingCallable() = lambda
53+
)
54+
}
55+
4256
from Annotatable annotated, Expr e
4357
where
4458
annotated.getAnAnnotation().getType().hasName("VisibleForTesting") and
@@ -89,7 +103,7 @@ where
89103
)
90104
) and
91105
// not in a test where use is appropriate
92-
not e.getEnclosingCallable() instanceof LikelyTestMethod and
106+
not isWithinTest(e) and
93107
// not when the accessing method or any enclosing method is @VisibleForTesting (test-to-test communication)
94108
not isWithinVisibleForTestingContext(e.getEnclosingCallable()) and
95109
// not when used in annotation contexts

java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,4 @@
1414
| packagetwo/Source.java:14:17:14:29 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
1515
| packagetwo/Source.java:20:28:20:47 | new AnnotatedClass(...) | Access of $@ annotated with VisibleForTesting found in production code. | packageone/AnnotatedClass.java:4:14:4:27 | AnnotatedClass | element |
1616
| packagetwo/Source.java:24:30:24:40 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
17-
| packagetwo/Source.java:28:27:28:39 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
18-
| packagetwo/Test.java:24:30:24:40 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
19-
| packagetwo/Test.java:28:27:28:39 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
17+
| packagetwo/Source.java:28:27:28:39 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |

java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ void f() {
2121

2222
// Lambda usage
2323
Runnable lambda = () -> {
24-
String lambdaS = Annotated.m; // $ SPURIOUS: Alert
24+
String lambdaS = Annotated.m; // COMPLIANT
2525
String lambdaS1 = Annotated.m1; // COMPLIANT
2626
String lambdaS2 = Annotated.m2; // COMPLIANT
2727

28-
int lambdaI = Annotated.f(); // $ SPURIOUS: Alert
28+
int lambdaI = Annotated.f(); // COMPLIANT
2929
int lambdaI2 = Annotated.fPublic(); // COMPLIANT
3030
int lambdaI3 = Annotated.fProtected(); // COMPLIANT
3131
};

0 commit comments

Comments
 (0)