Skip to content
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.

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

JS-283 Use repository to determine AnalysisMode #4771

Merged
merged 2 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@

import java.util.HashSet;
import java.util.List;
import java.util.stream.Stream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.rule.ActiveRules;
import org.sonar.api.batch.sensor.SensorContext;

public enum AnalysisMode {
Expand All @@ -37,7 +39,7 @@ public enum AnalysisMode {
private static final Logger LOG = LoggerFactory.getLogger(AnalysisMode.class);


public static AnalysisMode getMode(SensorContext context, List<EslintRule> rules) {
public static AnalysisMode getMode(SensorContext context) {
var logDefaultMode = "Analysis of unchanged files will not be skipped ({})";

var canSkipUnchangedFiles = context.canSkipUnchangedFiles();
Expand All @@ -48,8 +50,9 @@ public static AnalysisMode getMode(SensorContext context, List<EslintRule> rules

// This is not a common use case so falling back to default behaviour even if some optimization is possible
// (possible if all sonar-security rules are deactivated for analysis)
var containsUcfgRule = EslintRule.containsRuleWithKey(rules, EslintRule.UCFG_ESLINT_KEY);
if (!containsUcfgRule) {
// This is used to avoid creating instance of "unchaged" linter when not needed. However, linter id mechanism should be replaced by using
// configuration instead, same as we do for MAIN and TEST files
if (!hasSecurityRules(context.activeRules())) {
LOG.debug(logDefaultMode, "security rules are not available");
return AnalysisMode.DEFAULT;
}
Expand All @@ -60,6 +63,10 @@ public static AnalysisMode getMode(SensorContext context, List<EslintRule> rules
return AnalysisMode.SKIP_UNCHANGED;
}

private static boolean hasSecurityRules(ActiveRules activeRules) {
return Stream.of("jssecurity", "tssecurity").anyMatch(r -> !activeRules.findByRepository(r).isEmpty());
}

public static List<EslintRule> getUnchangedFileRules(List<EslintRule> rules) {
var rule = EslintRule.findFirstRuleWithKey(rules, EslintRule.UCFG_ESLINT_KEY);
return rule == null ? emptyList() : List.of(rule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ public List<Object> getConfigurations() {
return configurations;
}

static boolean containsRuleWithKey(List<EslintRule> rules, String eslintKey) {
return rules.stream().anyMatch(ruleMatcher(eslintKey));
}

static EslintRule findFirstRuleWithKey(List<EslintRule> rules, String eslintKey) {
return rules.stream().filter(ruleMatcher(eslintKey)).findFirst().orElse(null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.rule.ActiveRule;
import org.sonar.api.batch.rule.ActiveRules;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.internal.SonarRuntimeImpl;
import org.sonar.api.utils.Version;
Expand All @@ -56,8 +58,7 @@ void should_reflect_non_skippable_analysis() {
when(context.runtime()).thenReturn(SonarRuntimeImpl.forSonarLint(Version.create(9, 4)));
when(context.canSkipUnchangedFiles()).thenReturn(false);

var rules = rules("key1", "key2");
var mode = AnalysisMode.getMode(context, rules);
var mode = AnalysisMode.getMode(context);
assertThat(mode).isEqualTo(AnalysisMode.DEFAULT);
verify(context).canSkipUnchangedFiles();
}
Expand All @@ -66,9 +67,9 @@ void should_reflect_non_skippable_analysis() {
void should_reflect_skippable_without_security_analysis() {
when(context.runtime()).thenReturn(SonarRuntimeImpl.forSonarLint(Version.create(9, 4)));
when(context.canSkipUnchangedFiles()).thenReturn(true);
when(context.activeRules()).thenReturn(mock(ActiveRules.class));

var rules = rules("key1", "key2");
var mode = AnalysisMode.getMode(context, rules);
var mode = AnalysisMode.getMode(context);
assertThat(mode).isEqualTo(AnalysisMode.DEFAULT);
verify(context).canSkipUnchangedFiles();

Expand All @@ -84,8 +85,7 @@ void should_reflect_main_branch_analysis() {
when(context.runtime()).thenReturn(SonarRuntimeImpl.forSonarLint(Version.create(9, 4)));
when(context.canSkipUnchangedFiles()).thenReturn(false);

var rules = rules("key1", "key2", "ucfg");
var mode = AnalysisMode.getMode(context, rules);
var mode = AnalysisMode.getMode(context);
assertThat(mode).isEqualTo(AnalysisMode.DEFAULT);
verify(context).canSkipUnchangedFiles();

Expand All @@ -100,9 +100,11 @@ void should_reflect_main_branch_analysis() {
void should_reflect_pr_analysis() {
when(context.runtime()).thenReturn(SonarRuntimeImpl.forSonarLint(Version.create(9, 4)));
when(context.canSkipUnchangedFiles()).thenReturn(true);
ActiveRules activeRules = mock(ActiveRules.class);
when(activeRules.findByRepository("jssecurity")).thenReturn(List.of(mock(ActiveRule.class)));
when(context.activeRules()).thenReturn(activeRules);

var rules = rules("key1", "key2", "ucfg");
var mode = AnalysisMode.getMode(context, rules);
var mode = AnalysisMode.getMode(context);
assertThat(mode).isEqualTo(AnalysisMode.SKIP_UNCHANGED);
verify(context).canSkipUnchangedFiles();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@
package org.sonar.plugins.javascript.bridge;

import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.sonar.plugins.javascript.bridge.EslintRule.containsRuleWithKey;
import static org.sonar.plugins.javascript.bridge.EslintRule.findFirstRuleWithKey;

import java.util.Arrays;
Expand All @@ -33,13 +31,6 @@

class EslintRuleTest {

@Test
void should_search_rules() {
assertThat(containsRuleWithKey(rules(), "key1")).isFalse();
assertThat(containsRuleWithKey(rules("key1"), "key1")).isTrue();
assertThat(containsRuleWithKey(rules("key1", "key2"), "key3")).isFalse();
}

@Test
void should_find_first_rule() {
assertThat(findFirstRuleWithKey(rules(), "key1")).isNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void describe(SensorDescriptor descriptor) {
@Override
protected void analyzeFiles(List<InputFile> inputFiles) throws IOException {
var progressReport = new ProgressReport("Analysis progress", TimeUnit.SECONDS.toMillis(10));
analysisMode = AnalysisMode.getMode(context, checks.eslintRules());
analysisMode = AnalysisMode.getMode(context);
var success = false;
try {
progressReport.start(inputFiles.size(), inputFiles.iterator().next().toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.plugins.javascript.JavaScriptFilePredicate;
import org.sonar.plugins.javascript.JavaScriptLanguage;
import org.sonar.plugins.javascript.TypeScriptLanguage;
import org.sonar.plugins.javascript.bridge.AnalysisMode;
import org.sonar.plugins.javascript.bridge.BridgeServer;
import org.sonar.plugins.javascript.JavaScriptFilePredicate;
import org.sonar.plugins.javascript.sonarlint.SonarLintTypeCheckingChecker;

@DependedUpon("js-analysis")
Expand Down Expand Up @@ -99,7 +99,7 @@ protected List<InputFile> getInputFiles() {

@Override
protected void analyzeFiles(List<InputFile> inputFiles) throws IOException {
var analysisMode = AnalysisMode.getMode(context, checks.eslintRules());
var analysisMode = AnalysisMode.getMode(context);
bridgeServer.initLinter(
checks.eslintRules(),
environments,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void describe(SensorDescriptor descriptor) {

@Override
protected void analyzeFiles(List<InputFile> inputFiles) throws IOException {
analysisMode = AnalysisMode.getMode(context, checks.eslintRules());
analysisMode = AnalysisMode.getMode(context);
var progressReport = new ProgressReport("Analysis progress", TimeUnit.SECONDS.toMillis(10));
var success = false;
try {
Expand Down
Loading