Java: Exclude @VisibleForTesting-to-@VisibleForTesting access from VisibleForTestingAbuse alerts

Author: Napalys

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

@@ -33,6 +33,15 @@ predicate isWithinPackage(Expr e, RefType t) {
   e.getCompilationUnit().getPackage() = t.getPackage()
 }
 
+/**
+ * Holds if a callable or any of its enclosing callables is annotated with @VisibleForTesting
+ */
+predicate isWithinVisibleForTestingContext(Callable c) {
+  c.getAnAnnotation().getType().hasName("VisibleForTesting")
+  or
+  isWithinVisibleForTestingContext(c.getEnclosingCallable())
+}
+
 /**
  * Holds if a nested class is within a static context
  */
@@ -136,6 +145,8 @@ where
   ) and
   // not in a test where use is appropriate
   not e.getEnclosingCallable() instanceof LikelyTestMethod and
+  // not when the accessing method or any enclosing method is @VisibleForTesting (test-to-test communication)
+  not isWithinVisibleForTestingContext(e.getEnclosingCallable()) and
   // also omit our own ql unit test where it is acceptable
   not e.getEnclosingCallable()
       .getFile()

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

@@ -1,11 +1,3 @@
-| packageone/SourcePackage1.java:9:21:9:32 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
-| packageone/SourcePackage1.java:10:21:10:32 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
-| packageone/SourcePackage1.java:12:18:12:36 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
-| packageone/SourcePackage1.java:13:18:13:39 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
-| packageone/SourcePackage1.java:16:31:16:42 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
-| packageone/SourcePackage1.java:17:31:17:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
-| packageone/SourcePackage1.java:18:28:18:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
-| packageone/SourcePackage1.java:19:28:19:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
 | packageone/SourcePackage.java:9:21:9:32 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
 | packageone/SourcePackage.java:10:21:10:32 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
 | packageone/SourcePackage.java:16:18:16:36 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |

java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java

@@ -6,17 +6,17 @@ public class SourcePackage1 extends Annotated {
     @VisibleForTesting
     public void f() {
 
-        String s1 = Annotated.m1; // $ SPURIOUS: Alert
-        String s2 = Annotated.m2; // $ SPURIOUS: Alert
+        String s1 = Annotated.m1;
+        String s2 = Annotated.m2;
 
-        int i2 = Annotated.fPublic(); // $ SPURIOUS: Alert
-        int i3 = Annotated.fProtected(); // $ SPURIOUS: Alert
+        int i2 = Annotated.fPublic();
+        int i3 = Annotated.fProtected();
 
         Runnable lambda = () -> {
-            String lambdaS1 = Annotated.m1; // $ SPURIOUS: Alert
-            String lambdaS2 = Annotated.m2; // $ SPURIOUS: Alert
-            int lambdaI2 = Annotated.fPublic(); // $ SPURIOUS: Alert
-            int lambdaI3 = Annotated.fProtected(); // $ SPURIOUS: Alert
+            String lambdaS1 = Annotated.m1;
+            String lambdaS2 = Annotated.m2;
+            int lambdaI2 = Annotated.fPublic();
+            int lambdaI3 = Annotated.fProtected();
         };
     }
 }