SONARJAVA-5670 Make SonarComponents in JavaFrontend not @nullable. (#5234)

Null SonarComponents is used only tests. JavaFrontend is created in JavaSensor, which calls a method on SonarComponents before it is passed the the constructor, therefore the parameter can never be null in production.

Replacing the nulls in tests with mocks will simplify the production code.

Author: tomasz-tylenda-sonarsource

java-frontend/src/main/java/org/sonar/java/JavaFrontend.java

@@ -31,7 +31,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.batch.fs.InputFile;
-import org.sonar.java.annotations.VisibleForTesting;
 import org.sonar.java.ast.JavaAstScanner;
 import org.sonar.java.ast.visitors.FileLinesVisitor;
 import org.sonar.java.ast.visitors.SyntaxHighlighterVisitor;
@@ -59,7 +58,7 @@ public class JavaFrontend {
   private final JavaAstScanner astScannerForTests;
   private final JavaAstScanner astScannerForGeneratedFiles;
 
-  public JavaFrontend(JavaVersion javaVersion, @Nullable SonarComponents sonarComponents, Measurer measurer,
+  public JavaFrontend(JavaVersion javaVersion, SonarComponents sonarComponents, Measurer measurer,
                       JavaResourceLocator javaResourceLocator, @Nullable SonarJavaIssueFilter postAnalysisIssueFilter, JavaCheck... visitors) {
     this.javaVersion = javaVersion;
     this.sonarComponents = sonarComponents;
@@ -77,25 +76,17 @@ public JavaFrontend(JavaVersion javaVersion, @Nullable SonarComponents sonarComp
     List<JavaCheck> testCodeVisitors = new ArrayList<>(commonVisitors);
     testCodeVisitors.add(measurer.new TestFileMeasurer());
 
-    List<File> classpath = new ArrayList<>();
-    List<File> testClasspath = new ArrayList<>();
-    List<JavaCheck> jspCodeVisitors = new ArrayList<>();
-    List<File> jspClasspath = new ArrayList<>();
-    boolean inAndroidContext = false;
-
-    if (sonarComponents != null) {
-      if (!sonarComponents.isSonarLintContext()) {
-        codeVisitors.add(new FileLinesVisitor(sonarComponents));
-        codeVisitors.add(new SyntaxHighlighterVisitor(sonarComponents));
-        testCodeVisitors.add(new SyntaxHighlighterVisitor(sonarComponents));
-      }
-      classpath = sonarComponents.getJavaClasspath();
-      testClasspath = sonarComponents.getJavaTestClasspath();
-      jspClasspath = sonarComponents.getJspClasspath();
-      testCodeVisitors.addAll(sonarComponents.testChecks());
-      jspCodeVisitors = sonarComponents.jspChecks();
-      inAndroidContext = sonarComponents.inAndroidContext();
+    if (!sonarComponents.isSonarLintContext()) {
+      codeVisitors.add(new FileLinesVisitor(sonarComponents));
+      codeVisitors.add(new SyntaxHighlighterVisitor(sonarComponents));
+      testCodeVisitors.add(new SyntaxHighlighterVisitor(sonarComponents));
     }
+    List<File> classpath = sonarComponents.getJavaClasspath();
+    List<File> testClasspath = sonarComponents.getJavaTestClasspath();
+    List<File> jspClasspath = sonarComponents.getJspClasspath();
+    testCodeVisitors.addAll(sonarComponents.testChecks());
+    List<JavaCheck> jspCodeVisitors = sonarComponents.jspChecks();
+    boolean inAndroidContext = sonarComponents.inAndroidContext();
 
     globalClasspath = Stream.of(classpath, testClasspath, jspClasspath)
       .flatMap(Collection::stream).distinct().toList();
@@ -113,11 +104,6 @@ public JavaFrontend(JavaVersion javaVersion, @Nullable SonarComponents sonarComp
     astScannerForGeneratedFiles.setVisitorBridge(new VisitorsBridge(jspCodeVisitors, jspClasspath, sonarComponents, javaVersion, inAndroidContext));
   }
 
-  @VisibleForTesting
-  boolean analysisCancelled() {
-    return sonarComponents != null && sonarComponents.analysisCancelled();
-  }
-
   public void scan(Iterable<InputFile> sourceFiles, Iterable<InputFile> testFiles, Iterable<? extends InputFile> generatedFiles) {
     if (canOptimizeScanning()) {
       long successfullyScanned = 0L;
@@ -147,13 +133,12 @@ public void scan(Iterable<InputFile> sourceFiles, Iterable<InputFile> testFiles,
     }
 
     // SonarLint is not compatible with batch mode, it needs InputFile#contents() and batch mode use InputFile#absolutePath()
-    boolean isSonarLint = sonarComponents != null && sonarComponents.isSonarLintContext();
-    boolean fileByFileMode = isSonarLint || isFileByFileEnabled();
+    boolean fileByFileMode = sonarComponents.isSonarLintContext() || sonarComponents.isFileByFileEnabled();
     if (fileByFileMode) {
       scanAndMeasureTask(sourceFiles, astScanner::scan, "Main");
       scanAndMeasureTask(testFiles, astScannerForTests::scan, "Test");
       scanAndMeasureTask(generatedFiles, astScannerForGeneratedFiles::scan, "Generated");
-    } else if (isAutoScan()) {
+    } else if (sonarComponents.isAutoScan()) {
       scanAsBatch(new AutoScanBatchContext(), sourceFiles, testFiles);
     } else {
       scanAsBatch(new DefaultBatchModeContext(astScanner, "Main"), sourceFiles);
@@ -204,7 +189,7 @@ private void scanAsBatch(BatchModeContext context, Iterable<? extends InputFile>
   private void scanInBatches(BatchModeContext context, List<InputFile> allInputFiles) {
     String logUsingBatch = String.format("Using ECJ batch to parse %d %s java source files", allInputFiles.size(), context.descriptor());
     AnalysisProgress analysisProgress = new AnalysisProgress(allInputFiles.size());
-    long batchModeSizeInKB = getBatchModeSizeInKB();
+    long batchModeSizeInKB = sonarComponents.getBatchModeSizeInKB();
     if (batchModeSizeInKB < 0L || batchModeSizeInKB >= Long.MAX_VALUE / 1_000L) {
       LOG.info("{} in a single batch.", logUsingBatch);
       scanBatch(context, allInputFiles, analysisProgress);
@@ -222,10 +207,9 @@ private void scanInBatches(BatchModeContext context, List<InputFile> allInputFil
   private <T extends InputFile> void scanBatch(BatchModeContext context, List<T> batchFiles, AnalysisProgress analysisProgress) {
     analysisProgress.startBatch(batchFiles.size());
     Set<Runnable> environmentsCleaners = new HashSet<>();
-    boolean shouldIgnoreUnnamedModuleForSplitPackage = sonarComponents!= null && sonarComponents.shouldIgnoreUnnamedModuleForSplitPackage();
     JParserConfig.Mode.BATCH
-      .create(javaVersion, context.getClasspath(), shouldIgnoreUnnamedModuleForSplitPackage)
-      .parse(batchFiles, this::analysisCancelled, analysisProgress, (input, result) -> scanAsBatchCallback(input, result, context, environmentsCleaners));
+      .create(javaVersion, context.getClasspath(), sonarComponents.shouldIgnoreUnnamedModuleForSplitPackage())
+      .parse(batchFiles, sonarComponents::analysisCancelled, analysisProgress, (input, result) -> scanAsBatchCallback(input, result, context, environmentsCleaners));
     // Due to a bug in ECJ, JAR files remain locked after the analysis on Windows, we unlock them manually, at the end of each batches. See SONARJAVA-3609.
     environmentsCleaners.forEach(Runnable::run);
     analysisProgress.endBatch();
@@ -320,28 +304,13 @@ public void endOfAnalysis() {
 
   }
 
-  @VisibleForTesting
-  boolean isFileByFileEnabled() {
-    return sonarComponents != null && sonarComponents.isFileByFileEnabled();
-  }
-
-  @VisibleForTesting
-  boolean isAutoScan() {
-    return sonarComponents != null && sonarComponents.isAutoScan();
-  }
-
-  @VisibleForTesting
-  long getBatchModeSizeInKB() {
-    return sonarComponents == null ? -1L : sonarComponents.getBatchModeSizeInKB();
-  }
-
   private boolean isCacheEnabled() {
-    return sonarComponents != null && CacheContextImpl.of(sonarComponents).isCacheEnabled();
+    return CacheContextImpl.of(sonarComponents).isCacheEnabled();
   }
 
   private boolean canOptimizeScanning() {
     try {
-      return sonarComponents != null && sonarComponents.canSkipUnchangedFiles() && isCacheEnabled();
+      return sonarComponents.canSkipUnchangedFiles() && isCacheEnabled();
     } catch (ApiMismatchException e) {
       return false;
     }

java-frontend/src/main/java/org/sonar/java/SonarComponents.java

@@ -687,5 +687,5 @@ public SonarLintCache sonarLintCache() {
   public Configuration getConfiguration() {
     return context.config();
   }
-  
+
 }

java-frontend/src/test/java/org/sonar/java/JavaFrontendTest.java

@@ -51,6 +51,7 @@
 import org.sonar.api.scan.issue.filter.IssueFilterChain;
 import org.sonar.api.testfixtures.log.LogTesterJUnit5;
 import org.sonar.api.utils.Version;
+import org.sonar.java.caching.CacheContextImpl;
 import org.sonar.java.classpath.ClasspathForMain;
 import org.sonar.java.classpath.ClasspathForTest;
 import org.sonar.java.exceptions.ApiMismatchException;
@@ -79,6 +80,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.sonar.java.InputFileUtils.addFile;
+import static org.sonar.java.TestUtils.mockSonarComponents;
 
 @EnableRuleMigrationSupport
 class JavaFrontendTest {
@@ -179,7 +181,7 @@ void scanning_empty_project_should_be_logged_in_file_by_file_sonarlint() {
 
   @Test
   void scanning_empty_project_should_be_logged_in_batch() {
-    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), sonarComponents, new Measurer(sensorContext, mock(NoSonarFilter.class)), mock(JavaResourceLocator.class), mainCodeIssueScannerAndFilter);
+    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), mockSonarComponents(), new Measurer(sensorContext, mock(NoSonarFilter.class)), mock(JavaResourceLocator.class), mainCodeIssueScannerAndFilter);
     frontend.scan(Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
 
     assertThat(logTester.logs(Level.INFO)).containsExactly(
@@ -405,7 +407,7 @@ void test_scan_logs_when_caching_is_disabled_when_sonar_components_is_null() {
 
     JavaFrontend frontend = new JavaFrontend(
       new JavaVersionImpl(),
-      null,
+      mockSonarComponents(),
       mock(Measurer.class),
       mock(JavaResourceLocator.class),
       mainCodeIssueScannerAndFilter
@@ -422,13 +424,18 @@ void test_scan_logs_when_caching_is_disabled_when_sonar_components_is_null() {
       );
   }
 
+  /**
+   * Ensure that changes the mock, which may be needed when it is shared with other tests,
+   * do not affect JavaFrontendTest.
+   */
   @Test
-  void test_getters_with_null_sonarComponents() {
-    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), null, new Measurer(sensorContext, mock(NoSonarFilter.class)), mock(JavaResourceLocator.class), mainCodeIssueScannerAndFilter);
-    assertThat(frontend.isAutoScan()).isFalse();
-    assertThat(frontend.isFileByFileEnabled()).isFalse();
-    assertThat(frontend.analysisCancelled()).isFalse();
-    assertThat(frontend.getBatchModeSizeInKB()).isEqualTo(-1L);
+  void test_validate_mockSonarComponents() {
+    SonarComponents mock = mockSonarComponents();
+    assertThat(mock.isAutoScan()).isFalse();
+    assertThat(mock.isFileByFileEnabled()).isFalse();
+    assertThat(mock.analysisCancelled()).isFalse();
+    assertThat(mock.getBatchModeSizeInKB()).isEqualTo(-1L);
+    assertThat(CacheContextImpl.of(mock).isCacheEnabled()).isFalse();
   }
 
   @Test

java-frontend/src/test/java/org/sonar/java/JavaVersionAwareVisitorTest.java

@@ -31,6 +31,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.sonar.java.TestUtils.mockSonarComponents;
 
 class JavaVersionAwareVisitorTest {
 
@@ -77,7 +78,7 @@ void no_java_version_matching() {
 
   private void checkIssues(JavaVersion version) {
     messages.clear();
-    JavaFrontend frontend = new JavaFrontend(version, null, mock(Measurer.class), null, null, javaChecks);
+    JavaFrontend frontend = new JavaFrontend(version, mockSonarComponents(), mock(Measurer.class), null, null, javaChecks);
     frontend.scan(Collections.singletonList(TestUtils.inputFile("src/test/files/JavaVersionAwareChecks.java")),
       Collections.emptyList(), Collections.emptyList());
   }

java-frontend/src/test/java/org/sonar/java/MeasurerTest.java

@@ -29,6 +29,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.sonar.java.TestUtils.mockSonarComponents;
 
 class MeasurerTest {
 
@@ -92,7 +93,7 @@ private void checkMetric(String filename, String metric, Number expectedValue) {
     context.fileSystem().add(inputFile);
 
     Measurer measurer = new Measurer(context, mock(NoSonarFilter.class));
-    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), null, measurer, null, null, new JavaCheck[0]);
+    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), mockSonarComponents(), measurer, null, null);
 
     frontend.scan(Collections.singletonList(inputFile), Collections.emptyList(), Collections.emptyList());
 

java-frontend/src/test/java/org/sonar/java/MeasurerTester.java

@@ -34,6 +34,7 @@
 import org.sonar.plugins.java.api.JavaResourceLocator;
 
 import static org.mockito.Mockito.mock;
+import static org.sonar.java.TestUtils.mockSonarComponents;
 
 public abstract class MeasurerTester {
 
@@ -50,7 +51,7 @@ public void setUp() {
       .forEach(fs::add);
 
     Measurer measurer = new Measurer(context, mock(NoSonarFilter.class));
-    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), null, measurer, mock(JavaResourceLocator.class), null, new JavaCheck[0]);
+    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), mockSonarComponents(), measurer, mock(JavaResourceLocator.class), null);
     List<InputFile> files = StreamSupport.stream(fs.inputFiles().spliterator(), false).toList();
     frontend.scan(files, Collections.emptyList(), Collections.emptyList());
   }

java-frontend/src/test/java/org/sonar/java/TestUtils.java

@@ -21,12 +21,13 @@
 import java.nio.file.Files;
 import java.util.List;
 import java.util.stream.Stream;
-
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class TestUtils {
   private TestUtils() {
@@ -127,4 +128,20 @@ private static List<String> filterOutAnalysisProgressLogLines(Stream<String> log
   public static List<String> filterOutAnalysisProgressLogLines(List<String> logs) {
     return filterOutAnalysisProgressLogLines(logs.stream());
   }
+
+  /**
+   * Creates a Mockito test double for {@link SonarComponents}, pre-configured to avoid
+   * nulls in unit tests and remove the need for null checks in production code.
+   */
+  public static SonarComponents mockSonarComponents() {
+    SonarComponents mock = mock(SonarComponents.class);
+    when(mock.isSonarLintContext()).thenReturn(true);
+    when(mock.getBatchModeSizeInKB()).thenReturn(-1L);
+    when(mock.getJavaClasspath()).thenReturn(List.of());
+    when(mock.getJavaTestClasspath()).thenReturn(List.of());
+    when(mock.getJspClasspath()).thenReturn(List.of());
+    when(mock.testChecks()).thenReturn(List.of());
+    when(mock.jspChecks()).thenReturn(List.of());
+    return mock;
+  }
 }

java-frontend/src/test/java/org/sonar/java/ast/visitors/FileLinesVisitorTest.java

@@ -34,6 +34,7 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.sonar.java.TestUtils.mockSonarComponents;
 
 class FileLinesVisitorTest {
 
@@ -50,7 +51,7 @@ private void checkLines(String filename, FileLinesContext context) {
     SonarComponents sonarComponents = mock(SonarComponents.class);
     when(sonarComponents.fileLinesContextFor(Mockito.any(InputFile.class))).thenReturn(context);
 
-    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), null, mock(Measurer.class), null, null, new FileLinesVisitor(sonarComponents));
+    JavaFrontend frontend = new JavaFrontend(new JavaVersionImpl(), mockSonarComponents(), mock(Measurer.class), null, null, new FileLinesVisitor(sonarComponents));
 
     frontend.scan(Collections.singletonList(inputFile), Collections.emptyList(), Collections.emptyList());
   }

java-frontend/src/test/java/org/sonar/java/ast/visitors/SyntaxHighlighterVisitorTest.java

@@ -47,6 +47,7 @@
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
+import static org.sonar.java.TestUtils.mockSonarComponents;
 
 @EnableRuleMigrationSupport
 class SyntaxHighlighterVisitorTest {
@@ -254,7 +255,7 @@ void sealed_classes() {
 
   private void scan(InputFile inputFile) {
     JavaVersion javaVersion = JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION;
-    JavaFrontend frontend = new JavaFrontend(javaVersion, null, mock(Measurer.class), null, null, syntaxHighlighterVisitor);
+    JavaFrontend frontend = new JavaFrontend(javaVersion, mockSonarComponents(), mock(Measurer.class), null, null, syntaxHighlighterVisitor);
     frontend.scan(Collections.singletonList(inputFile), Collections.emptyList(), Collections.emptyList());
   }