Java: enchanced check if it is within same package

Author: Napalys

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

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 |

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();
     }

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();
     }