SONARJAVA-4698 Fix false positive in S1191 when variable is named "sun"

java-checks-test-sources/default/src/main/files/non-compiling/checks/SunPackagesUsedCheckSample.java

@@ -1,13 +1,15 @@
 import java.util.ArrayList;
 
 class SunPackagesUsedCheckSample {
+  private Object sun; // variable named "sun"
+
   private void f() {
     java.util.List a;
     sun.Foo b; // Noncompliant
 //  ^^^^^^^
     sun.Foo.toto.asd c; // secondary
 //  ^^^^^^^^^^^^^^^^<
-    
+
   }
 
   public Object uselessMethod() {
@@ -16,4 +18,18 @@ public Object uselessMethod() {
     }
     return null;
   }
+
+  // SONARJAVA-4698: False positive when variable is named "sun"
+  public void fooWithFieldNamedSun() {
+    sun.toString(); // Compliant - "sun" is a field of type Object, not a sun.* package class
+  }
+
+  public void barWithParameterNamedSun(Object sun) {
+    sun.toString(); // Compliant - "sun" is a parameter of type Object, not a sun.* package class
+  }
+
+  public void bazWithLocalVariableNamedSun() {
+    Object sun = new Object();
+    sun.toString(); // Compliant - "sun" is a local variable of type Object, not a sun.* package class
+  }
 }

java-checks-test-sources/default/src/main/java/checks/SunPackagesUsedCheckSample.java

@@ -0,0 +1,25 @@
+package checks;
+
+class SunPackagesUsedCheckSample {
+  private Object sun; // variable named "sun"
+
+  // SONARJAVA-4698: Variables named "sun" should not trigger the rule
+  public void fooWithFieldNamedSun() {
+    sun.toString(); // Compliant - "sun" is a field of type Object, not a sun.* package class
+  }
+
+  public void barWithParameterNamedSun(Object sun) {
+    sun.toString(); // Compliant - "sun" is a parameter of type Object, not a sun.* package class
+  }
+
+  public void bazWithLocalVariableNamedSun() {
+    Object sun = new Object();
+    sun.toString(); // Compliant - "sun" is a local variable of type Object, not a sun.* package class
+  }
+
+  // Actual sun.* package usage - should trigger the rule
+  public void useSunMiscUnsafe() {
+    sun.misc.Unsafe unsafe = null; // Noncompliant {{Use classes from the Java API instead of Sun classes.}}
+//  ^^^^^^^^^^^^^^^
+  }
+}

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

@@ -66,7 +66,7 @@ private void reportIssueWithSecondaries(JavaFileScannerContext context) {
   @Override
   public void visitMemberSelectExpression(MemberSelectExpressionTree tree) {
     String reference = ExpressionsHelper.concatenate(tree);
-    if (!isExcluded(reference) && isSunClass(reference)) {
+    if (!isExcluded(reference) && isSunClass(reference) && isActuallySunPackage(tree)) {
       reportedTrees.add(tree);
     }
   }
@@ -75,6 +75,21 @@ private static boolean isSunClass(String reference) {
     return reference.startsWith("sun.");
   }
 
+  private static boolean isActuallySunPackage(MemberSelectExpressionTree tree) {
+    // Check if the expression's type actually comes from a sun.* package
+    // This prevents false positives when a variable is named "sun"
+    var type = tree.expression().symbolType();
+    if (!type.isUnknown()) {
+      // When we have type information, use it to determine if it's actually a sun.* package
+      String fullyQualifiedName = type.fullyQualifiedName();
+      return fullyQualifiedName.startsWith("sun.");
+    }
+    // When type is unknown (non-compiling code), we can't use semantic analysis
+    // In this case, rely on the string-based check which has already been done
+    // This means we may miss some FPs in non-compiling code, but we'll catch them with proper semantic analysis
+    return true;
+  }
+
   private boolean isExcluded(String reference) {
     for (String str : excludePackages) {
       if (!str.isEmpty() && reference.startsWith(str)) {

java-checks/src/test/java/org/sonar/java/checks/SunPackagesUsedCheckTest.java

@@ -40,4 +40,12 @@ void check_with_exclusion() {
       .verifyIssues();
   }
 
+  @Test
+  void detected_with_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(TestUtils.mainCodeSourcesPath("checks/SunPackagesUsedCheckSample.java"))
+      .withCheck(new SunPackagesUsedCheck())
+      .verifyIssues();
+  }
+
 }