SONARJAVA-5482 stop raising issues if variable or identifier symbol is unknown (#5141)

erwan-serandour · web-flow · commit f239443c7ba9 · 2025-05-14T09:20:28.000+02:00
diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S1854.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S1854.json
@@ -9,14 +9,6 @@
 "org.eclipse.jetty:jetty-project:jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/WebSocketUriMappingTest.java": [
 91
 ],
-"org.eclipse.jetty:jetty-project:jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java": [
-92,
-94,
-97,
-105,
-147,
-272
-],
 "org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java": [
 861
 ],
diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S1854.json b/its/ruling/src/test/resources/eclipse-jetty/java-S1854.json
@@ -9,14 +9,6 @@
 "org.eclipse.jetty:jetty-project:jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/WebSocketUriMappingTest.java": [
 91
 ],
-"org.eclipse.jetty:jetty-project:jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java": [
-92,
-94,
-97,
-105,
-147,
-272
-],
 "org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java": [
 861
 ],
diff --git a/java-checks-test-sources/default/src/main/java/checks/DeadStoreCheckBrokenSemantic.java b/java-checks-test-sources/default/src/main/java/checks/DeadStoreCheckBrokenSemantic.java
@@ -0,0 +1,9 @@
+package checks;
+
+public class DeadStoreCheckBrokenSemantic {
+
+  public void singleAssignment() {
+    int a = 42;
+    System.out.println(a);
+  }
+}
diff --git a/java-checks-test-sources/default/src/main/java/checks/DeadStoreCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/DeadStoreCheckSample.java
@@ -10,6 +10,10 @@ class DeadStoreCheckSample {
 
   int var;
 
+  void singleAssignment(){
+    int z = 10; // Noncompliant
+  }
+
   int foo(int u) {
     int x = 0;// Compliant - default value
     x = 3; // Noncompliant
diff --git a/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/CheckVerifier.java b/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/CheckVerifier.java
@@ -18,13 +18,15 @@
 
 import java.io.File;
 import java.util.Collection;
+import java.util.function.Consumer;
 import javax.annotation.Nullable;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.sensor.cache.ReadCache;
 import org.sonar.api.batch.sensor.cache.WriteCache;
 import org.sonar.java.checks.verifier.internal.InternalCheckVerifier;
 import org.sonar.java.checks.verifier.internal.JavaCheckVerifier;
 import org.sonar.plugins.java.api.JavaFileScanner;
+import org.sonar.plugins.java.api.tree.CompilationUnitTree;
 
 /**
  * This interface defines how to use checks (rules) verifiers. Its goal is to provide all the required information
@@ -235,7 +237,21 @@ static CheckVerifier newInternalVerifier() {
    * @param rootDirectory The path of the project root working directory
    * @return the verifier configured with the project root working directory.
    */
-  CheckVerifier withProjectLevelWorkDir(String rootDirectory);
+  default CheckVerifier withProjectLevelWorkDir(String rootDirectory) {
+    throw new UnsupportedOperationException("Method not implemented, feel free to implement.");
+  }
+
+  /**
+   * Allows to modify the compilation unit tree after parsing.
+   * This is useful for adding or modifying nodes in the tree for testing purposes.
+   * There is at most one modifier per {@link CheckVerifier}.
+   *
+   * @param compilationUnitModifier the modifier to apply to the compilation unit tree
+   * @return the verifier configured with the compilation unit modifier.
+   */
+  default CheckVerifier withCompilationUnitModifier(Consumer<CompilationUnitTree> compilationUnitModifier) {
+    throw new UnsupportedOperationException("Method not implemented, feel free to implement.");
+  }
 
   /**
    * Verifies that all the expected issues are correctly raised by the rule(s),
diff --git a/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifier.java b/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifier.java
@@ -248,11 +248,6 @@ public CheckVerifier withCache(@Nullable ReadCache readCache, @Nullable WriteCac
     return this;
   }
 
-  @Override
-  public CheckVerifier withProjectLevelWorkDir(String rootDirectory) {
-    throw new RuntimeException("Method not implemented, feel free to implement.");
-  }
-
   @Override
   public void verifyIssues() {
     requiresNonNull(checks, CHECK_OR_CHECKS);
diff --git a/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifier.java b/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifier.java
@@ -28,6 +28,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Consumer;
 import javax.annotation.Nullable;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.sensor.cache.ReadCache;
@@ -50,6 +51,7 @@
 import org.sonar.plugins.java.api.JavaFileScanner;
 import org.sonar.plugins.java.api.JavaVersion;
 import org.sonar.plugins.java.api.caching.CacheContext;
+import org.sonar.plugins.java.api.tree.CompilationUnitTree;
 import org.sonarsource.analyzer.commons.checks.verifier.MultiFileVerifier;
 import org.sonarsource.analyzer.commons.checks.verifier.quickfix.QuickFix;
 import org.sonarsource.analyzer.commons.checks.verifier.quickfix.TextEdit;
@@ -90,6 +92,7 @@ public static JavaCheckVerifier newInstance() {
   private List<InputFile> files = null;
   private boolean withoutSemantic = false;
   private boolean isCacheEnabled = false;
+  private Consumer<CompilationUnitTree> compilationUnitModifier = unused -> {};
 
   @VisibleForTesting
   CacheContext cacheContext = null;
@@ -124,7 +127,7 @@ private MultiFileVerifier createVerifier() {
       visitorsBridge.setCacheContext(cacheContext);
       filesToParse = astScanner.scanWithoutParsing(files).get(false);
     }
-    astScanner.scan(filesToParse);
+    astScanner.scanForTesting(filesToParse, compilationUnitModifier);
 
     addComments(verifier, commentLinesVisitor);
 
@@ -335,6 +338,12 @@ public CheckVerifier withProjectLevelWorkDir(String rootDirectory) {
     return this;
   }
 
+  @Override
+  public CheckVerifier withCompilationUnitModifier(Consumer<CompilationUnitTree> compilationUnitModifier) {
+    this.compilationUnitModifier = compilationUnitModifier;
+    return this;
+  }
+
   @Override
   public void verifyIssues() {
     requiresNonNull(checks, CHECK_OR_CHECKS);
diff --git a/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifierTest.java b/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifierTest.java
@@ -524,7 +524,7 @@ void context_root_working_directory_not_supported() {
     });
 
     assertThat(e)
-      .isInstanceOf(RuntimeException.class)
+      .isInstanceOf(UnsupportedOperationException.class)
       .hasMessage("Method not implemented, feel free to implement.");
   }
 
@@ -1053,4 +1053,21 @@ void withCache_can_handle_a_mix_of_caches_combination() {
     assertThat(dummyReadInternalWrite.cacheContext.getWriteCache()).isInstanceOf(JavaWriteCache.class);
   }
 
+  @Test
+  void compilationUnitModifier_not_supported() {
+
+    InternalCheckVerifier checkVerifier = InternalCheckVerifier.newInstance()
+      .onFile(TEST_FILE);
+
+    Throwable e = catchThrowable(() -> {
+      checkVerifier
+        .withCompilationUnitModifier(tree -> {
+        });
+    });
+
+    assertThat(e)
+      .isInstanceOf(UnsupportedOperationException.class)
+      .hasMessage("Method not implemented, feel free to implement.");
+  }
+
 }
diff --git a/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java b/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java
@@ -20,6 +20,8 @@
 import java.io.IOException;
 import java.nio.file.Path;
 import java.security.NoSuchAlgorithmException;
+import java.util.List;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 import org.junit.jupiter.api.Test;
 import org.sonar.api.batch.fs.InputFile;
@@ -33,7 +35,11 @@
 import org.sonar.java.checks.verifier.CheckVerifier;
 import org.sonar.java.checks.verifier.internal.CheckVerifierTestUtils.IssueWithQuickFix;
 import org.sonar.java.checks.verifier.internal.CheckVerifierTestUtils.NoEffectEndOfAnalysisCheck;
+import org.sonar.java.model.InternalSyntaxToken;
 import org.sonar.java.model.JavaVersionImpl;
+import org.sonar.java.model.declaration.ClassTreeImpl;
+import org.sonar.java.model.declaration.ModifiersTreeImpl;
+import org.sonar.java.model.expression.IdentifierTreeImpl;
 import org.sonar.java.reporting.AnalyzerMessage;
 import org.sonar.java.reporting.JavaQuickFix;
 import org.sonar.java.reporting.JavaTextEdit;
@@ -42,6 +48,8 @@
 import org.sonar.plugins.java.api.caching.CacheContext;
 import org.sonar.plugins.java.api.caching.JavaReadCache;
 import org.sonar.plugins.java.api.caching.JavaWriteCache;
+import org.sonar.plugins.java.api.tree.CompilationUnitTree;
+import org.sonar.plugins.java.api.tree.IdentifierTree;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
@@ -307,4 +315,28 @@ void withCache_can_handle_a_mix_of_caches_combination() {
     assertThat(dummyReadInternalWrite.cacheContext.getWriteCache()).isInstanceOf(JavaWriteCache.class);
   }
 
+  @Test
+  void compilationUnitModifier_modify_tree() {
+    Consumer<CompilationUnitTree> modifier = tree -> {
+      ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0);
+      IdentifierTree ident = new IdentifierTreeImpl(new InternalSyntaxToken(1, 1, "Modified", List.of(), false));
+      classTree.complete((ModifiersTreeImpl)classTree.modifiers(), classTree.declarationKeyword(), ident);
+    };
+
+    var check = new JavaFileScanner() {
+      @Override
+      public void scanFile(JavaFileScannerContext context) {
+        CompilationUnitTree tree = context.getTree();
+        ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0);
+        assertThat(classTree.simpleName().name()).isEqualTo("Modified");
+      }
+    };
+    
+    JavaCheckVerifier.newInstance()
+      .onFile(TEST_FILE)
+      .withCheck(check)
+      .withCompilationUnitModifier(modifier)
+      .verifyNoIssues();
+  }
+
 }
diff --git a/java-checks/src/main/java/org/sonar/java/checks/DeadStoreCheck.java b/java-checks/src/main/java/org/sonar/java/checks/DeadStoreCheck.java
@@ -181,6 +181,7 @@ private void handleVariable(Set<Symbol> out, VariableTree localVar) {
     Symbol symbol = localVar.symbol();
     ExpressionTree initializer = localVar.initializer();
     if (initializer != null
+      && !localVar.symbol().isUnknown()
       && !isUsualDefaultValue(initializer)
       && !out.contains(symbol)
       && !UNRESOLVED_IDENTIFIERS_VISITOR.isUnresolved(symbol.name())) {
diff --git a/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/DeadStoreCheckTest.java
@@ -19,6 +19,9 @@
 import org.junit.jupiter.api.Test;
 import org.sonar.java.checks.verifier.CheckVerifier;
 import org.sonar.java.checks.verifier.TestUtils;
+import org.sonar.java.model.declaration.VariableTreeImpl;
+import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
+import org.sonar.plugins.java.api.tree.VariableTree;
 
 class DeadStoreCheckTest {
 
@@ -30,6 +33,16 @@ void test() {
       .verifyIssues();
   }
 
+  @Test
+  void test_variable_identifier_with_unknown_symbol() {
+    EraseSymbols eraser = new EraseSymbols();
+    CheckVerifier.newVerifier()
+      .onFile(TestUtils.mainCodeSourcesPath("checks/DeadStoreCheckBrokenSemantic.java"))
+      .withCheck(new DeadStoreCheck())
+      .withCompilationUnitModifier(eraser::visitCompilationUnit)
+      .verifyNoIssues();
+  }
+
   @Test
   void test_non_compiling() {
     CheckVerifier.newVerifier()
@@ -38,4 +51,14 @@ void test_non_compiling() {
       .verifyIssues();
   }
 
+  private static class EraseSymbols extends BaseTreeVisitor {
+
+    @Override
+    public void visitVariable(VariableTree tree) {
+      if (tree instanceof VariableTreeImpl varImpl) {
+        varImpl.variableBinding = null;
+      }
+      super.visitVariable(tree);
+    }
+  }
 }
diff --git a/java-frontend/src/main/java/org/sonar/java/ast/JavaAstScanner.java b/java-frontend/src/main/java/org/sonar/java/ast/JavaAstScanner.java
@@ -42,6 +42,7 @@
 import org.sonar.java.model.JavaTree;
 import org.sonar.java.model.VisitorsBridge;
 import org.sonar.plugins.java.api.JavaVersion;
+import org.sonar.plugins.java.api.tree.CompilationUnitTree;
 
 public class JavaAstScanner {
   private static final Logger LOG = LoggerFactory.getLogger(JavaAstScanner.class);
@@ -78,6 +79,22 @@ public Map<Boolean, List<InputFile>> scanWithoutParsing(Iterable<? extends Input
   }
 
   public void scan(Iterable<? extends InputFile> inputFiles) {
+    scan(inputFiles, compilationUnitTree -> {});
+  }
+
+  /**
+   * Scan the given files and modify
+   *
+   * @param inputFiles The list of files to analyze
+   * @param modifyCompilationUnit allow you to modify the ast before running the analysis on it, for example to remove semantic information
+   */
+  @VisibleForTesting
+  public void scanForTesting(Iterable<? extends InputFile> inputFiles, Consumer<CompilationUnitTree> modifyCompilationUnit) {
+    scan(inputFiles, modifyCompilationUnit);
+  }
+
+
+  private void scan(Iterable<? extends InputFile> inputFiles, Consumer<CompilationUnitTree> modifyCompilationUnit) {
     List<? extends InputFile> filesNames = filterModuleInfo(inputFiles).toList();
     AnalysisProgress analysisProgress = new AnalysisProgress(filesNames.size());
     try {
@@ -90,7 +107,8 @@ public void scan(Iterable<? extends InputFile> inputFiles) {
           analysisProgress,
           (i, r) -> simpleScan(i, r,
             // Due to a bug in ECJ, JAR files remain locked after the analysis on Windows, we unlock them manually. See SONARJAVA-3609.
-            JavaAstScanner::cleanUpAst));
+            JavaAstScanner::cleanUpAst,
+            modifyCompilationUnit));
     } finally {
       endOfAnalysis();
     }
@@ -125,9 +143,16 @@ private boolean analysisCancelled() {
   }
 
   public void simpleScan(InputFile inputFile, JParserConfig.Result result, Consumer<JavaTree.CompilationUnitTreeImpl> cleanUp) {
+    simpleScan(inputFile, result, cleanUp, compilationUnitTree -> {});
+  }
+
+  // modifyCompilationUnit should be used for testing.
+  public void simpleScan(InputFile inputFile, JParserConfig.Result result, Consumer<JavaTree.CompilationUnitTreeImpl> cleanUp,
+    Consumer<CompilationUnitTree> modifyCompilationUnit) {
     visitor.setCurrentFile(inputFile);
     try {
       JavaTree.CompilationUnitTreeImpl ast = result.get();
+      modifyCompilationUnit.accept(ast);
       visitor.visitFile(ast, sonarComponents != null && sonarComponents.fileCanBeSkipped(inputFile));
       String path = inputFile.toString();
       collectUndefinedTypes(path, ast.sema.undefinedTypes());
diff --git a/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java b/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java
@@ -53,13 +53,15 @@
 import org.sonar.java.model.VisitorsBridge;
 import org.sonar.java.model.declaration.ClassTreeImpl;
 import org.sonar.java.model.declaration.MethodTreeImpl;
+import org.sonar.java.model.expression.IdentifierTreeImpl;
 import org.sonar.java.model.statement.BlockTreeImpl;
 import org.sonar.java.notchecks.VisitorNotInChecksPackage;
 import org.sonar.java.testing.ThreadLocalLogTester;
 import org.sonar.plugins.java.api.JavaFileScanner;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
 import org.sonar.plugins.java.api.ModuleScannerContext;
 import org.sonar.plugins.java.api.internal.EndOfAnalysis;
+import org.sonar.plugins.java.api.tree.CompilationUnitTree;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.fail;
@@ -447,6 +449,36 @@ void scanWithoutParsing_filters_out_the_files_that_could_be_successfully_scanned
     assertThat(actual.get(true)).containsExactly(successful, successful);
   }
 
+  @Test
+  void test_modifyCompilationUnit_modify_ast() {
+    InputFile trivialCompilationUnit = TestUtils.inputFile("src/test/resources/AstScannerNoParseError.txt");
+
+    var check = new JavaFileScanner() {
+      @Override
+      public void scanFile(JavaFileScannerContext context) {
+        CompilationUnitTree tree = context.getTree();
+        ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0);
+        assertThat(classTree.simpleName().symbol().isUnknown()).isTrue();
+      }
+    };
+
+    VisitorsBridge visitorsBridge = new VisitorsBridge(
+      List.of(check),
+      Collections.emptyList(),
+      null);
+
+    JavaAstScanner scanner = new JavaAstScanner(null);
+    scanner.setVisitorBridge(visitorsBridge);
+    scanner.scanForTesting(Collections.singletonList(trivialCompilationUnit), compilationUnit -> {
+      var clazz = (ClassTreeImpl) ((JavaTree.CompilationUnitTreeImpl) compilationUnit).types().get(0);
+      assertThat(clazz.simpleName().name()).isEqualTo("C");
+      assertThat(clazz.simpleName().symbol().isUnknown()).isFalse();
+      IdentifierTreeImpl identifierTree = (IdentifierTreeImpl) clazz.simpleName();
+      identifierTree.binding = null;
+    });
+
+  }
+
   private void scanSingleFile(InputFile file, boolean failOnException) {
     scanFilesWithVisitors(Collections.singletonList(file), Collections.emptyList(), -1, failOnException, false);
   }
diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/SECheckVerifier.java b/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/SECheckVerifier.java

Original file line number	Diff line number	Diff line change
`@@ -9,14 +9,6 @@`
`9`	`9`	`"org.eclipse.jetty:jetty-project:jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/WebSocketUriMappingTest.java": [`
`10`	`10`	`91`
`11`	`11`	`],`
`12`		`-"org.eclipse.jetty:jetty-project:jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java": [`
`13`		`-92,`
`14`		`-94,`
`15`		`-97,`
`16`		`-105,`
`17`		`-147,`
`18`		`-272`
`19`		`-],`
`20`	`12`	`"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java": [`
`21`	`13`	`861`
`22`	`14`	`],`