SONARJAVA-5659 Make inAndroidContext final (#5223)

In order to make code easier to understand we made two changes:
* inAndroidContext is now final (in prod code)
* VisitorsBridgeForTests is constructed using a builder

Author: tomasz-tylenda-sonarsource

java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifier.java

@@ -282,17 +282,18 @@ private void verifyAll() {
       visitors.add(expectations.parser());
     }
     SonarComponents sonarComponents = CheckVerifierUtils.sonarComponents(isCacheEnabled, readCache, writeCache, null);
-    VisitorsBridgeForTests visitorsBridge;
     JavaVersion actualVersion = javaVersion == null ? DEFAULT_JAVA_VERSION : javaVersion;
-    if (withoutSemantic) {
-      visitorsBridge = new VisitorsBridgeForTests(visitors, sonarComponents, actualVersion);
-    } else {
+    VisitorsBridgeForTests.Builder visitorsBridgeBuilder = new VisitorsBridgeForTests.Builder(visitors)
+      .withJavaVersion(actualVersion)
+      .withSonarComponents(sonarComponents)
+      .withAndroidContext(inAndroidContext);
+    if (!withoutSemantic) {
       List<File> actualClasspath = classpath == null ? TestClasspathUtils.DEFAULT_MODULE.getClassPath() : classpath;
-      visitorsBridge = new VisitorsBridgeForTests(visitors, actualClasspath, sonarComponents, actualVersion);
+      visitorsBridgeBuilder.enableSemanticWithProjectClasspath(actualClasspath);
     }
+    VisitorsBridgeForTests visitorsBridge = visitorsBridgeBuilder.build();
 
     JavaAstScanner astScanner = new JavaAstScanner(sonarComponents);
-    visitorsBridge.setInAndroidContext(inAndroidContext);
     astScanner.setVisitorBridge(visitorsBridge);
 
     List<InputFile> filesToParse = files;

java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifier.java

@@ -99,16 +99,16 @@ private MultiFileVerifier createVerifier() {
     CommentLinesVisitor commentLinesVisitor = new CommentLinesVisitor();
     visitors.add(commentLinesVisitor);
     SonarComponents sonarComponents = CheckVerifierUtils.sonarComponents(isCacheEnabled, readCache, writeCache, rootDirectory);
-    VisitorsBridgeForTests visitorsBridge;
-    if (withoutSemantic) {
-      visitorsBridge = new VisitorsBridgeForTests(visitors, sonarComponents, actualVersion);
-    } else {
-      visitorsBridge = new VisitorsBridgeForTests(visitors, actualClasspath, sonarComponents, actualVersion);
+    VisitorsBridgeForTests.Builder visitorsBridgeBuilder = new VisitorsBridgeForTests.Builder(visitors)
+      .withJavaVersion(actualVersion)
+      .withSonarComponents(sonarComponents)
+      .withAndroidContext(inAndroidContext);
+    if(!withoutSemantic) {
+      visitorsBridgeBuilder.enableSemanticWithProjectClasspath(actualClasspath);
     }
 
     JavaAstScanner astScanner = new JavaAstScanner(sonarComponents);
-    visitorsBridge.setInAndroidContext(inAndroidContext);
-
+    VisitorsBridgeForTests visitorsBridge = visitorsBridgeBuilder.build();
     astScanner.setVisitorBridge(visitorsBridge);
 
     List<InputFile> filesToParse = files;

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

@@ -36,7 +36,9 @@ void test() {
     SonarComponents sonarComponents = mock(SonarComponents.class);
     when(sonarComponents.inputFileContents(any())).thenCallRealMethod();
 
-    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests(new ParsingErrorCheck(), sonarComponents);
+    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests.Builder(new ParsingErrorCheck())
+      .withSonarComponents(sonarComponents)
+      .build();
     JavaAstScanner.scanSingleFileForTests(TestUtils.inputFile("src/test/files/checks/parsing/ParsingError.java"), visitorsBridge);
     Set<AnalyzerMessage> issues = visitorsBridge.lastCreatedTestContext().getIssues();
     assertThat(issues).hasSize(1);
@@ -54,7 +56,9 @@ void javacEmptyStatementsInImportsBug() {
     SonarComponents sonarComponents = mock(SonarComponents.class);
     when(sonarComponents.inputFileContents(any())).thenCallRealMethod();
 
-    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests(new ParsingErrorCheck(), sonarComponents);
+    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests.Builder(new ParsingErrorCheck())
+      .withSonarComponents(sonarComponents)
+      .build();
     JavaAstScanner.scanSingleFileForTests(TestUtils.inputFile("src/test/files/checks/parsing/EmptyStatementsInImportsBug.java"), visitorsBridge);
     Set<AnalyzerMessage> issues = visitorsBridge.lastCreatedTestContext().getIssues();
     assertThat(issues).hasSize(1);

java-checks/src/test/java/org/sonar/java/filters/AnyRuleIssueFilterTest.java

@@ -26,7 +26,6 @@
 import org.sonar.api.scan.issue.filter.FilterableIssue;
 import org.sonar.java.ast.JavaAstScanner;
 import org.sonar.java.checks.verifier.TestUtils;
-import org.sonar.java.model.JavaVersionImpl;
 import org.sonar.java.testing.VisitorsBridgeForTests;
 import org.sonar.plugins.java.api.location.Range;
 import org.sonar.plugins.java.api.tree.SyntaxToken;
@@ -132,7 +131,10 @@ private AbstractBooleanAssert<?> assertThatIssueWillBeAccepted(@Nullable Integer
   }
 
   private static void scanFile(JavaIssueFilter filter) {
-    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests(Collections.singletonList(filter), Collections.emptyList(), null, new JavaVersionImpl());
+    VisitorsBridgeForTests visitorsBridge =
+      new VisitorsBridgeForTests.Builder(filter)
+        .enableSemanticWithProjectClasspath(Collections.emptyList())
+        .build();
     JavaAstScanner.scanSingleFileForTests(INPUT_FILE, visitorsBridge);
   }
 

java-checks/src/test/java/org/sonar/java/filters/BaseTreeVisitorIssueFilterTest.java

@@ -176,7 +176,7 @@ private static class FakeRuleWithoutKey implements JavaCheck {
   }
 
   private static void scanFile(JavaIssueFilter filter) {
-    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests(Collections.singletonList(filter), Collections.emptyList(), null, new JavaVersionImpl());
+    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests.Builder(filter).build();
     JavaAstScanner.scanSingleFileForTests(INPUT_FILE, visitorsBridge);
   }
 }

java-checks/src/test/java/org/sonar/java/filters/FilterVerifier.java

@@ -41,7 +41,6 @@
 import org.sonar.java.ast.visitors.SubscriptionVisitor;
 import org.sonar.java.checks.verifier.CheckVerifier;
 import org.sonar.java.checks.verifier.TestUtils;
-import org.sonar.java.model.JavaVersionImpl;
 import org.sonar.java.model.LineUtils;
 import org.sonar.java.reporting.AnalyzerMessage;
 import org.sonar.java.testing.JavaFileScannerContextForTests;
@@ -81,12 +80,12 @@ public void verify(String filename, JavaIssueFilter filter, JavaCheck... extraJa
     projectClasspath.add(new File("target/test-classes"));
 
     InputFile inputFile = TestUtils.inputFile(filename);
-    VisitorsBridgeForTests visitorsBridge;
-    if (this.withoutSemantic) {
-      visitorsBridge = new VisitorsBridgeForTests(visitors, sonarComponents(inputFile), new JavaVersionImpl());
-    } else {
-      visitorsBridge = new VisitorsBridgeForTests(visitors, projectClasspath, sonarComponents(inputFile), new JavaVersionImpl());
+    VisitorsBridgeForTests.Builder visitorsBridgeBuilder = new VisitorsBridgeForTests.Builder(visitors)
+      .withSonarComponents(sonarComponents(inputFile));
+    if (!this.withoutSemantic) {
+      visitorsBridgeBuilder.enableSemanticWithProjectClasspath(projectClasspath);
     }
+    VisitorsBridgeForTests visitorsBridge = visitorsBridgeBuilder.build();
     JavaAstScanner.scanSingleFileForTests(inputFile, visitorsBridge);
     JavaFileScannerContextForTests testJavaFileScannerContext = visitorsBridge.lastCreatedTestContext();
 

java-frontend/src/main/java/org/sonar/java/model/VisitorsBridge.java

@@ -75,8 +75,7 @@ public class VisitorsBridge {
   protected InputFile currentFile;
   protected final JavaVersion javaVersion;
   private final List<File> classpath;
-  // `inAndroidContext` should be made private final
-  protected boolean inAndroidContext = false;
+  protected final boolean inAndroidContext;
   private int fullyScannedFileCount = 0;
   private int skippedFileCount = 0;
   @VisibleForTesting
@@ -85,29 +84,28 @@ public class VisitorsBridge {
 
   @VisibleForTesting
   public VisitorsBridge(JavaFileScanner visitor) {
-    this(Collections.singletonList(visitor), new ArrayList<>(), null, new JavaVersionImpl());
+    this(Collections.singletonList(visitor), new ArrayList<>(), null, new JavaVersionImpl(), false);
   }
 
   @VisibleForTesting
   public VisitorsBridge(Iterable<? extends JavaCheck> visitors, List<File> projectClasspath, @Nullable SonarComponents sonarComponents) {
-    this(visitors, projectClasspath, sonarComponents, new JavaVersionImpl());
+    this(visitors, projectClasspath, sonarComponents, new JavaVersionImpl(), false);
   }
 
-  public VisitorsBridge(Iterable<? extends JavaCheck> visitors, List<File> projectClasspath, @Nullable SonarComponents sonarComponents, JavaVersion javaVersion,
-    boolean inAndroidContext) {
-    this(visitors, projectClasspath, sonarComponents, javaVersion);
-    setInAndroidContext(inAndroidContext);
+  public VisitorsBridge(Iterable<? extends JavaCheck> visitors, List<File> projectClasspath, @Nullable SonarComponents sonarComponents, JavaVersion javaVersion) {
+    this(visitors, projectClasspath, sonarComponents, javaVersion, false);
   }
 
-  public VisitorsBridge(Iterable<? extends JavaCheck> visitors, List<File> projectClasspath, @Nullable SonarComponents sonarComponents, JavaVersion javaVersion) {
+  public VisitorsBridge(Iterable<? extends JavaCheck> visitors, List<File> projectClasspath, @Nullable SonarComponents sonarComponents, JavaVersion javaVersion,
+    boolean inAndroidContext) {
     this.visitors = visitors;
     this.allScanners = new ArrayList<>();
     this.scannersThatCannotBeSkipped = new ArrayList<>();
     this.classpath = projectClasspath;
     this.sonarComponents = sonarComponents;
     this.cacheContext = CacheContextImpl.of(sonarComponents);
-
     this.javaVersion = javaVersion;
+    this.inAndroidContext = inAndroidContext;
     dependencyService = new DependencyVersionInference();
     updateScanners();
   }
@@ -180,10 +178,6 @@ public List<File> getClasspath() {
     return classpath;
   }
 
-  public void setInAndroidContext(boolean inAndroidContext) {
-    this.inAndroidContext = inAndroidContext;
-  }
-
   public void setCacheContext(CacheContext cacheContext) {
     this.cacheContext = cacheContext;
   }

java-frontend/src/main/java/org/sonar/java/testing/VisitorsBridgeForTests.java

@@ -24,7 +24,6 @@
 import org.sonar.java.SonarComponents;
 import org.sonar.java.annotations.VisibleForTesting;
 import org.sonar.java.model.JavaVersionImpl;
-import org.sonar.plugins.java.api.semantic.Sema;
 import org.sonar.java.model.VisitorsBridge;
 import org.sonar.plugins.java.api.InputFileScannerContext;
 import org.sonar.plugins.java.api.JavaCheck;
@@ -33,27 +32,70 @@
 import org.sonar.plugins.java.api.JavaVersion;
 import org.sonar.plugins.java.api.ModuleScannerContext;
 import org.sonar.plugins.java.api.caching.CacheContext;
+import org.sonar.plugins.java.api.semantic.Sema;
 import org.sonar.plugins.java.api.tree.CompilationUnitTree;
 
 public class VisitorsBridgeForTests extends VisitorsBridge {
 
   private JavaFileScannerContextForTests testContext;
   private JavaFileScannerContextForTests moduleContext;
-  private boolean enableSemantic = true;
-
+  private final boolean enableSemantic;
 
-  @VisibleForTesting
-  public VisitorsBridgeForTests(JavaFileScanner visitor, SonarComponents sonarComponents) {
-    this(Collections.singletonList(visitor), Collections.emptyList(), sonarComponents, new JavaVersionImpl());
+  private VisitorsBridgeForTests(Builder builder) {
+    super(builder.visitors, builder.projectClasspath, builder.sonarComponents, builder.javaVersion, builder.inAndroidContext);
+    this.enableSemantic = builder.enableSemantic;
   }
 
-  public VisitorsBridgeForTests(Iterable<? extends JavaCheck> visitors, @Nullable SonarComponents sonarComponents, JavaVersion javaVersion) {
-    super(visitors, Collections.emptyList(), sonarComponents, javaVersion);
-    enableSemantic = false;
+  public static class Builder {
+    Iterable<? extends JavaCheck> visitors;
+    SonarComponents sonarComponents;
+    JavaVersion javaVersion;
+    List<File> projectClasspath;
+    boolean enableSemantic;
+    boolean inAndroidContext;
+
+    public Builder(JavaFileScanner visitor) {
+      this(Collections.singletonList(visitor));
+    }
+
+    public Builder(Iterable<? extends JavaCheck> visitors) {
+      this.visitors = visitors;
+      this.sonarComponents = null;
+      this.javaVersion = new JavaVersionImpl();
+      this.projectClasspath = Collections.emptyList();
+      this.enableSemantic = false;
+      this.inAndroidContext = false;
+    }
+
+    public Builder withJavaVersion(JavaVersion javaVersion) {
+      this.javaVersion = javaVersion;
+      return this;
+    }
+
+    public Builder withSonarComponents(SonarComponents sonarComponents) {
+      this.sonarComponents = sonarComponents;
+      return this;
+    }
+
+    public Builder enableSemanticWithProjectClasspath(List<File> projectClasspath) {
+      this.projectClasspath = projectClasspath;
+      this.enableSemantic = true;
+      return this;
+    }
+
+    public Builder withAndroidContext(boolean inAndroidContext) {
+      this.inAndroidContext = inAndroidContext;
+      return this;
+    }
+
+    public VisitorsBridgeForTests build() {
+      return new VisitorsBridgeForTests(this);
+    }
   }
 
-  public VisitorsBridgeForTests(Iterable<? extends JavaCheck> visitors, List<File> projectClasspath, @Nullable SonarComponents sonarComponents, JavaVersion javaVersion) {
-    super(visitors, projectClasspath, sonarComponents, javaVersion);
+  @VisibleForTesting
+  boolean inAndroidContext() {
+    return inAndroidContext;
   }
 
   @Override

java-frontend/src/test/java/org/sonar/java/testing/VisitorsBridgeForTestsTest.java

@@ -17,6 +17,7 @@
 package org.sonar.java.testing;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.Collections;
 import org.junit.jupiter.api.Test;
 import org.sonar.api.batch.fs.InputFile;
@@ -46,13 +47,18 @@ void test_semantic_disabled() {
     sonarComponents.setSensorContext(context);
 
     Tree parse = JParserTestUtils.parse("class A{}");
-    VisitorsBridgeForTests visitorsBridgeForTests = new VisitorsBridgeForTests(Collections.singletonList(new DummyVisitor()), sonarComponents, new JavaVersionImpl());
+    VisitorsBridgeForTests visitorsBridgeForTests = new VisitorsBridgeForTests.Builder(new DummyVisitor())
+      .withSonarComponents(sonarComponents)
+      .build();
     visitorsBridgeForTests.setCurrentFile(TestUtils.emptyInputFile("dummy.java"));
     visitorsBridgeForTests.visitFile(parse, false);
     assertThat(visitorsBridgeForTests.lastCreatedTestContext().getSemanticModel()).isNull();
 
     parse = JParserTestUtils.parse("class A{}");
-    visitorsBridgeForTests = new VisitorsBridgeForTests(new DummyVisitor(), sonarComponents);
+    visitorsBridgeForTests = new VisitorsBridgeForTests.Builder(new DummyVisitor())
+      .withSonarComponents(sonarComponents)
+      .enableSemanticWithProjectClasspath(new ArrayList<>())
+      .build();
     visitorsBridgeForTests.setCurrentFile(TestUtils.emptyInputFile("dummy.java"));
     visitorsBridgeForTests.visitFile(parse, false);
     assertThat(visitorsBridgeForTests.lastCreatedTestContext().getSemanticModel()).isNotNull();
@@ -66,7 +72,9 @@ void test_report_with_analysis_message() {
 
     Tree parse = JParserTestUtils.parse("class A{}");
     DummyVisitor javaCheck = new DummyVisitor();
-    VisitorsBridgeForTests visitorsBridgeForTests = new VisitorsBridgeForTests(Collections.singletonList(javaCheck), sonarComponents, new JavaVersionImpl());
+    VisitorsBridgeForTests visitorsBridgeForTests = new VisitorsBridgeForTests.Builder(javaCheck)
+      .withSonarComponents(sonarComponents)
+      .build();
     visitorsBridgeForTests.setCurrentFile(TestUtils.emptyInputFile("dummy.java"));
     visitorsBridgeForTests.visitFile(parse, false);
     JavaFileScannerContextForTests lastContext = visitorsBridgeForTests.lastCreatedTestContext();
@@ -88,7 +96,9 @@ void create_InputFileScannerContext_also_sets_testContext_field() {
     SonarComponents sonarComponents = new SonarComponents(null, context.fileSystem(), null, null, null, null);
     sonarComponents.setSensorContext(context);
     DummyVisitor javaCheck = new DummyVisitor();
-    VisitorsBridgeForTests visitorsBridgeForTests = new VisitorsBridgeForTests(Collections.singletonList(javaCheck), sonarComponents, new JavaVersionImpl());
+    VisitorsBridgeForTests visitorsBridgeForTests = new VisitorsBridgeForTests.Builder(javaCheck)
+      .withSonarComponents(sonarComponents)
+      .build();
     var inputFile = mock(InputFile.class);
 
     var expectedTestContext =
@@ -99,7 +109,9 @@ void create_InputFileScannerContext_also_sets_testContext_field() {
 
   @Test
   void lastCreatedModuleContext_returns_last_created_module_context() {
-    var bridge = new VisitorsBridgeForTests(Collections.emptyList(), mock(SonarComponents.class), new JavaVersionImpl());
+    var bridge = new VisitorsBridgeForTests.Builder(Collections.emptyList())
+      .withSonarComponents(mock(SonarComponents.class))
+      .build();
     var firstModuleContext = bridge.createScannerContext(null, new JavaVersionImpl(), false, null);
     var secondAndExpectedModuleContext = bridge.createScannerContext(null, new JavaVersionImpl(), false, null);
     var firstTestContext = bridge.createScannerContext((CompilationUnitTree) null, null, null, false);
@@ -112,6 +124,17 @@ void lastCreatedModuleContext_returns_last_created_module_context() {
       .isNotSameAs(secondTestContext);
   }
 
+  @Test
+  void test_builder() {
+    VisitorsBridgeForTests visitorsBridge =
+      new VisitorsBridgeForTests.Builder(Collections.emptyList())
+        .withJavaVersion(JavaVersionImpl.fromString("17"))
+        .withAndroidContext(true)
+        .build();
+    assertThat(visitorsBridge.getJavaVersion().asInt()).isEqualTo(17);
+    assertThat(visitorsBridge.inAndroidContext()).isTrue();
+  }
+
   private static class DummyVisitor implements JavaFileScanner {
     @Override
     public void scanFile(JavaFileScannerContext context) {

java-frontend/src/test/java/org/sonar/plugins/java/api/IssuableSubscriptionVisitorTest.java

@@ -38,7 +38,7 @@ class IssuableSubscriptionVisitorTest {
 
   @Test
   void test_custom_rules_report_issues() {
-    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests(Collections.singletonList(new CustomRule()), new ArrayList<>(), null, new JavaVersionImpl());
+    VisitorsBridgeForTests visitorsBridge = new VisitorsBridgeForTests.Builder(new CustomRule()).build();
     JavaAstScanner.scanSingleFileForTests(TestUtils.inputFile("src/test/resources/IssuableSubscriptionClass.java"), visitorsBridge);
     Set<AnalyzerMessage> issues = visitorsBridge.lastCreatedTestContext().getIssues();
     assertThat(issues).hasSize(8);