From 0c14d93bc678319a70da5e1a0c2cc26815a6d30b Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 10:53:59 +0200
Subject: [PATCH 01/22] Java: Added new query `java/visible-for-testing-abuse`

---
 .../VisibleForTestingAbuse.md                 |  39 ++++++
 .../VisibleForTestingAbuse.ql                 | 128 ++++++++++++++++++
 .../VisibleForTestingAbuse.expected           |   4 +
 .../VisibleForTestingAbuse.qlref              |   1 +
 .../packageone/AnnotatedClass.java            |   6 +
 .../packageone/SourcePackage.java             |  10 ++
 .../packageone/VisibleForTesting.java         |   4 +
 .../packagetwo/Annotated.java                 |  15 ++
 .../packagetwo/Source.java                    |  12 ++
 .../packagetwo/Test.java                      |  12 ++
 10 files changed, 231 insertions(+)
 create mode 100644 java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md
 create mode 100644 java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packageone/AnnotatedClass.java
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packageone/VisibleForTesting.java
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md
new file mode 100644
index 000000000000..bbda346aa9a5
--- /dev/null
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md	
@@ -0,0 +1,39 @@
+# J-T-003: Accessing any method, field or class annotated with `@VisibleForTesting` from production code is discouraged
+
+Accessing class members annotated with `@VisibleForTesting` from production code goes against the intention of the annotation and may indicate programmer error.
+
+## Overview
+
+The `@VisibleForTesting` serves to increase visibility of methods, fields or classes for the purposes of testing. Accessing methods, fields or classes that are annotated with `@VisibleForTesting` in production code (not test code) abuses the intention of the annotation.
+
+## Recommendation
+
+Only access methods, fields or classes annotated with `@VisibleForTesting` from test code. If the visibility of the methods, fields or classes should generally be relaxed, use Java language access modifiers.
+
+## Example
+
+```java
+public class Annotated {
+@VisibleForTesting static int f(){}
+}
+
+/* src/test/java/Test.java */
+int i = Annotated.f(); // COMPLIANT
+
+/* src/main/Source.java */
+ int i = Annotated.f(); // NON_COMPLIANT
+
+```
+
+## Implementation notes
+
+This rule alerts on any implementation of the annotation `VisibleForTesting`, regardless of where it is provided from.
+
+The rule also uses the following logic to determine what an abuse of the annotation is:
+
+  1) If public or protected member/type is annotated with `VisibleForTesting`, it's assumed that package-private access is enough for production code. Therefore the rule alerts when a public or protected member/type annotated with `VisibleForTesting` is used outside of its declaring package.
+  2) If package-private member/type is annotated with `VisibleForTesting`, it's assumed that private access is enough for production code. Therefore the rule alerts when a package-private member/type annotated with `VisibleForTesting` is used outside its declaring class.
+
+## References
+- Example Specific Implementation of a VisibleForTesting Annotation: [AssertJ VisibleForTesting](https://javadoc.io/doc/org.assertj/assertj-core/latest/org/assertj/core/util/VisibleForTesting.html)
+- Assumptions of what level of access is permittable for each access modifier and the annotation: [JetBrains VisibleForTesting](https://javadoc.io/doc/org.jetbrains/annotations/22.0.0/org/jetbrains/annotations/VisibleForTesting.html)
\ No newline at end of file
diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
new file mode 100644
index 000000000000..3900f8474485
--- /dev/null
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -0,0 +1,128 @@
+/**
+ * @id java/visible-for-testing-abuse
+ * @name Accessing any method, field or class annotated with `@VisibleForTesting` from production code is discouraged
+ * @description Accessing any method, field or class annotated with `@VisibleForTesting` from
+ *              production code goes against the intention of the annotation and may indicate
+ *              programmer error.
+ * @kind problem
+ * @precision high
+ * @problem.severity warning
+ * @tags maintainability
+ *       readability
+ */
+
+import java
+
+/**
+ * A `Callable` is within some `RefType`
+ */
+predicate isWithinType(Callable c, RefType t) { c.getDeclaringType() = t }
+
+/**
+ * A `Callable` is within same package as the `RefType`
+ */
+predicate isWithinPackage(Callable c, RefType t) {
+  c.getDeclaringType().getPackage() = t.getPackage()
+}
+
+predicate withinStaticContext(NestedClass c) {
+  c.isStatic() or
+  c.(AnonymousClass).getClassInstanceExpr().getEnclosingCallable().isStatic() // JLS 15.9.2
+}
+
+RefType enclosingInstanceType(Class inner) {
+  not withinStaticContext(inner) and
+  result = inner.(NestedClass).getEnclosingType()
+}
+
+class OuterClass extends Class {
+  OuterClass() { this = enclosingInstanceType+(_) }
+}
+
+/**
+ * An innerclass is accessed outside of its outerclass
+ * and also outside of its fellow inner parallel classes
+ */
+predicate isWithinDirectOuterClassOrSiblingInner(
+  Callable classInstanceEnclosing, RefType typeBeingConstructed
+) {
+  exists(NestedClass inner, OuterClass outer |
+    outer = enclosingInstanceType(inner) and
+    typeBeingConstructed = inner and
+    // where the inner is called from the outer class
+    classInstanceEnclosing.getDeclaringType() = outer
+  )
+  or
+  // and inner is called from the a parallel inner
+  exists(NestedClass inner, OuterClass outer, NestedClass otherinner |
+    typeBeingConstructed = inner and
+    outer = enclosingInstanceType(otherinner) and
+    outer = enclosingInstanceType(inner) and
+    classInstanceEnclosing.getDeclaringType() = otherinner
+  )
+}
+
+from Annotatable annotated, Annotation annotation, Expr e
+where
+  annotation.getType().hasName("VisibleForTesting") and
+  annotated.getAnAnnotation() = annotation and
+  (
+    // field access
+    exists(FieldAccess v |
+      v = e and
+      v.getField() = annotated and
+      // depending on the visiblity of the field, using the annotation to abuse the visibility may/may not be occurring
+      (
+        // if its package protected report when its used outside its class bc it should have been private (class only permitted)
+        v.getField().isPackageProtected() and
+        not isWithinType(v.getEnclosingCallable(), v.getField().getDeclaringType())
+        or
+        // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
+        (v.getField().isPublic() or v.getField().isProtected()) and
+        not isWithinPackage(v.getEnclosingCallable(), v.getField().getDeclaringType())
+      )
+    )
+    or
+    // class instantiation
+    exists(ClassInstanceExpr c |
+      c = e and
+      c.getConstructedType() = annotated and
+      // depending on the visiblity of the class, using the annotation to abuse the visibility may/may not be occurring
+      // if public report when its used outside its package because package protected should have been enough (package only permitted)
+      (
+        c.getConstructedType().isPublic() and
+        not isWithinPackage(c.getEnclosingCallable(), c.getConstructedType())
+        or
+        // if its package protected report when its used outside its outer class bc it should have been private (outer class only permitted)
+        c.getConstructedType().hasNoModifier() and
+        // and the class is an innerclass, because otherwise recommending a lower accessibility makes no sense (only inner classes can be private)
+        exists(enclosingInstanceType(c.getConstructedType())) and
+        not isWithinDirectOuterClassOrSiblingInner(c.getEnclosingCallable(), c.getConstructedType())
+      )
+    )
+    or
+    // method access
+    exists(MethodCall c |
+      c = e and
+      c.getMethod() = annotated and
+      // depending on the visiblity of the method, using the annotation to abuse the visibility may/may not be occurring
+      (
+        // if its package protected report when its used outside its class bc it should have been private (class only permitted)
+        c.getMethod().isPackageProtected() and
+        not isWithinType(c.getEnclosingCallable(), c.getMethod().getDeclaringType())
+        or
+        // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
+        (c.getMethod().isPublic() or c.getMethod().isProtected()) and
+        not isWithinPackage(c.getEnclosingCallable(), c.getMethod().getDeclaringType())
+      )
+    )
+  ) and
+  // not in a test where use is appropriate
+  not e.getEnclosingCallable() instanceof LikelyTestMethod and
+  // also omit our own ql unit test where it is acceptable
+  not e.getEnclosingCallable()
+      .getFile()
+      .getAbsolutePath()
+      .matches("%java/ql/test/query-tests/%Test.java")
+select e, "Access of $@ annotated with VisibleForTesting found in production code.", annotated,
+  "element"
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
new file mode 100644
index 000000000000..3d1278c1418f
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -0,0 +1,4 @@
+| packageone/SourcePackage.java:8:21:8:32 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
+| packagetwo/Source.java:7:17:7:29 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:12:16:12:16 | f | element |
+| packagetwo/Source.java:8:20:8:30 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
+| packagetwo/Source.java:9:28:9:47 | new AnnotatedClass(...) | Access of $@ annotated with VisibleForTesting found in production code. | packageone/AnnotatedClass.java:4:14:4:27 | AnnotatedClass | element |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref
new file mode 100644
index 000000000000..6f789e7b47ba
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref
@@ -0,0 +1 @@
+Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/AnnotatedClass.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/AnnotatedClass.java
new file mode 100644
index 000000000000..1fdbea1571e2
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/AnnotatedClass.java
@@ -0,0 +1,6 @@
+package packageone;
+
+@VisibleForTesting
+public class AnnotatedClass {
+    public AnnotatedClass() {}
+}
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
new file mode 100644
index 000000000000..118d021c2771
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
@@ -0,0 +1,10 @@
+package packageone;
+
+import packagetwo.Annotated;
+
+public class SourcePackage extends Annotated {
+    void f() {
+        AnnotatedClass a = new AnnotatedClass(); // COMPLIANT - same package
+        String s1 = Annotated.m1; // NON_COMPLIANT
+    }
+}
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/VisibleForTesting.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/VisibleForTesting.java
new file mode 100644
index 000000000000..28aedbf4e539
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/VisibleForTesting.java
@@ -0,0 +1,4 @@
+package packageone;
+
+public @interface VisibleForTesting {
+}
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
new file mode 100644
index 000000000000..e253e4a2cddc
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
@@ -0,0 +1,15 @@
+package packagetwo;
+
+import packageone.*;
+
+public class Annotated {
+    @VisibleForTesting
+    static String m;
+    @VisibleForTesting
+    static protected String m1;
+
+    @VisibleForTesting
+    static int f() {
+        return 1;
+    }
+}
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
new file mode 100644
index 000000000000..9d60b2cc5a7c
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
@@ -0,0 +1,12 @@
+package packagetwo;
+
+import packageone.*;
+
+public class Source {
+    void f() {
+        int i = Annotated.f(); // NON_COMPLIANT
+        String s = Annotated.m; // NON_COMPLIANT
+        AnnotatedClass a = new AnnotatedClass(); // NON_COMPLIANT
+        String s1 = Annotated.m1; // COMPLIANT - same package
+    }
+}
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
new file mode 100644
index 000000000000..d4552657f159
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
@@ -0,0 +1,12 @@
+package packagetwo;
+
+import packageone.*;
+
+public class Test {
+    void f() {
+        int i = Annotated.f(); // COMPLIANT
+        String s = Annotated.m; // COMPLIANT
+        AnnotatedClass a = new AnnotatedClass(); // COMPLIANT
+        String s1 = Annotated.m1; // COMPLIANT
+    }
+}

From 652e9cba3d8a1fcb3a54a69b63826e2a2715f134 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 11:23:02 +0200
Subject: [PATCH 02/22] Java: Added inline test expectations for
 `java/visible-for-testing-abuse`

---
 .../VisibleForTestingAbuse/VisibleForTestingAbuse.qlref     | 3 ++-
 .../VisibleForTestingAbuse/packageone/SourcePackage.java    | 2 +-
 .../VisibleForTestingAbuse/packagetwo/Source.java           | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref
index 6f789e7b47ba..57947f804319 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.qlref
@@ -1 +1,2 @@
-Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
+query: Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
index 118d021c2771..5198555d689c 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
@@ -5,6 +5,6 @@
 public class SourcePackage extends Annotated {
     void f() {
         AnnotatedClass a = new AnnotatedClass(); // COMPLIANT - same package
-        String s1 = Annotated.m1; // NON_COMPLIANT
+        String s1 = Annotated.m1; // $ Alert
     }
 }
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
index 9d60b2cc5a7c..d513b9dd47fb 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
@@ -4,9 +4,9 @@
 
 public class Source {
     void f() {
-        int i = Annotated.f(); // NON_COMPLIANT
-        String s = Annotated.m; // NON_COMPLIANT
-        AnnotatedClass a = new AnnotatedClass(); // NON_COMPLIANT
+        int i = Annotated.f(); // $ Alert
+        String s = Annotated.m; // $ Alert
+        AnnotatedClass a = new AnnotatedClass(); // $ Alert
         String s1 = Annotated.m1; // COMPLIANT - same package
     }
 }

From ff6ddd2893432f2b260455324af7f644a049864b Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 11:28:01 +0200
Subject: [PATCH 03/22] Java: Promoted `java/visible-for-testing-abuse` to
 quality

---
 .../java/query-suite/java-code-quality-extended.qls.expected   | 1 +
 .../java/query-suite/java-code-quality.qls.expected            | 1 +
 .../Implementation Hiding/VisibleForTestingAbuse.ql            | 3 ++-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected
index 4a7364379953..23bf29d21df1 100644
--- a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected
+++ b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected
@@ -73,6 +73,7 @@ ql/java/ql/src/Violations of Best Practice/Exception Handling/IgnoreExceptionalR
 ql/java/ql/src/Violations of Best Practice/Exception Handling/NumberFormatException.ql
 ql/java/ql/src/Violations of Best Practice/Implementation Hiding/AbstractToConcreteCollection.ql
 ql/java/ql/src/Violations of Best Practice/Implementation Hiding/ExposeRepresentation.ql
+ql/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/AmbiguousOuterSuper.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingMethodNames.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql
diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected
index 17253dbe0f89..8a6a605fdae8 100644
--- a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected
+++ b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected
@@ -71,6 +71,7 @@ ql/java/ql/src/Violations of Best Practice/Exception Handling/IgnoreExceptionalR
 ql/java/ql/src/Violations of Best Practice/Exception Handling/NumberFormatException.ql
 ql/java/ql/src/Violations of Best Practice/Implementation Hiding/AbstractToConcreteCollection.ql
 ql/java/ql/src/Violations of Best Practice/Implementation Hiding/ExposeRepresentation.ql
+ql/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/AmbiguousOuterSuper.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingMethodNames.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql
diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 3900f8474485..54ce53fcd4c4 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -7,7 +7,8 @@
  * @kind problem
  * @precision high
  * @problem.severity warning
- * @tags maintainability
+ * @tags quality
+ *       maintainability
  *       readability
  */
 

From 2a16f4829ec658c8181bd35b960aa30a19720023 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 12:55:08 +0200
Subject: [PATCH 04/22] Java: Expanded test suite of
 `java/visible-for-testing-abuse`

---
 .../VisibleForTestingAbuse.expected           | 27 ++++++++-
 .../packageone/SourcePackage.java             | 26 +++++++-
 .../packagetwo/Annotated.java                 | 60 +++++++++++++++++++
 .../packagetwo/Source.java                    | 26 +++++++-
 .../packagetwo/Test.java                      | 26 +++++++-
 5 files changed, 157 insertions(+), 8 deletions(-)

diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 3d1278c1418f..6dd775b2802c 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -1,4 +1,25 @@
-| packageone/SourcePackage.java:8:21:8:32 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
-| packagetwo/Source.java:7:17:7:29 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:12:16:12:16 | f | 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 |
+| packageone/SourcePackage.java:17:18:17:39 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
+| packageone/SourcePackage.java:25:31:25:42 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
+| packageone/SourcePackage.java:26:31:26:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
+| packageone/SourcePackage.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
+| packageone/SourcePackage.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
+| packagetwo/Annotated.java:49:31:49:31 | m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
+| packagetwo/Annotated.java:50:32:50:33 | m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
+| packagetwo/Annotated.java:51:32:51:33 | m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
+| packagetwo/Annotated.java:54:26:54:28 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
+| packagetwo/Annotated.java:56:32:56:40 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
+| packagetwo/Annotated.java:57:35:57:46 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
+| packagetwo/Annotated.java:64:28:64:28 | m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
+| packagetwo/Annotated.java:69:26:69:28 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
 | packagetwo/Source.java:8:20:8:30 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
-| packagetwo/Source.java:9:28:9:47 | new AnnotatedClass(...) | Access of $@ annotated with VisibleForTesting found in production code. | packageone/AnnotatedClass.java:4:14:4:27 | AnnotatedClass | element |
+| 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 |
+| 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 |
+| 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 |
+| packagetwo/Source.java:25:31:25:42 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
+| packagetwo/Source.java:26:31:26:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
+| 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 |
+| packagetwo/Source.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
+| packagetwo/Source.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
index 5198555d689c..96363da79c97 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
@@ -4,7 +4,31 @@
 
 public class SourcePackage extends Annotated {
     void f() {
-        AnnotatedClass a = new AnnotatedClass(); // COMPLIANT - same package
+        // Fields - cross-package access (only accessible ones)
+        // String s = Annotated.m; // Cannot access package-private from different package
         String s1 = Annotated.m1; // $ Alert
+        String s2 = Annotated.m2; // $ Alert
+        // String s3 = Annotated.m3; // Cannot access private field
+        
+        // Methods - cross-package access (only accessible ones)
+        // int i = Annotated.f(); // Cannot access package-private from different package
+        // int i1 = Annotated.fPrivate(); // Cannot access private method
+        int i2 = Annotated.fPublic(); // $ Alert
+        int i3 = Annotated.fProtected(); // $ Alert
+        
+        // Same package class
+        AnnotatedClass a = new AnnotatedClass(); // COMPLIANT - same package
+        
+        // Lambda usage - cross-package (only accessible members)
+        Runnable lambda = () -> {
+            // String lambdaS = Annotated.m; // Cannot access package-private
+            String lambdaS1 = Annotated.m1; // $ Alert
+            String lambdaS2 = Annotated.m2; // $ Alert
+            
+            // int lambdaI = Annotated.f(); // Cannot access package-private
+            int lambdaI2 = Annotated.fPublic(); // $ Alert
+            int lambdaI3 = Annotated.fProtected(); // $ Alert
+        };
+        lambda.run();
     }
 }
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
index e253e4a2cddc..cdf0cfeae51b 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
@@ -7,9 +7,69 @@ public class Annotated {
     static String m;
     @VisibleForTesting
     static protected String m1;
+    @VisibleForTesting
+    static public String m2;
+    @VisibleForTesting
+    static private String m3;
 
     @VisibleForTesting
     static int f() {
         return 1;
     }
+
+    @VisibleForTesting
+    static private int fPrivate() {
+        return 1;
+    }
+
+    @VisibleForTesting
+    static public int fPublic() {
+        return 1;
+    }
+
+    @VisibleForTesting
+    static protected int fProtected() {
+        return 1;
+    }
+
+    private static void resetPriorities() {
+        String priority = m;
+        String priority1 = m1;
+        String priority2 = m2;
+        String priority3 = m3;
+
+        int result = f();
+        int resultPrivate = fPrivate();
+        int resultPublic = fPublic();
+        int resultProtected = fProtected();
+    }
+
+    private static void resetPriorities2() {
+        Runnable task = () -> {
+            String priority = m; // $ SPURIOUS: Alert
+            String priority1 = m1; // $ SPURIOUS: Alert
+            String priority2 = m2; // $ SPURIOUS: Alert
+            String priority3 = m3;
+
+            int result = f(); // $ SPURIOUS: Alert
+            int resultPrivate = fPrivate();
+            int resultPublic = fPublic(); // $ SPURIOUS: Alert
+            int resultProtected = fProtected(); // $ SPURIOUS: Alert
+        };
+        task.run();
+    }
+
+    private static class InnerClass {
+        void useVisibleForMembers() {
+            String field = m; // $ SPURIOUS: Alert
+            String field1 = m1;
+            String field2 = m2;
+            String field3 = m3;
+
+            int method = f(); // $ SPURIOUS: Alert
+            int methodPrivate = fPrivate();
+            int methodPublic = fPublic();
+            int methodProtected = fProtected();
+        }
+    }
 }
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
index d513b9dd47fb..358e145022c3 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
@@ -4,9 +4,31 @@
 
 public class Source {
     void f() {
-        int i = Annotated.f(); // $ Alert
+        // Fields
         String s = Annotated.m; // $ Alert
-        AnnotatedClass a = new AnnotatedClass(); // $ Alert
         String s1 = Annotated.m1; // COMPLIANT - same package
+        String s2 = Annotated.m2;
+        // String s3 = Annotated.m3; // Cannot access private field
+        
+        // Methods
+        int i = Annotated.f(); // $ Alert
+        // int i1 = Annotated.fPrivate(); // Cannot access private method
+        int i2 = Annotated.fPublic();
+        int i3 = Annotated.fProtected();
+        
+        // Other class
+        AnnotatedClass a = new AnnotatedClass(); // $ Alert
+        
+        // Lambda usage
+        Runnable lambda = () -> {
+            String lambdaS = Annotated.m; // $ Alert
+            String lambdaS1 = Annotated.m1; // $ SPURIOUS: Alert
+            String lambdaS2 = Annotated.m2; // $ SPURIOUS: Alert
+            
+            int lambdaI = Annotated.f(); // $ Alert
+            int lambdaI2 = Annotated.fPublic(); // $ SPURIOUS: Alert
+            int lambdaI3 = Annotated.fProtected(); // $ SPURIOUS: Alert
+        };
+        lambda.run();
     }
 }
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
index d4552657f159..b861d921e9a3 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
@@ -4,9 +4,31 @@
 
 public class Test {
     void f() {
-        int i = Annotated.f(); // COMPLIANT
+        // Fields
         String s = Annotated.m; // COMPLIANT
-        AnnotatedClass a = new AnnotatedClass(); // COMPLIANT
         String s1 = Annotated.m1; // COMPLIANT
+        String s2 = Annotated.m2; // COMPLIANT
+        // String s3 = Annotated.m3; // Cannot access private field
+        
+        // Methods
+        int i = Annotated.f(); // COMPLIANT
+        // int i1 = Annotated.fPrivate(); // Cannot access private method
+        int i2 = Annotated.fPublic(); // COMPLIANT
+        int i3 = Annotated.fProtected(); // COMPLIANT
+        
+        // Other class
+        AnnotatedClass a = new AnnotatedClass(); // COMPLIANT
+        
+        // Lambda usage
+        Runnable lambda = () -> {
+            String lambdaS = Annotated.m; // COMPLIANT
+            String lambdaS1 = Annotated.m1; // COMPLIANT
+            String lambdaS2 = Annotated.m2; // COMPLIANT
+            
+            int lambdaI = Annotated.f(); // COMPLIANT
+            int lambdaI2 = Annotated.fPublic(); // COMPLIANT
+            int lambdaI3 = Annotated.fProtected(); // COMPLIANT
+        };
+        lambda.run();
     }
 }

From fbf18af076129df000de83343475ed2372c7499c Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 13:20:18 +0200
Subject: [PATCH 05/22] Java: enchanced check if it is within same package

---
 .../Implementation Hiding/VisibleForTestingAbuse.ql    | 10 +++++-----
 .../VisibleForTestingAbuse.expected                    |  8 --------
 .../VisibleForTestingAbuse/packagetwo/Annotated.java   |  8 ++++----
 .../VisibleForTestingAbuse/packagetwo/Source.java      |  8 ++++----
 4 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 54ce53fcd4c4..e41e1b897c42 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -22,8 +22,8 @@ predicate isWithinType(Callable c, RefType t) { c.getDeclaringType() = t }
 /**
  * A `Callable` is within same package as the `RefType`
  */
-predicate isWithinPackage(Callable c, RefType t) {
-  c.getDeclaringType().getPackage() = t.getPackage()
+predicate isWithinPackage(Expr e, RefType t) {
+  e.getCompilationUnit().getPackage() = t.getPackage()
 }
 
 predicate withinStaticContext(NestedClass c) {
@@ -80,7 +80,7 @@ where
         or
         // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
         (v.getField().isPublic() or v.getField().isProtected()) and
-        not isWithinPackage(v.getEnclosingCallable(), v.getField().getDeclaringType())
+        not isWithinPackage(v, v.getField().getDeclaringType())
       )
     )
     or
@@ -92,7 +92,7 @@ where
       // if public report when its used outside its package because package protected should have been enough (package only permitted)
       (
         c.getConstructedType().isPublic() and
-        not isWithinPackage(c.getEnclosingCallable(), c.getConstructedType())
+        not isWithinPackage(c, c.getConstructedType())
         or
         // if its package protected report when its used outside its outer class bc it should have been private (outer class only permitted)
         c.getConstructedType().hasNoModifier() and
@@ -114,7 +114,7 @@ where
         or
         // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
         (c.getMethod().isPublic() or c.getMethod().isProtected()) and
-        not isWithinPackage(c.getEnclosingCallable(), c.getMethod().getDeclaringType())
+        not isWithinPackage(c, c.getMethod().getDeclaringType())
       )
     )
   ) and
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 6dd775b2802c..044960e97af9 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -7,19 +7,11 @@
 | packageone/SourcePackage.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
 | packageone/SourcePackage.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
 | packagetwo/Annotated.java:49:31:49:31 | m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
-| packagetwo/Annotated.java:50:32:50:33 | m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
-| packagetwo/Annotated.java:51:32:51:33 | m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
 | packagetwo/Annotated.java:54:26:54:28 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
-| packagetwo/Annotated.java:56:32:56:40 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
-| packagetwo/Annotated.java:57:35:57:46 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
 | packagetwo/Annotated.java:64:28:64:28 | m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
 | packagetwo/Annotated.java:69:26:69:28 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
 | packagetwo/Source.java:8:20:8:30 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
 | 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 |
 | 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 |
 | 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 |
-| packagetwo/Source.java:25:31:25:42 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
-| packagetwo/Source.java:26:31:26:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
 | 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 |
-| packagetwo/Source.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
-| packagetwo/Source.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
index cdf0cfeae51b..de0085cedb46 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
@@ -47,14 +47,14 @@ private static void resetPriorities() {
     private static void resetPriorities2() {
         Runnable task = () -> {
             String priority = m; // $ SPURIOUS: Alert
-            String priority1 = m1; // $ SPURIOUS: Alert
-            String priority2 = m2; // $ SPURIOUS: Alert
+            String priority1 = m1;
+            String priority2 = m2;
             String priority3 = m3;
 
             int result = f(); // $ SPURIOUS: Alert
             int resultPrivate = fPrivate();
-            int resultPublic = fPublic(); // $ SPURIOUS: Alert
-            int resultProtected = fProtected(); // $ SPURIOUS: Alert
+            int resultPublic = fPublic();
+            int resultProtected = fProtected();
         };
         task.run();
     }
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
index 358e145022c3..94a5cccfd43b 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Source.java
@@ -22,12 +22,12 @@ void f() {
         // Lambda usage
         Runnable lambda = () -> {
             String lambdaS = Annotated.m; // $ Alert
-            String lambdaS1 = Annotated.m1; // $ SPURIOUS: Alert
-            String lambdaS2 = Annotated.m2; // $ SPURIOUS: Alert
+            String lambdaS1 = Annotated.m1;
+            String lambdaS2 = Annotated.m2;
             
             int lambdaI = Annotated.f(); // $ Alert
-            int lambdaI2 = Annotated.fPublic(); // $ SPURIOUS: Alert
-            int lambdaI3 = Annotated.fProtected(); // $ SPURIOUS: Alert
+            int lambdaI2 = Annotated.fPublic();
+            int lambdaI3 = Annotated.fProtected();
         };
         lambda.run();
     }

From 9dfb4d4301ae8b53a1fc29a360605e1a133e15cc Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 13:28:16 +0200
Subject: [PATCH 06/22] Java: Enchanced `isWithinType` to also include lambdas,
 inner classes etc.

---
 .../Implementation Hiding/VisibleForTestingAbuse.ql       | 8 ++++++--
 .../VisibleForTestingAbuse.expected                       | 4 ----
 .../VisibleForTestingAbuse/packagetwo/Annotated.java      | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index e41e1b897c42..bd1ad0c6ce75 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -15,9 +15,13 @@
 import java
 
 /**
- * A `Callable` is within some `RefType`
+ * A `Callable` is within some `RefType` (including through lambdas and inner classes)
  */
-predicate isWithinType(Callable c, RefType t) { c.getDeclaringType() = t }
+predicate isWithinType(Callable c, RefType t) {
+  c.getDeclaringType() = t
+  or
+  c.getDeclaringType().getEnclosingType*() = t
+}
 
 /**
  * A `Callable` is within same package as the `RefType`
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 044960e97af9..0668105426b4 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -6,10 +6,6 @@
 | packageone/SourcePackage.java:26:31:26:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
 | packageone/SourcePackage.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
 | packageone/SourcePackage.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
-| packagetwo/Annotated.java:49:31:49:31 | m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
-| packagetwo/Annotated.java:54:26:54:28 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
-| packagetwo/Annotated.java:64:28:64:28 | m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
-| packagetwo/Annotated.java:69:26:69:28 | f(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:16:16:16:16 | f | element |
 | packagetwo/Source.java:8:20:8:30 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
 | 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 |
 | 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 |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
index de0085cedb46..ae0bf49a03e6 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
@@ -46,12 +46,12 @@ private static void resetPriorities() {
 
     private static void resetPriorities2() {
         Runnable task = () -> {
-            String priority = m; // $ SPURIOUS: Alert
+            String priority = m;
             String priority1 = m1;
             String priority2 = m2;
             String priority3 = m3;
 
-            int result = f(); // $ SPURIOUS: Alert
+            int result = f();
             int resultPrivate = fPrivate();
             int resultPublic = fPublic();
             int resultProtected = fProtected();
@@ -61,12 +61,12 @@ private static void resetPriorities2() {
 
     private static class InnerClass {
         void useVisibleForMembers() {
-            String field = m; // $ SPURIOUS: Alert
+            String field = m;
             String field1 = m1;
             String field2 = m2;
             String field3 = m3;
 
-            int method = f(); // $ SPURIOUS: Alert
+            int method = f();
             int methodPrivate = fPrivate();
             int methodPublic = fPublic();
             int methodProtected = fProtected();

From 7e2a1944f6e06b20b18c9d8add6938afa1eb70c9 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 13:33:37 +0200
Subject: [PATCH 07/22] Java: Fix Predicate QLDoc style.

---
 .../VisibleForTestingAbuse.ql                     | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index bd1ad0c6ce75..98b97ebe5116 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -15,7 +15,7 @@
 import java
 
 /**
- * A `Callable` is within some `RefType` (including through lambdas and inner classes)
+ * Holds if a `Callable` is within some `RefType` (including through lambdas and inner classes)
  */
 predicate isWithinType(Callable c, RefType t) {
   c.getDeclaringType() = t
@@ -24,28 +24,37 @@ predicate isWithinType(Callable c, RefType t) {
 }
 
 /**
- * A `Callable` is within same package as the `RefType`
+ * Holds if a `Callable` is within same package as the `RefType`
  */
 predicate isWithinPackage(Expr e, RefType t) {
   e.getCompilationUnit().getPackage() = t.getPackage()
 }
 
+/**
+ * Holds if a nested class is within a static context
+ */
 predicate withinStaticContext(NestedClass c) {
   c.isStatic() or
   c.(AnonymousClass).getClassInstanceExpr().getEnclosingCallable().isStatic() // JLS 15.9.2
 }
 
+/**
+ * Gets the enclosing instance type for a non-static inner class
+ */
 RefType enclosingInstanceType(Class inner) {
   not withinStaticContext(inner) and
   result = inner.(NestedClass).getEnclosingType()
 }
 
+/**
+ * A class that encloses one or more inner classes
+ */
 class OuterClass extends Class {
   OuterClass() { this = enclosingInstanceType+(_) }
 }
 
 /**
- * An innerclass is accessed outside of its outerclass
+ * Holds if an innerclass is accessed outside of its outerclass
  * and also outside of its fellow inner parallel classes
  */
 predicate isWithinDirectOuterClassOrSiblingInner(

From 1e2e6eccd780d48557cc40a8c6f2f878a45005b0 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 14:37:29 +0200
Subject: [PATCH 08/22] Java: Test @VisibleForTesting method accessing
 @VisibleForTesting members

---
 .../VisibleForTestingAbuse.expected           |  9 ++++++++
 .../packageone/SourcePackage1.java            | 22 +++++++++++++++++++
 .../packagetwo/Annotated.java                 | 17 ++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java

diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 0668105426b4..89c6bac4a33b 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -1,3 +1,11 @@
+| 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 |
@@ -6,6 +14,7 @@
 | packageone/SourcePackage.java:26:31:26:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
 | packageone/SourcePackage.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
 | packageone/SourcePackage.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
+| packagetwo/Annotated.java:89:20:89:34 | getSize(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:79:13:79:19 | getSize | element |
 | packagetwo/Source.java:8:20:8:30 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
 | 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 |
 | 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 |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java
new file mode 100644
index 000000000000..b09fbc904cb4
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java
@@ -0,0 +1,22 @@
+package packageone;
+
+import packagetwo.Annotated;
+
+public class SourcePackage1 extends Annotated {
+    @VisibleForTesting
+    public void f() {
+
+        String s1 = Annotated.m1; // $ SPURIOUS: Alert
+        String s2 = Annotated.m2; // $ SPURIOUS: Alert
+
+        int i2 = Annotated.fPublic(); // $ SPURIOUS: Alert
+        int i3 = Annotated.fProtected(); // $ SPURIOUS: Alert
+
+        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
+        };
+    }
+}
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
index ae0bf49a03e6..eb6d0ad59f32 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
@@ -72,4 +72,21 @@ void useVisibleForMembers() {
             int methodProtected = fProtected();
         }
     }
+
+    @VisibleForTesting
+    static class InnerTestClass {
+        @VisibleForTesting
+        int getSize() {
+            return 42;
+        }
+        
+        @VisibleForTesting
+        private String data;
+    }
+    
+    private void useInnerClass() {
+        InnerTestClass inner = new InnerTestClass();
+        int size = inner.getSize(); // $ SPURIOUS: Alert
+        String value = inner.data;
+    }
 }

From e4042402bcf37a8796e5e715f0cf5d404f0d431b Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 14:40:19 +0200
Subject: [PATCH 09/22] Java: Resolve spurious VisibleForTestingAbuse alerts
 for inner class access patterns

---
 .../Implementation Hiding/VisibleForTestingAbuse.ql   | 11 +++++++----
 .../VisibleForTestingAbuse.expected                   |  1 -
 .../VisibleForTestingAbuse/packagetwo/Annotated.java  |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 98b97ebe5116..ef356b37ef71 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -15,12 +15,15 @@
 import java
 
 /**
- * Holds if a `Callable` is within some `RefType` (including through lambdas and inner classes)
+ * Holds if a `Callable` is within the same type hierarchy as `RefType`
+ * (including through lambdas, inner classes, and outer classes)
  */
 predicate isWithinType(Callable c, RefType t) {
-  c.getDeclaringType() = t
-  or
-  c.getDeclaringType().getEnclosingType*() = t
+  // Either the callable is in the target type, or they share a common enclosing type
+  exists(RefType commonType |
+    (c.getDeclaringType() = commonType or c.getDeclaringType().getEnclosingType*() = commonType) and
+    (t = commonType or t.getEnclosingType*() = commonType)
+  )
 }
 
 /**
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 89c6bac4a33b..cc755cf74737 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -14,7 +14,6 @@
 | packageone/SourcePackage.java:26:31:26:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
 | packageone/SourcePackage.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
 | packageone/SourcePackage.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
-| packagetwo/Annotated.java:89:20:89:34 | getSize(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:79:13:79:19 | getSize | element |
 | packagetwo/Source.java:8:20:8:30 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
 | 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 |
 | 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 |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
index eb6d0ad59f32..ad5dbed3f9b6 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Annotated.java
@@ -86,7 +86,7 @@ int getSize() {
     
     private void useInnerClass() {
         InnerTestClass inner = new InnerTestClass();
-        int size = inner.getSize(); // $ SPURIOUS: Alert
+        int size = inner.getSize();
         String value = inner.data;
     }
 }

From 225723bfeb13d1ab75575631e385ae20d379a721 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 14:57:16 +0200
Subject: [PATCH 10/22] Java: Exclude @VisibleForTesting-to-@VisibleForTesting
 access from VisibleForTestingAbuse alerts

---
 .../VisibleForTestingAbuse.ql                    | 11 +++++++++++
 .../VisibleForTestingAbuse.expected              |  8 --------
 .../packageone/SourcePackage1.java               | 16 ++++++++--------
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index ef356b37ef71..334d85563825 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/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()
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index cc755cf74737..0668105426b4 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/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 |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java
index b09fbc904cb4..d47aa167b46c 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage1.java
+++ b/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();
         };
     }
 }

From eb46e54c43d553c148571c0435ce9e5ad17e04f2 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 17:03:31 +0200
Subject: [PATCH 11/22] Java: Refactor VisibleForTestingAbuse query to reduce
 complexity

---
 .../VisibleForTestingAbuse.ql                 | 82 ++++---------------
 1 file changed, 15 insertions(+), 67 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 334d85563825..10675222f3e6 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -42,56 +42,9 @@ predicate isWithinVisibleForTestingContext(Callable c) {
   isWithinVisibleForTestingContext(c.getEnclosingCallable())
 }
 
-/**
- * Holds if a nested class is within a static context
- */
-predicate withinStaticContext(NestedClass c) {
-  c.isStatic() or
-  c.(AnonymousClass).getClassInstanceExpr().getEnclosingCallable().isStatic() // JLS 15.9.2
-}
-
-/**
- * Gets the enclosing instance type for a non-static inner class
- */
-RefType enclosingInstanceType(Class inner) {
-  not withinStaticContext(inner) and
-  result = inner.(NestedClass).getEnclosingType()
-}
-
-/**
- * A class that encloses one or more inner classes
- */
-class OuterClass extends Class {
-  OuterClass() { this = enclosingInstanceType+(_) }
-}
-
-/**
- * Holds if an innerclass is accessed outside of its outerclass
- * and also outside of its fellow inner parallel classes
- */
-predicate isWithinDirectOuterClassOrSiblingInner(
-  Callable classInstanceEnclosing, RefType typeBeingConstructed
-) {
-  exists(NestedClass inner, OuterClass outer |
-    outer = enclosingInstanceType(inner) and
-    typeBeingConstructed = inner and
-    // where the inner is called from the outer class
-    classInstanceEnclosing.getDeclaringType() = outer
-  )
-  or
-  // and inner is called from the a parallel inner
-  exists(NestedClass inner, OuterClass outer, NestedClass otherinner |
-    typeBeingConstructed = inner and
-    outer = enclosingInstanceType(otherinner) and
-    outer = enclosingInstanceType(inner) and
-    classInstanceEnclosing.getDeclaringType() = otherinner
-  )
-}
-
-from Annotatable annotated, Annotation annotation, Expr e
+from Annotatable annotated, Expr e
 where
-  annotation.getType().hasName("VisibleForTesting") and
-  annotated.getAnAnnotation() = annotation and
+  annotated.getAnAnnotation().getType().hasName("VisibleForTesting") and
   (
     // field access
     exists(FieldAccess v |
@@ -109,24 +62,6 @@ where
       )
     )
     or
-    // class instantiation
-    exists(ClassInstanceExpr c |
-      c = e and
-      c.getConstructedType() = annotated and
-      // depending on the visiblity of the class, using the annotation to abuse the visibility may/may not be occurring
-      // if public report when its used outside its package because package protected should have been enough (package only permitted)
-      (
-        c.getConstructedType().isPublic() and
-        not isWithinPackage(c, c.getConstructedType())
-        or
-        // if its package protected report when its used outside its outer class bc it should have been private (outer class only permitted)
-        c.getConstructedType().hasNoModifier() and
-        // and the class is an innerclass, because otherwise recommending a lower accessibility makes no sense (only inner classes can be private)
-        exists(enclosingInstanceType(c.getConstructedType())) and
-        not isWithinDirectOuterClassOrSiblingInner(c.getEnclosingCallable(), c.getConstructedType())
-      )
-    )
-    or
     // method access
     exists(MethodCall c |
       c = e and
@@ -142,6 +77,19 @@ where
         not isWithinPackage(c, c.getMethod().getDeclaringType())
       )
     )
+    or
+    // Class instantiation - report if used outside appropriate scope
+    exists(ClassInstanceExpr c |
+      c = e and
+      c.getConstructedType() = annotated and
+      (
+        c.getConstructedType().isPublic() and not isWithinPackage(c, c.getConstructedType())
+        or
+        c.getConstructedType().hasNoModifier() and
+        c.getConstructedType() instanceof NestedClass and
+        not isWithinType(c.getEnclosingCallable(), c.getConstructedType())
+      )
+    )
   ) and
   // not in a test where use is appropriate
   not e.getEnclosingCallable() instanceof LikelyTestMethod and

From ea831a8352250d3422ce8cc2c090239b9b0677b7 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 17:44:35 +0200
Subject: [PATCH 12/22] Java: Fix VisibleForTestingAbuse false positives in
 annotations

---
 .../VisibleForTestingAbuse.ql                  |  2 ++
 .../packagetwo/UseWithinAnnotation.java        | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/UseWithinAnnotation.java

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 10675222f3e6..0833440ca1b9 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -95,6 +95,8 @@ where
   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
+  // not when used in annotation contexts
+  not e.getParent*() instanceof Annotation and
   // also omit our own ql unit test where it is acceptable
   not e.getEnclosingCallable()
       .getFile()
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/UseWithinAnnotation.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/UseWithinAnnotation.java
new file mode 100644
index 000000000000..f6cdb32d53c7
--- /dev/null
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/UseWithinAnnotation.java
@@ -0,0 +1,18 @@
+package packagetwo;
+
+import packageone.*;
+
+@interface Range {
+    int min() default 0;
+    int max() default 100;
+}
+
+public class UseWithinAnnotation {
+    @VisibleForTesting
+    static final int MAX_LISTING_LENGTH_MIN = 1;
+    @VisibleForTesting
+    static final int MAX_LISTING_LENGTH_MAX = 1000;
+
+    @Range(min = MAX_LISTING_LENGTH_MIN, max = MAX_LISTING_LENGTH_MAX)
+    private int maxListingLength = MAX_LISTING_LENGTH_MAX;
+}

From d20fd5beba8b2bfcc3f949dcb620a3786da25e75 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Wed, 6 Aug 2025 17:55:16 +0200
Subject: [PATCH 13/22] Java: updated `visible-for-testing-abuse` meta data and
 docs.

---
 .../VisibleForTestingAbuse.md                 | 19 ++++++++-----------
 .../VisibleForTestingAbuse.ql                 |  4 ++--
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md
index bbda346aa9a5..5b1320d6c852 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md	
@@ -1,10 +1,8 @@
-# J-T-003: Accessing any method, field or class annotated with `@VisibleForTesting` from production code is discouraged
+## Overview
 
 Accessing class members annotated with `@VisibleForTesting` from production code goes against the intention of the annotation and may indicate programmer error.
 
-## Overview
-
-The `@VisibleForTesting` serves to increase visibility of methods, fields or classes for the purposes of testing. Accessing methods, fields or classes that are annotated with `@VisibleForTesting` in production code (not test code) abuses the intention of the annotation.
+The `@VisibleForTesting` annotation serves to increase visibility of methods, fields or classes for the purposes of testing. Accessing these annotated elements in production code (not test code) abuses the intention of the annotation.
 
 ## Recommendation
 
@@ -14,15 +12,14 @@ Only access methods, fields or classes annotated with `@VisibleForTesting` from
 
 ```java
 public class Annotated {
-@VisibleForTesting static int f(){}
+    @VisibleForTesting static int f() { return 42; }
 }
 
 /* src/test/java/Test.java */
 int i = Annotated.f(); // COMPLIANT
 
 /* src/main/Source.java */
- int i = Annotated.f(); // NON_COMPLIANT
-
+int i = Annotated.f(); // NON_COMPLIANT
 ```
 
 ## Implementation notes
@@ -31,9 +28,9 @@ This rule alerts on any implementation of the annotation `VisibleForTesting`, re
 
 The rule also uses the following logic to determine what an abuse of the annotation is:
 
-  1) If public or protected member/type is annotated with `VisibleForTesting`, it's assumed that package-private access is enough for production code. Therefore the rule alerts when a public or protected member/type annotated with `VisibleForTesting` is used outside of its declaring package.
-  2) If package-private member/type is annotated with `VisibleForTesting`, it's assumed that private access is enough for production code. Therefore the rule alerts when a package-private member/type annotated with `VisibleForTesting` is used outside its declaring class.
+  1. If a public or protected member/type is annotated with `@VisibleForTesting`, it's assumed that package-private access is enough for production code. Therefore the rule alerts when a public or protected member/type annotated with `@VisibleForTesting` is used outside of its declaring package.
+  2. If a package-private member/type is annotated with `@VisibleForTesting`, it's assumed that private access is enough for production code. Therefore the rule alerts when a package-private member/type annotated with `@VisibleForTesting` is used outside its declaring class.
 
 ## References
-- Example Specific Implementation of a VisibleForTesting Annotation: [AssertJ VisibleForTesting](https://javadoc.io/doc/org.assertj/assertj-core/latest/org/assertj/core/util/VisibleForTesting.html)
-- Assumptions of what level of access is permittable for each access modifier and the annotation: [JetBrains VisibleForTesting](https://javadoc.io/doc/org.jetbrains/annotations/22.0.0/org/jetbrains/annotations/VisibleForTesting.html)
\ No newline at end of file
+- Javadoc: [AssertJ VisibleForTesting](https://javadoc.io/doc/org.assertj/assertj-core/latest/org/assertj/core/util/VisibleForTesting.html).
+- Javadoc: [JetBrains VisibleForTesting](https://javadoc.io/doc/org.jetbrains/annotations/22.0.0/org/jetbrains/annotations/VisibleForTesting.html).
diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 0833440ca1b9..21ef1fed0af7 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -1,7 +1,7 @@
 /**
  * @id java/visible-for-testing-abuse
- * @name Accessing any method, field or class annotated with `@VisibleForTesting` from production code is discouraged
- * @description Accessing any method, field or class annotated with `@VisibleForTesting` from
+ * @name Use of VisibleForTesting in production code
+ * @description Accessing methods, fields or classes annotated with `@VisibleForTesting` from
  *              production code goes against the intention of the annotation and may indicate
  *              programmer error.
  * @kind problem

From 0b172080aa59254717acf1187c0d7e6fe239c2ae Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Thu, 7 Aug 2025 10:14:40 +0200
Subject: [PATCH 14/22] Update java/ql/src/Violations of Best
 Practice/Implementation Hiding/VisibleForTestingAbuse.ql

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---
 .../Implementation Hiding/VisibleForTestingAbuse.ql           | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 21ef1fed0af7..a1580d5dd3cd 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -50,7 +50,7 @@ where
     exists(FieldAccess v |
       v = e and
       v.getField() = annotated and
-      // depending on the visiblity of the field, using the annotation to abuse the visibility may/may not be occurring
+      // depending on the visibility of the field, using the annotation to abuse the visibility may/may not be occurring
       (
         // if its package protected report when its used outside its class bc it should have been private (class only permitted)
         v.getField().isPackageProtected() and
@@ -66,7 +66,7 @@ where
     exists(MethodCall c |
       c = e and
       c.getMethod() = annotated and
-      // depending on the visiblity of the method, using the annotation to abuse the visibility may/may not be occurring
+      // depending on the visibility of the method, using the annotation to abuse the visibility may/may not be occurring
       (
         // if its package protected report when its used outside its class bc it should have been private (class only permitted)
         c.getMethod().isPackageProtected() and

From 66f2911497e34590d98d348f03753faa44d5705a Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Fri, 22 Aug 2025 08:43:20 +0200
Subject: [PATCH 15/22] Update java/ql/src/Violations of Best
 Practice/Implementation Hiding/VisibleForTestingAbuse.ql

Co-authored-by: Michael Nebel <michaelnebel@github.com>
---
 .../Implementation Hiding/VisibleForTestingAbuse.ql          | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index a1580d5dd3cd..0fbb3a5f1dd1 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -20,10 +20,7 @@ import java
  */
 predicate isWithinType(Callable c, RefType t) {
   // Either the callable is in the target type, or they share a common enclosing type
-  exists(RefType commonType |
-    (c.getDeclaringType() = commonType or c.getDeclaringType().getEnclosingType*() = commonType) and
-    (t = commonType or t.getEnclosingType*() = commonType)
-  )
+  c.getDeclaringType().getEnclosingType*() = t.getEnclosingType*()
 }
 
 /**

From 38b3df07ee86c005b8ebf1fccbe7a165277212a0 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Fri, 22 Aug 2025 07:20:52 +0000
Subject: [PATCH 16/22] Java: Address comments

---
 .../VisibleForTestingAbuse.ql                 | 74 +++++++++----------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 0fbb3a5f1dd1..0ef3404d9880 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -24,7 +24,7 @@ predicate isWithinType(Callable c, RefType t) {
 }
 
 /**
- * Holds if a `Callable` is within same package as the `RefType`
+ * Holds if `e` is within the same package as `t`
  */
 predicate isWithinPackage(Expr e, RefType t) {
   e.getCompilationUnit().getPackage() = t.getPackage()
@@ -44,49 +44,49 @@ where
   annotated.getAnAnnotation().getType().hasName("VisibleForTesting") and
   (
     // field access
-    exists(FieldAccess v |
-      v = e and
-      v.getField() = annotated and
-      // depending on the visibility of the field, using the annotation to abuse the visibility may/may not be occurring
-      (
-        // if its package protected report when its used outside its class bc it should have been private (class only permitted)
-        v.getField().isPackageProtected() and
-        not isWithinType(v.getEnclosingCallable(), v.getField().getDeclaringType())
-        or
-        // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
-        (v.getField().isPublic() or v.getField().isProtected()) and
-        not isWithinPackage(v, v.getField().getDeclaringType())
+    e =
+      any(FieldAccess v |
+        v.getField() = annotated and
+        // depending on the visibility of the field, using the annotation to abuse the visibility may/may not be occurring
+        (
+          // if its package protected report when its used outside its class because it should have been private (class only permitted)
+          v.getField().isPackageProtected() and
+          not isWithinType(v.getEnclosingCallable(), v.getField().getDeclaringType())
+          or
+          // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
+          (v.getField().isPublic() or v.getField().isProtected()) and
+          not isWithinPackage(v, v.getField().getDeclaringType())
+        )
       )
-    )
     or
     // method access
-    exists(MethodCall c |
-      c = e and
-      c.getMethod() = annotated and
-      // depending on the visibility of the method, using the annotation to abuse the visibility may/may not be occurring
-      (
-        // if its package protected report when its used outside its class bc it should have been private (class only permitted)
-        c.getMethod().isPackageProtected() and
-        not isWithinType(c.getEnclosingCallable(), c.getMethod().getDeclaringType())
-        or
-        // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
-        (c.getMethod().isPublic() or c.getMethod().isProtected()) and
-        not isWithinPackage(c, c.getMethod().getDeclaringType())
+    e =
+      any(MethodCall c |
+        c.getMethod() = annotated and
+        // depending on the visibility of the method, using the annotation to abuse the visibility may/may not be occurring
+        (
+          // if its package protected report when its used outside its class because it should have been private (class only permitted)
+          c.getMethod().isPackageProtected() and
+          not isWithinType(c.getEnclosingCallable(), c.getMethod().getDeclaringType())
+          or
+          // if public or protected report when its used outside its package because package protected should have been enough (package only permitted)
+          (c.getMethod().isPublic() or c.getMethod().isProtected()) and
+          not isWithinPackage(c, c.getMethod().getDeclaringType())
+        )
       )
-    )
     or
     // Class instantiation - report if used outside appropriate scope
-    exists(ClassInstanceExpr c |
-      c = e and
-      c.getConstructedType() = annotated and
-      (
-        c.getConstructedType().isPublic() and not isWithinPackage(c, c.getConstructedType())
-        or
-        c.getConstructedType().hasNoModifier() and
-        c.getConstructedType() instanceof NestedClass and
-        not isWithinType(c.getEnclosingCallable(), c.getConstructedType())
+    e =
+      any(ClassInstanceExpr c |
+        c.getConstructedType() = annotated and
+        (
+          c.getConstructedType().isPublic() and not isWithinPackage(c, c.getConstructedType())
+          or
+          c.getConstructedType().hasNoModifier() and
+          c.getConstructedType() instanceof NestedClass and
+          not isWithinType(c.getEnclosingCallable(), c.getConstructedType())
+        )
       )
-    )
   ) and
   // not in a test where use is appropriate
   not e.getEnclosingCallable() instanceof LikelyTestMethod and

From 4705ad2e321d88e0426af210d616bc80b4a7bdbb Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Fri, 22 Aug 2025 07:23:13 +0000
Subject: [PATCH 17/22] Java: Added extra test cases for fields

---
 .../VisibleForTestingAbuse/VisibleForTestingAbuse.expected    | 4 ++++
 .../VisibleForTestingAbuse/packageone/SourcePackage.java      | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 0668105426b4..3fe39f3d60fa 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -6,6 +6,10 @@
 | packageone/SourcePackage.java:26:31:26:42 | Annotated.m2 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:11:26:11:27 | m2 | element |
 | packageone/SourcePackage.java:29:28:29:46 | fPublic(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:26:23:26:29 | fPublic | element |
 | packageone/SourcePackage.java:30:28:30:49 | fProtected(...) | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:31:26:31:35 | fProtected | element |
+| packageone/SourcePackage.java:34:23:34:34 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
+| packageone/SourcePackage.java:35:30:35:41 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
+| packageone/SourcePackage.java:36:31:36:42 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
+| packageone/SourcePackage.java:37:33:37:44 | Annotated.m1 | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:9:29:9:30 | m1 | element |
 | packagetwo/Source.java:8:20:8:30 | Annotated.m | Access of $@ annotated with VisibleForTesting found in production code. | packagetwo/Annotated.java:7:19:7:19 | m | element |
 | 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 |
 | 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 |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
index 96363da79c97..7826f5e1ee0e 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packageone/SourcePackage.java
@@ -31,4 +31,8 @@ void f() {
         };
         lambda.run();
     }
+    String myField1 = Annotated.m1; // $ Alert
+    public String myField2 = Annotated.m1; // $ Alert
+    private String myField3 = Annotated.m1; // $ Alert
+    protected String myField4 = Annotated.m1; // $ Alert
 }

From 4149968f33f8d95fb1f82f2b06df8e360d8b145d Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Sun, 24 Aug 2025 09:58:35 +0000
Subject: [PATCH 18/22] Java: Remove the hardcoded path filter that excluded
 CodeQL's own unit tests from the `java/visible-for-testing-abuse` query.

---
 .../Implementation Hiding/VisibleForTestingAbuse.ql        | 7 +------
 .../VisibleForTestingAbuse/VisibleForTestingAbuse.expected | 2 ++
 .../VisibleForTestingAbuse/packagetwo/Test.java            | 4 ++--
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 0ef3404d9880..078dc39166a5 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -93,11 +93,6 @@ where
   // not when the accessing method or any enclosing method is @VisibleForTesting (test-to-test communication)
   not isWithinVisibleForTestingContext(e.getEnclosingCallable()) and
   // not when used in annotation contexts
-  not e.getParent*() instanceof Annotation and
-  // also omit our own ql unit test where it is acceptable
-  not e.getEnclosingCallable()
-      .getFile()
-      .getAbsolutePath()
-      .matches("%java/ql/test/query-tests/%Test.java")
+  not e.getParent*() instanceof Annotation
 select e, "Access of $@ annotated with VisibleForTesting found in production code.", annotated,
   "element"
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 3fe39f3d60fa..368dcb8c7a58 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -15,3 +15,5 @@
 | 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 |
 | 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 |
 | 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 |
+| 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 |
+| 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 |
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
index b861d921e9a3..ee35416ef878 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
@@ -21,11 +21,11 @@ void f() {
         
         // Lambda usage
         Runnable lambda = () -> {
-            String lambdaS = Annotated.m; // COMPLIANT
+            String lambdaS = Annotated.m; // $ SPURIOUS: Alert
             String lambdaS1 = Annotated.m1; // COMPLIANT
             String lambdaS2 = Annotated.m2; // COMPLIANT
             
-            int lambdaI = Annotated.f(); // COMPLIANT
+            int lambdaI = Annotated.f(); // $ SPURIOUS: Alert
             int lambdaI2 = Annotated.fPublic(); // COMPLIANT
             int lambdaI3 = Annotated.fProtected(); // COMPLIANT
         };

From 38f517ecfaaa3eb1d6b18d0969900aeaddfca420 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Sun, 24 Aug 2025 10:02:43 +0000
Subject: [PATCH 19/22] Java: Add lambda-aware test detection to
 VisibleForTesting query

---
 .../VisibleForTestingAbuse.ql                    | 16 +++++++++++++++-
 .../VisibleForTestingAbuse.expected              |  4 +---
 .../VisibleForTestingAbuse/packagetwo/Test.java  |  4 ++--
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 078dc39166a5..6766aa81e3e1 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -39,6 +39,20 @@ predicate isWithinVisibleForTestingContext(Callable c) {
   isWithinVisibleForTestingContext(c.getEnclosingCallable())
 }
 
+/**
+ * Holds if `e` is within a test method context, including lambda expressions
+ * within test methods and nested lambdas.
+ */
+private predicate isWithinTest(Expr e) {
+  e.getEnclosingCallable() instanceof LikelyTestMethod
+  or
+  exists(Method lambda, LambdaExpr lambdaExpr |
+    lambda = lambdaExpr.asMethod() and
+    lambda.getEnclosingCallable*() instanceof LikelyTestMethod and
+    e.getEnclosingCallable() = lambda
+  )
+}
+
 from Annotatable annotated, Expr e
 where
   annotated.getAnAnnotation().getType().hasName("VisibleForTesting") and
@@ -89,7 +103,7 @@ where
       )
   ) and
   // not in a test where use is appropriate
-  not e.getEnclosingCallable() instanceof LikelyTestMethod and
+  not isWithinTest(e) and
   // not when the accessing method or any enclosing method is @VisibleForTesting (test-to-test communication)
   not isWithinVisibleForTestingContext(e.getEnclosingCallable()) and
   // not when used in annotation contexts
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
index 368dcb8c7a58..61f5b0ffc579 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/VisibleForTestingAbuse.expected
@@ -14,6 +14,4 @@
 | 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 |
 | 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 |
 | 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 |
-| 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 |
-| 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 |
-| 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 |
+| 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 |
\ No newline at end of file
diff --git a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
index ee35416ef878..b861d921e9a3 100644
--- a/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
+++ b/java/ql/test/query-tests/VisibleForTestingAbuse/packagetwo/Test.java
@@ -21,11 +21,11 @@ void f() {
         
         // Lambda usage
         Runnable lambda = () -> {
-            String lambdaS = Annotated.m; // $ SPURIOUS: Alert
+            String lambdaS = Annotated.m; // COMPLIANT
             String lambdaS1 = Annotated.m1; // COMPLIANT
             String lambdaS2 = Annotated.m2; // COMPLIANT
             
-            int lambdaI = Annotated.f(); // $ SPURIOUS: Alert
+            int lambdaI = Annotated.f(); // COMPLIANT
             int lambdaI2 = Annotated.fPublic(); // COMPLIANT
             int lambdaI3 = Annotated.fProtected(); // COMPLIANT
         };

From d3be456c5cfa206642706443a0c8ab9d822aadcb Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Thu, 28 Aug 2025 15:01:43 +0200
Subject: [PATCH 20/22] Update java/ql/src/Violations of Best
 Practice/Implementation Hiding/VisibleForTestingAbuse.ql

Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
---
 .../Implementation Hiding/VisibleForTestingAbuse.ql             | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 6766aa81e3e1..2da8ae4b9626 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -16,7 +16,7 @@ import java
 
 /**
  * Holds if a `Callable` is within the same type hierarchy as `RefType`
- * (including through lambdas, inner classes, and outer classes)
+ * (including through lambdas, inner classes, and outer classes).
  */
 predicate isWithinType(Callable c, RefType t) {
   // Either the callable is in the target type, or they share a common enclosing type

From c836104717a0a10cab8e350693db9d9d38f200bc Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Thu, 28 Aug 2025 15:01:53 +0200
Subject: [PATCH 21/22] Update java/ql/src/Violations of Best
 Practice/Implementation Hiding/VisibleForTestingAbuse.md

Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
---
 .../Implementation Hiding/VisibleForTestingAbuse.md             | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md
index 5b1320d6c852..bcf10901549e 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.md	
@@ -32,5 +32,5 @@ The rule also uses the following logic to determine what an abuse of the annotat
   2. If a package-private member/type is annotated with `@VisibleForTesting`, it's assumed that private access is enough for production code. Therefore the rule alerts when a package-private member/type annotated with `@VisibleForTesting` is used outside its declaring class.
 
 ## References
-- Javadoc: [AssertJ VisibleForTesting](https://javadoc.io/doc/org.assertj/assertj-core/latest/org/assertj/core/util/VisibleForTesting.html).
+- Javadoc: [AssertJ VisibleForTesting](https://javadoc.io/doc/org.assertj/assertj-core/latest/org.assertj.core/org/assertj/core/util/VisibleForTesting.html).
 - Javadoc: [JetBrains VisibleForTesting](https://javadoc.io/doc/org.jetbrains/annotations/22.0.0/org/jetbrains/annotations/VisibleForTesting.html).

From 6132900e12d1d41dbcf885224a0cb20b6c30e7df Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Fri, 29 Aug 2025 08:09:03 +0200
Subject: [PATCH 22/22] Java: add full stops for ql docs

---
 .../Implementation Hiding/VisibleForTestingAbuse.ql           | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql
index 2da8ae4b9626..c882d3f629bb 100644
--- a/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
+++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/VisibleForTestingAbuse.ql	
@@ -24,14 +24,14 @@ predicate isWithinType(Callable c, RefType t) {
 }
 
 /**
- * Holds if `e` is within the same package as `t`
+ * Holds if `e` is within the same package as `t`.
  */
 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
+ * Holds if a callable or any of its enclosing callables is annotated with @VisibleForTesting.
  */
 predicate isWithinVisibleForTestingContext(Callable c) {
   c.getAnAnnotation().getType().hasName("VisibleForTesting")