Skip to content

SONARCH-708, SONARJAVA-5497: Extend SonarJava CheckRegistrar API for registering custom file scanner hooks #5083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kaufco merged 6 commits into from
May 5, 2025
Merged

SONARCH-708, SONARJAVA-5497: Extend SonarJava CheckRegistrar API for registering custom file scanner hooks #5083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8c1ed5a
SONARCH-708 SonarJava: extend CheckRegistrar API for registering cust…
kaufco Apr 11, 2025
1a3cd0e
QG
kaufco Apr 11, 2025
c1c3e4b
Added API comments and comments to explain the purpose of the test
kaufco Apr 17, 2025
e3731be
Improve API doc, remove empty method test, add report test
kaufco Apr 25, 2025
566c32b
Added E2E test
kaufco May 5, 2025
b5a6873
Fix typo
kaufco May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions ...hecks-testkit/src/main/java/org/sonar/java/checks/verifier/TestCheckRegistrarContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
import java.util.Set;
import org.apache.commons.lang3.StringUtils;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.rule.RuleScope;
import org.sonar.api.rules.RuleAnnotationUtils;
import org.sonar.plugins.java.api.CheckRegistrar;
import org.sonar.plugins.java.api.JavaCheck;
import org.sonar.plugins.java.api.JavaFileScanner;

public class TestCheckRegistrarContext extends CheckRegistrar.RegistrarContext {

Expand All @@ -52,16 +54,34 @@ public void registerTestChecks(String repositoryKey, Collection<?> javaCheckClas

@Override
public void registerMainSharedCheck(JavaCheck check, Collection<RuleKey> ruleKeys) {
registerMainHook(check);
mainRuleKeys.addAll(ruleKeys);
}

private void registerMainHook(JavaCheck check) {
mainCheckClasses.add(check.getClass());
mainCheckInstances.add(check);
mainRuleKeys.addAll(ruleKeys);
}

@Override
public void registerTestSharedCheck(JavaCheck check, Collection<RuleKey> ruleKeys) {
registerTestHook(check);
testRuleKeys.addAll(ruleKeys);
}

private void registerTestHook(JavaCheck check) {
testCheckClasses.add(check.getClass());
testCheckInstances.add(check);
testRuleKeys.addAll(ruleKeys);
}

@Override
public void registerCustomFileScanner(RuleScope ruleScope, JavaFileScanner scanner) {
if (ruleScope == RuleScope.MAIN || ruleScope == RuleScope.ALL) {
registerMainHook(scanner);
}
if (ruleScope == RuleScope.TEST || ruleScope == RuleScope.ALL) {
registerTestHook(scanner);
}
}

@Override
Expand Down
33 changes: 33 additions & 0 deletions ...s-testkit/src/test/java/org/sonar/java/checks/verifier/TestCheckRegistrarContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@
import java.util.List;
import org.junit.jupiter.api.Test;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.rule.RuleScope;
import org.sonar.check.Rule;
import org.sonar.plugins.java.api.JavaCheck;
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JavaFileScannerContext;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand Down Expand Up @@ -125,4 +128,34 @@ void should_fail_if_not_a_JavaCheck() {
.hasMessage("Fail to instantiate class java.lang.Object");
}

@Test
void register_custom_file_scanners_with_no_active_rules() {
class DummyScanner implements JavaFileScanner {
@Override
public void scanFile(JavaFileScannerContext context) {
// Dummy implementation. We just need the class instance
}
}

class MainScanner extends DummyScanner {
}

class TestScanner extends DummyScanner {
}

class AllScanner extends DummyScanner {
}

var ctx = new TestCheckRegistrarContext();
ctx.registerCustomFileScanner(RuleScope.MAIN, new MainScanner());
ctx.registerCustomFileScanner(RuleScope.TEST, new TestScanner());
ctx.registerCustomFileScanner(RuleScope.ALL, new AllScanner());

assertThat(ctx.mainCheckInstances)
.extracting(c -> c.getClass().getSimpleName())
.containsExactly("MainScanner", "AllScanner");
assertThat(ctx.testCheckInstances)
.extracting(c -> c.getClass().getSimpleName())
.containsExactly("TestScanner", "AllScanner");
}
}
12 changes: 12 additions & 0 deletions java-frontend/src/main/java/org/sonar/java/SonarComponents.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.sonar.api.measures.FileLinesContext;
import org.sonar.api.measures.FileLinesContextFactory;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.rule.RuleScope;
import org.sonar.api.utils.Version;
import org.sonar.java.annotations.VisibleForTesting;
import org.sonar.java.caching.ContentHashCache;
Expand All @@ -69,6 +70,7 @@
import org.sonar.java.reporting.JavaIssue;
import org.sonar.plugins.java.api.CheckRegistrar;
import org.sonar.plugins.java.api.JavaCheck;
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JspCodeVisitor;
import org.sonar.plugins.java.api.caching.SonarLintCache;
import org.sonarsource.api.sonarlint.SonarLintSide;
Expand Down Expand Up @@ -305,6 +307,16 @@ public void registerTestSharedCheck(JavaCheck check, Collection<RuleKey> ruleKey
}
}

@Override
public void registerCustomFileScanner(RuleScope ruleScope, JavaFileScanner scanner) {
if (ruleScope == RuleScope.MAIN || ruleScope == RuleScope.ALL) {
mainChecks.add(scanner);
}
if (ruleScope == RuleScope.TEST || ruleScope == RuleScope.ALL) {
testChecks.add(scanner);
}
}

@Override
public void registerAutoScanCompatibleRules(Collection<RuleKey> ruleKeys) {
additionalAutoScanCompatibleRuleKeys.addAll(ruleKeys);
Expand Down
11 changes: 11 additions & 0 deletions java-frontend/src/main/java/org/sonar/plugins/java/api/CheckRegistrar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.sonar.api.batch.rule.CheckFactory;
import org.sonar.api.batch.rule.Checks;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.rule.RuleScope;
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.java.Preconditions;
import org.sonar.java.annotations.Beta;
Expand Down Expand Up @@ -171,6 +172,16 @@ public void registerMainSharedCheck(JavaCheck check, Collection<RuleKey> ruleKey
// to be overridden
}

/**
* Registers a custom file scanner not related to any rule or repository.
* CheckRegistrars call this function to register a custom file scanner for execution during the analysis
* on all source files that match the given rule scope (MAIN, TEST or ALL).
* Custom file scanners reporting an issue will have no effect, since no rule is associated.
Copy link
Copy Markdown
Contributor

@dorian-burihabwa-sonarsource dorian-burihabwa-sonarsource May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will have no effect: meaning that it is safe to report and not trigger any issue at runtime?

Copy link
Copy Markdown
Contributor Author

@kaufco kaufco May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. There's also a test for this. Reason: there was a null check somewhere in the implementation chain of reportIssue / addIssue (I don't remember in which class/function exactly) triggering the underlying low-level function only if ruleKey != null, doing nothing otherwise.

*/
public void registerCustomFileScanner(RuleScope ruleScope, JavaFileScanner scanner) {
Comment thread
leonardo-pilastri-sonarsource marked this conversation as resolved.
// to be overridden
}

/**
* Registers one test code check related to not one but a list of rules. The check will be active if at least one
* of the given rule key is active. In this context injection of @RuleProperty and auto instantiation of rules
Expand Down
44 changes: 42 additions & 2 deletions java-frontend/src/test/java/org/sonar/java/JavaFrontendTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import org.eclipse.core.runtime.OperationCanceledException;
import org.jetbrains.annotations.NotNull;
import org.junit.Rule;
Expand All @@ -51,6 +52,7 @@
import org.sonar.api.issue.NoSonarFilter;
import org.sonar.api.measures.FileLinesContext;
import org.sonar.api.measures.FileLinesContextFactory;
import org.sonar.api.rule.RuleScope;
import org.sonar.api.scan.issue.filter.FilterableIssue;
import org.sonar.api.scan.issue.filter.IssueFilterChain;
import org.sonar.api.testfixtures.log.LogTesterJUnit5;
Expand All @@ -60,6 +62,8 @@
import org.sonar.java.exceptions.ApiMismatchException;
import org.sonar.java.filters.SonarJavaIssueFilter;
import org.sonar.java.model.JavaVersionImpl;
import org.sonar.plugins.java.api.CheckRegistrar;
import org.sonar.plugins.java.api.JavaCheck;
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.JavaResourceLocator;
Expand Down Expand Up @@ -191,6 +195,38 @@ void scanning_empty_project_should_be_logged_in_batch() {
);
}

@Test
void scan_method_is_called_for_hook() throws IOException {
class Hook implements CheckRegistrar, JavaFileScanner {
int callCount;

@Override
public void register(RegistrarContext registrarContext) {
registrarContext.registerCustomFileScanner(RuleScope.ALL, this);
}

@Override
public void scanFile(JavaFileScannerContext context) {
callCount++;
}
}

var settings = new MapSettings();
if (sensorContext == null) {
File baseDir = temp.getRoot().getAbsoluteFile();
sensorContext = SensorContextTester.create(baseDir);
sensorContext.setSettings(settings);
}

var hook = new Hook();
var inputFiles = List.of(
addFile("class A {}", sensorContext),
addFile("class B {}", sensorContext)
);
scan(settings, SONARLINT_RUNTIME, inputFiles, new CheckRegistrar[]{hook});
assertThat(hook.callCount).isEqualTo(2);
}

@Test
void scanning_empty_project_should_be_logged_in_autoscan() {
MapSettings settings = new MapSettings();
Expand Down Expand Up @@ -808,6 +844,10 @@ private List<InputFile> scan(MapSettings settings, SonarRuntime sonarRuntime, St
}

private List<InputFile> scan(MapSettings settings, SonarRuntime sonarRuntime, List<InputFile> inputFiles) {
return scan(settings, sonarRuntime, inputFiles, null);
}

private List<InputFile> scan(MapSettings settings, SonarRuntime sonarRuntime, List<InputFile> inputFiles, @Nullable CheckRegistrar[] checkRegistrars) {
if (sensorContext == null) {
File baseDir = temp.getRoot().getAbsoluteFile();
sensorContext = SensorContextTester.create(baseDir);
Expand All @@ -823,15 +863,15 @@ private List<InputFile> scan(MapSettings settings, SonarRuntime sonarRuntime, Li
javaClasspath = mock(ClasspathForMain.class);
javaTestClasspath = mock(ClasspathForTest.class);
sonarComponents = new SonarComponents(fileLinesContextFactory, sensorContext.fileSystem(), javaClasspath, javaTestClasspath,
mock(CheckFactory.class), mock(ActiveRules.class));
mock(CheckFactory.class), mock(ActiveRules.class), checkRegistrars);
sonarComponents.setSensorContext(sensorContext);
sonarComponents.mainChecks().add(mainCodeIssueScannerAndFilter);
sonarComponents.testChecks().add(testCodeIssueScannerAndFilter);
JavaVersion javaVersion = settings.asConfig().get(JavaVersion.SOURCE_VERSION)
.map(JavaVersionImpl::fromString)
.orElse(new JavaVersionImpl());
JavaFrontend frontend = new JavaFrontend(javaVersion, sonarComponents, new Measurer(sensorContext, mock(NoSonarFilter.class)), mock(JavaResourceLocator.class),
null, mainCodeIssueScannerAndFilter);
null, sonarComponents.mainChecks().toArray(new JavaCheck[0]));
frontend.scan(inputFiles, Collections.emptyList(), Collections.emptyList());

return inputFiles;
Expand Down
56 changes: 56 additions & 0 deletions java-frontend/src/test/java/org/sonar/java/SonarComponentsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.sonar.api.measures.FileLinesContext;
import org.sonar.api.measures.FileLinesContextFactory;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.rule.RuleScope;
import org.sonar.api.testfixtures.log.LogTesterJUnit5;
import org.sonar.api.utils.Version;
import org.sonar.check.Rule;
Expand All @@ -81,6 +82,8 @@
import org.sonar.java.testing.ThreadLocalLogTester;
import org.sonar.plugins.java.api.CheckRegistrar;
import org.sonar.plugins.java.api.JavaCheck;
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.JspCodeVisitor;
import org.sonar.plugins.java.api.caching.SonarLintCache;
import org.sonarsource.sonarlint.core.plugin.commons.sonarapi.SonarLintRuntimeImpl;
Expand Down Expand Up @@ -407,6 +410,44 @@ class RuleF implements JavaCheck {
.containsExactly("RuleE");
}

@Test
void register_custom_file_scanners_with_no_active_rules() {
var noActiveRules = (new ActiveRulesBuilder()).build();
CheckFactory specificCheckFactory = new CheckFactory(noActiveRules);
SensorContextTester specificContext = SensorContextTester.create(new File(".")).setActiveRules(noActiveRules);

class DummyScanner implements JavaFileScanner {
@Override
public void scanFile(JavaFileScannerContext context) {
// Dummy implementation. We just need the class instance
}
}

class MainScanner extends DummyScanner {
}

class TestScanner extends DummyScanner {
}

class AllScanner extends DummyScanner {
}

SonarComponents sonarComponents = new SonarComponents(fileLinesContextFactory, null, null,
null, specificCheckFactory, noActiveRules, new CheckRegistrar[]{
ctx -> ctx.registerCustomFileScanner(RuleScope.MAIN, new MainScanner()),
ctx -> ctx.registerCustomFileScanner(RuleScope.TEST, new TestScanner()),
ctx -> ctx.registerCustomFileScanner(RuleScope.ALL, new AllScanner())
});

sonarComponents.setSensorContext(specificContext);
assertThat(sonarComponents.mainChecks())
.extracting(c -> c.getClass().getSimpleName())
.containsExactly("MainScanner", "AllScanner");
assertThat(sonarComponents.testChecks())
.extracting(c -> c.getClass().getSimpleName())
.containsExactly("TestScanner", "AllScanner");
}

@Test
void register_custom_rule_by_instances_instead_of_classes() {
ActiveRules activeRules = activeRules("java:S101", "java:S102");
Expand Down Expand Up @@ -471,6 +512,21 @@ void no_issue_when_check_not_found() {
verify(context, never()).newIssue();
}

@Test
void no_issue_when_reporting_from_custom_file_scanner() {
Comment thread
leonardo-pilastri-sonarsource marked this conversation as resolved.
JavaFileScanner customScanner = scannerContext -> {
// empty
};
CheckRegistrar registrar = registrarContext ->
registrarContext.registerCustomFileScanner(RuleScope.ALL, customScanner);
SonarComponents sonarComponents = new SonarComponents(fileLinesContextFactory, null, null,
null, checkFactory, context.activeRules(), new CheckRegistrar[]{registrar});
sonarComponents.setSensorContext(context);

sonarComponents.addIssue(TestUtils.emptyInputFile("file.java"), customScanner, 0, "message", null);
verify(context, never()).newIssue();
}

@Test
void add_issue_or_parse_error() {
JavaCheck expectedCheck = new CustomCheck();
Expand Down