SONARJAVA-4093 don't raise issues in S3252 when class owning static field is not accessible from the current package (#4948)

Author: erwan-serandour

…compiling/checks/StaticMemberAccess.java …hecks/StaticMemberAccessCheckSample.javajava-checks-test-sources/default/src/main/files/non-compiling/checks/StaticMemberAccess.java renamed to java-checks-test-sources/default/src/main/files/non-compiling/checks/StaticMemberAccessCheckSample.java java-checks-test-sources/default/src/main/files/non-compiling/checks/StaticMemberAccess.java renamed to java-checks-test-sources/default/src/main/files/non-compiling/checks/StaticMemberAccessCheckSample.java

@@ -0,0 +1,14 @@
+public class ClassInDefaultPackage {
+
+  static class A {
+    public static final int CONSTANT = 42;
+
+  }
+
+  static class B extends A {
+
+    public static void foo()  {
+      int x = B.CONSTANT; // Noncompliant
+    }
+  }
+}

java-checks-test-sources/default/src/main/java/checks/S3252_StaticMemberAccessCheckSample/ClassInDefaultPackage.java

@@ -0,0 +1,10 @@
+package checks.S3252_StaticMemberAccessCheckSample;
+
+import checks.S3252_StaticMemberAccessCheckSample.hide_non_public.StaticMemberAccessCheckSampleHelper;
+
+public class ImportFromOtherPackage {
+  public void foo(){
+    int x = StaticMemberAccessCheckSampleHelper.B.CONSTANT; // Compliant A is not accessible so we should not raise an issue
+    int y = StaticMemberAccessCheckSampleHelper.Bar.CONSTANT; // Noncompliant
+  }
+}

java-checks-test-sources/default/src/main/java/checks/S3252_StaticMemberAccessCheckSample/ImportFromOtherPackage.java

@@ -1,4 +1,4 @@
-package checks;
+package checks.S3252_StaticMemberAccessCheckSample;
 
 import com.google.common.collect.ImmutableSet;
 import java.util.Set;
@@ -14,7 +14,7 @@ static void foo() {
 
 class StaticMemberAccessChild extends StaticMemberAccessParent {
   public StaticMemberAccessChild() {
-    StaticMemberAccessChild.counter++; // Noncompliant {{Use static access with "checks.StaticMemberAccessParent" for "counter".}}
+    StaticMemberAccessChild.counter++; // Noncompliant {{Use static access with "checks.S3252_StaticMemberAccessCheckSample.StaticMemberAccessParent" for "counter".}}
     StaticMemberAccessParent.counter++; // Compliant
 
     StaticMemberAccessChild.foo(); // Noncompliant
@@ -47,5 +47,4 @@ class GuavaFP {
     "sun.rmi.transport.misc",
     "sun.rmi.server.call",
     "sun.rmi.dgc");
-  
 }

…main/java/checks/StaticMemberAccess.java …ample/StaticMemberAccessCheckSample.javajava-checks-test-sources/default/src/main/java/checks/StaticMemberAccess.java renamed to java-checks-test-sources/default/src/main/java/checks/S3252_StaticMemberAccessCheckSample/StaticMemberAccessCheckSample.java java-checks-test-sources/default/src/main/java/checks/StaticMemberAccess.java renamed to java-checks-test-sources/default/src/main/java/checks/S3252_StaticMemberAccessCheckSample/StaticMemberAccessCheckSample.java

@@ -0,0 +1,20 @@
+package checks.S3252_StaticMemberAccessCheckSample.hide_non_public;
+
+public class StaticMemberAccessCheckSampleHelper {
+
+  static class A {
+    public static final int CONSTANT = 42;
+  }
+
+  static public class B  extends A {
+  }
+
+  static public class Foo {
+    public static final int CONSTANT = 42;
+  }
+
+  static public class Bar  extends Foo {
+  }
+
+
+}

java-checks-test-sources/default/src/main/java/checks/S3252_StaticMemberAccessCheckSample/hide_non_public/StaticMemberAccessCheckSampleHelper.java

@@ -16,19 +16,21 @@
  */
 package org.sonar.java.checks;
 
-import java.util.Collections;
 import java.util.List;
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.model.PackageUtils;
 import org.sonar.java.reporting.JavaQuickFix;
 import org.sonar.java.reporting.JavaTextEdit;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.semantic.MethodMatchers;
 import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.CompilationUnitTree;
 import org.sonar.plugins.java.api.tree.ExpressionTree;
 import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.PackageDeclarationTree;
 import org.sonar.plugins.java.api.tree.Tree;
 
 @Rule(key = "S3252")
@@ -40,22 +42,36 @@ public class StaticMemberAccessCheck extends IssuableSubscriptionVisitor {
     .withAnyParameters()
     .build();
 
+  private String currentPackage = "";
+
   @Override
   public List<Tree.Kind> nodesToVisit() {
-    return Collections.singletonList(Tree.Kind.MEMBER_SELECT);
+    return List.of(Tree.Kind.MEMBER_SELECT, Tree.Kind.COMPILATION_UNIT);
   }
 
 
   @Override
   public void visitNode(Tree tree) {
+    if (tree instanceof CompilationUnitTree file) {
+      PackageDeclarationTree packageDecl = file.packageDeclaration();
+      currentPackage = packageDecl == null ? "" :  PackageUtils.packageName(packageDecl, ".");
+      return;
+    }
+
     MemberSelectExpressionTree mse = (MemberSelectExpressionTree) tree;
     Symbol symbol = mse.identifier().symbol();
-    if (symbol.isStatic()  && !isListOrSetOf(mse)) {
+    if (symbol.isStatic() && !isListOrSetOf(mse)) {
       ExpressionTree expression = mse.expression();
-      Type staticType = symbol.owner().type();
+
+      Symbol owner = symbol.owner();
+      Type staticType = owner.type();
       Type expressionType = expression.symbolType();
+      boolean memberOwnerIsInCurrentPackage = currentPackage.equals(classPackage(expressionType));
+
       if (!staticType.isUnknown() && !expressionType.isUnknown()
-        && !expressionType.erasure().equals(staticType.erasure())) {
+        && !expressionType.erasure().equals(staticType.erasure())
+        && (memberOwnerIsInCurrentPackage || owner.isPublic())
+      ) {
         QuickFixHelper.newIssue(context)
           .forRule(this)
           .onTree(mse.identifier())
@@ -66,6 +82,11 @@ public void visitNode(Tree tree) {
     }
   }
 
+  private static String classPackage(Type classType) {
+    int endPackage = classType.fullyQualifiedName().lastIndexOf('.');
+    return endPackage == -1 ? "" : classType.fullyQualifiedName().substring(0, endPackage);
+  }
+
   private static boolean isListOrSetOf(MemberSelectExpressionTree mse) {
     // this is necessary because we incorrectly resolve to Set#of List#of methods on JDK11
     // see SONARJAVA-3095

java-checks/src/main/java/org/sonar/java/checks/StaticMemberAccessCheck.java

@@ -23,13 +23,37 @@
 import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath;
 
 class StaticMemberAccessCheckTest {
-
-  private static final String FILE_NAME = "checks/StaticMemberAccess.java";
+  private static final String COMPILING_SAMPLE_FOLDER = "checks/S3252_StaticMemberAccessCheckSample/";
 
   @Test
   void test() {
     CheckVerifier.newVerifier()
-      .onFile(mainCodeSourcesPath(FILE_NAME))
+      .onFile(mainCodeSourcesPath(COMPILING_SAMPLE_FOLDER + "StaticMemberAccessCheckSample.java"))
+      .withCheck(new StaticMemberAccessCheck())
+      .verifyIssues();
+  }
+
+  @Test
+  void test_import_from_other_package() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath(COMPILING_SAMPLE_FOLDER + "ImportFromOtherPackage.java"))
+      .withCheck(new StaticMemberAccessCheck())
+      .verifyIssues();
+  }
+
+  @Test
+  void test_import_from_other_package_without_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath(COMPILING_SAMPLE_FOLDER + "ImportFromOtherPackage.java"))
+      .withoutSemantic()
+      .withCheck(new StaticMemberAccessCheck())
+      .verifyNoIssues();
+  }
+
+  @Test
+  void test_when_class_is_in_default_package() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath(COMPILING_SAMPLE_FOLDER + "ClassInDefaultPackage.java"))
       .withCheck(new StaticMemberAccessCheck())
       .verifyIssues();
   }
@@ -45,7 +69,7 @@ void quick_fixes() {
   @Test
   void test_non_compiling() {
     CheckVerifier.newVerifier()
-      .onFile(nonCompilingTestSourcesPath(FILE_NAME))
+      .onFile(nonCompilingTestSourcesPath("checks/StaticMemberAccessCheckSample.java"))
       .withCheck(new StaticMemberAccessCheck())
       .verifyIssues();
   }