Skip to content

Commit

Permalink
Execute Java based parser only for custom rules
Browse files Browse the repository at this point in the history
  • Loading branch information
@saberduck
saberduck committed Dec 8, 2020
1 parent c12dd5e commit 1f9a0b8
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 13 deletions.
7 changes: 7 additions & 0 deletions sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/AbstractChecks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public class AbstractChecks {
private final CustomJavaScriptRulesDefinition[] customRulesDefinitions;
private final CustomRuleRepository[] customRuleRepositories;
private final Set<Checks<JavaScriptCheck>> checksByRepository = new HashSet<>();
private boolean hasCustomChecks;

public AbstractChecks(CheckFactory checkFactory, @Nullable CustomJavaScriptRulesDefinition[] customRulesDefinitions, @Nullable CustomRuleRepository[] customRuleRepositories) {
this.checkFactory = checkFactory;
Expand All @@ -70,6 +71,7 @@ private void addCustomChecks(CustomRuleRepository.Language language) {
LOG.debug("Adding rules for repository '{}' {} from {}", rulesDefinition.repositoryKey(), rulesDefinition.checkClasses(),
rulesDefinition.getClass().getCanonicalName());
doAddChecks(rulesDefinition.repositoryKey(), Arrays.asList(rulesDefinition.checkClasses()));
hasCustomChecks = true;
}
}

Expand All @@ -80,12 +82,17 @@ private void addCustomChecks(CustomRuleRepository.Language language) {
repo.checkClasses(),
repo.getClass().getCanonicalName());
doAddChecks(repo.repositoryKey(), repo.checkClasses());
hasCustomChecks = true;
}
}
}

}

public boolean hasCustomChecks() {
return hasCustomChecks;
}

public List<JavaScriptCheck> all() {
List<JavaScriptCheck> allVisitors = new ArrayList<>();

Expand Down
9 changes: 6 additions & 3 deletions ...plugin/src/main/java/org/sonar/plugins/javascript/eslint/JavaScriptEslintBasedSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,12 @@ public JavaScriptEslintBasedSensor(JavaScriptChecks checks, NoSonarFilter noSona
@Override
void analyzeFiles() throws IOException, InterruptedException {
runEslintAnalysis(provideDefaultTsConfig());
PROFILER.startInfo("Java-based frontend sensor [javascript]");
new JavaScriptSensor(checks).execute(context);
PROFILER.stopInfo();
if (checks.hasCustomChecks()) {
PROFILER.startInfo("Java-based frontend sensor [javascript] for custom rules");
LOG.warn("Custom JavaScript rules are deprecated and API will be removed in future version.");
new JavaScriptSensor(checks).execute(context);
PROFILER.stopInfo();
}
}

private List<String> provideDefaultTsConfig() throws IOException {
Expand Down
56 changes: 48 additions & 8 deletions sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptSensorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.sonar.plugins.javascript.api.JavaScriptCheck;
import org.sonar.plugins.javascript.api.tree.ScriptTree;
import org.sonar.plugins.javascript.api.tree.Tree;
import org.sonar.plugins.javascript.api.tree.expression.LiteralTree;
import org.sonar.plugins.javascript.api.visitors.DoubleDispatchVisitorCheck;
import org.sonar.plugins.javascript.api.visitors.LineIssue;
import org.sonar.plugins.javascript.api.visitors.TreeVisitor;
Expand Down Expand Up @@ -115,7 +116,20 @@ public List<Class<? extends JavaScriptCheck>> checkClasses() {
private final SensorContextTester context = SensorContextTester.create(baseDir);

private JavaScriptSensor createSensor() {
return new JavaScriptSensor(new JavaScriptChecks(checkFactory));
CustomRuleRepository[] ruleRepository = {new CustomRuleRepository() {

@Override
public String repositoryKey() {
return "javascript";
}

@Override
public List<Class<? extends JavaScriptCheck>> checkClasses() {
return ImmutableList.of(OctalNumberCheck.class);
}
}};

return new JavaScriptSensor(new JavaScriptChecks(checkFactory, ruleRepository));
}

private JavaScriptSensor createSensorWithCustomRules() {
Expand Down Expand Up @@ -147,11 +161,10 @@ public void should_only_log_debug_if_a_parsing_error() {
context.setActiveRules(activeRules);
createSensor().execute(context);
Collection<Issue> issues = context.allIssues();
assertThat(issues).hasSize(0);
assertThat(context.allAnalysisErrors()).hasSize(0);
assertThat(issues).isEmpty();
assertThat(context.allAnalysisErrors()).isEmpty();

assertThat(logTester.logs(LoggerLevel.DEBUG).get(0)).isEqualTo("Unable to parse file with java-based frontend (some rules will not be executed): " + inputFile.toString());
assertThat(logTester.logs(LoggerLevel.DEBUG).get(1)).startsWith("Parse error at line 3 column 1:");
assertThat(logTester.logs(LoggerLevel.DEBUG)).contains("Unable to parse file with java-based frontend (some rules will not be executed): " + inputFile.toString());
}

@Test
Expand Down Expand Up @@ -182,7 +195,7 @@ public void should_not_add_error_to_context_when_analysis_raises_issues() {
inputFile("file.js");

ActiveRules activeRules = (new ActiveRulesBuilder())
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of(CheckList.JS_REPOSITORY_KEY, "S1314")).build())
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of(CheckList.JS_REPOSITORY_KEY, "octal")).build())
.build();
checkFactory = new CheckFactory(activeRules);

Expand All @@ -201,7 +214,7 @@ public void should_save_issue() throws Exception {
ActiveRules activeRules = (new ActiveRulesBuilder())
.addRule(new NewActiveRule.Builder()
// octal number
.setRuleKey(RuleKey.of(CheckList.JS_REPOSITORY_KEY, "S1314"))
.setRuleKey(RuleKey.of(CheckList.JS_REPOSITORY_KEY, "octal"))
.build())
.build();

Expand Down Expand Up @@ -304,7 +317,7 @@ public void should_cancel_progress_report_and_return_with_no_exception_when_cont

private void analyseFile(String relativePath) {
ActiveRules activeRules = (new ActiveRulesBuilder())
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of(CheckList.JS_REPOSITORY_KEY, "S1314")).build())
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of(CheckList.JS_REPOSITORY_KEY, "octal")).build())
.build();
checkFactory = new CheckFactory(activeRules);
context.setActiveRules(activeRules);
Expand Down Expand Up @@ -410,6 +423,33 @@ public void visitScript(ScriptTree tree) {
}
}

@Rule(
key = "octal",
name = "Octal",
description = "desc",
tags = {"bug"})
public static class OctalNumberCheck extends DoubleDispatchVisitorCheck {
private static final String MESSAGE = "Replace the value of the octal number (%s) by its decimal equivalent (%s).";

@Override
public void visitLiteral(LiteralTree tree) {
if (tree.is(Tree.Kind.NUMERIC_LITERAL)) {
String value = tree.value();
if (value.length() > 1 && value.startsWith("0")) {
int newValue;
try {
newValue = Integer.parseInt(value, 8);
} catch (NumberFormatException e) {
return;
}
if (newValue > 9) {
addIssue(tree, String.format(MESSAGE, value, newValue));
}
}
}
}
}

@Rule(key = "key2")
public static class CustomRuleWithRuleRepository extends DoubleDispatchVisitorCheck {

Expand Down
30 changes: 28 additions & 2 deletions ...in/src/test/java/org/sonar/plugins/javascript/eslint/JavaScriptEslintBasedSensorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/
package org.sonar.plugins.javascript.eslint;

import com.google.common.collect.ImmutableList;
import com.google.gson.Gson;
import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -62,6 +63,9 @@
import org.sonar.api.utils.log.LoggerLevel;
import org.sonar.javascript.checks.CheckList;
import org.sonar.plugins.javascript.JavaScriptChecks;
import org.sonar.plugins.javascript.JavaScriptSensorTest;
import org.sonar.plugins.javascript.api.CustomRuleRepository;
import org.sonar.plugins.javascript.api.JavaScriptCheck;
import org.sonar.plugins.javascript.eslint.EslintBridgeServer.AnalysisRequest;
import org.sonar.plugins.javascript.eslint.EslintBridgeServer.AnalysisResponse;
import org.sonarsource.nodejs.NodeCommandException;
Expand Down Expand Up @@ -147,6 +151,7 @@ public void should_create_issues_from_eslint_based_rules() throws Exception {
assertThat(firstIssue.ruleKey().rule()).isEqualTo("S3923");
assertThat(secondIssue.ruleKey().rule()).isEqualTo("S3923");
assertThat(thirdIssue.ruleKey().rule()).isEqualTo("S1451");
assertThat(logTester.logs(LoggerLevel.WARN)).doesNotContain("Custom JavaScript rules are deprecated and API will be removed in future version.");
}

private AnalysisResponse response(String json) {
Expand Down Expand Up @@ -484,13 +489,34 @@ public void should_run_old_frontend() throws Exception {
.build();
context.fileSystem().add(inputFile);

JavaScriptChecks checks = checks("S1314");
JavaScriptChecks checks = createChecksWithOctalCustomRule();
NoSonarFilter noSonarFilter = new NoSonarFilter();
JavaScriptEslintBasedSensor sensor = new JavaScriptEslintBasedSensor(checks, noSonarFilter, fileLinesContextFactory, eslintBridgeServerMock, null, tempFolder);
sensor.execute(context);

assertThat(context.allIssues()).hasSize(1);
assertThat(context.allIssues()).extracting(i -> i.ruleKey().toString()).containsExactly("javascript:S1314");
assertThat(context.allIssues()).extracting(i -> i.ruleKey().toString()).containsExactly("javascript:octal");
assertThat(logTester.logs(LoggerLevel.WARN)).contains("Custom JavaScript rules are deprecated and API will be removed in future version.");
}


private JavaScriptChecks createChecksWithOctalCustomRule() {
CustomRuleRepository[] ruleRepository = {new CustomRuleRepository() {

@Override
public String repositoryKey() {
return "javascript";
}

@Override
public List<Class<? extends JavaScriptCheck>> checkClasses() {
return ImmutableList.of(JavaScriptSensorTest.OctalNumberCheck.class);
}
}};
ActiveRulesBuilder builder = new ActiveRulesBuilder();
builder.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of(CheckList.JS_REPOSITORY_KEY, "octal")).build());
CheckFactory checkFactory = new CheckFactory(builder.build());
return new JavaScriptChecks(checkFactory, ruleRepository);
}

@Test
Expand Down

0 comments on commit 1f9a0b8

Please sign in to comment.