Skip to content

Commit

Permalink
SLCORE-365 Handle deprecated rule keys in connected mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@henryju
henryju committed Mar 24, 2022
1 parent db91693 commit ff48688
Show file tree
Hide file tree
Showing 10 changed files with 417 additions and 35 deletions.
101 changes: 74 additions & 27 deletions core/src/main/java/org/sonarsource/sonarlint/core/ConnectedSonarLintEngineImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.sonarsource.sonarlint.core.analysis.api.Issue;
import org.sonarsource.sonarlint.core.analysis.command.AnalyzeCommand;
import org.sonarsource.sonarlint.core.client.api.common.PluginDetails;
import org.sonarsource.sonarlint.core.client.api.common.RuleKey;
import org.sonarsource.sonarlint.core.client.api.common.analysis.DefaultClientIssue;
import org.sonarsource.sonarlint.core.client.api.common.analysis.IssueListener;
import org.sonarsource.sonarlint.core.client.api.connected.ConnectedAnalysisConfiguration;
Expand Down Expand Up @@ -99,6 +100,7 @@
import org.sonarsource.sonarlint.core.storage.UpdateStorageOnRuleSetChanged;

import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toMap;
import static org.apache.commons.lang3.StringUtils.trimToNull;
import static org.sonarsource.sonarlint.core.container.storage.ProjectStoragePaths.encodeForFs;

Expand Down Expand Up @@ -196,14 +198,14 @@ private static class ActiveRulesContext {
private final List<ActiveRule> activeRules = new ArrayList<>();
private final Map<String, ActiveRuleMetadata> activeRulesMetadata = new HashMap<>();

public void includeRule(SonarLintRuleDefinition ruleDefinition, ServerActiveRule activeRule) {
var activeRuleForAnalysis = new ActiveRule(activeRule.getRuleKey(), ruleDefinition.getLanguage().getLanguageKey());
public void includeRule(SonarLintRuleDefinition ruleOrTemplateDefinition, ServerActiveRule activeRule) {
var activeRuleForAnalysis = new ActiveRule(activeRule.getRuleKey(), ruleOrTemplateDefinition.getLanguage().getLanguageKey());
activeRuleForAnalysis.setTemplateRuleKey(trimToNull(activeRule.getTemplateKey()));
Map<String, String> effectiveParams = new HashMap<>(ruleDefinition.getDefaultParams());
Map<String, String> effectiveParams = new HashMap<>(ruleOrTemplateDefinition.getDefaultParams());
effectiveParams.putAll(activeRule.getParams());
activeRuleForAnalysis.setParams(effectiveParams);
activeRules.add(activeRuleForAnalysis);
activeRulesMetadata.put(activeRule.getRuleKey(), new ActiveRuleMetadata(activeRule.getSeverity(), ruleDefinition.getType()));
activeRulesMetadata.put(activeRule.getRuleKey(), new ActiveRuleMetadata(activeRule.getSeverity(), ruleOrTemplateDefinition.getType()));
}

public void includeRule(SonarLintRuleDefinition rule) {
Expand Down Expand Up @@ -271,33 +273,66 @@ private ActiveRulesContext buildActiveRulesContext(ConnectedAnalysisConfiguratio
return analysisRulesContext;
}

var allRulesDefinitionsByKey = analysisContext.get().allRulesDefinitionsByKey;
projectStorage.getAnalyzerConfiguration(projectKey).getRuleSetByLanguageKey().entrySet()
.stream().filter(e -> Language.forKey(e.getKey()).filter(l -> globalConfig.getEnabledLanguages().contains(l)).isPresent())
.forEach(e -> {
var languageKey = e.getKey();
var ruleSet = e.getValue();

LOG.debug(" * {}: {} active rules", languageKey, ruleSet.getRules().size());
for (ServerActiveRule activeRuleFromStorage : ruleSet.getRules()) {
var ruleDefinitionKey = StringUtils.isNotBlank(activeRuleFromStorage.getTemplateKey()) ? activeRuleFromStorage.getTemplateKey() : activeRuleFromStorage.getRuleKey();
var ruleDefinition = allRulesDefinitionsByKey.get(ruleDefinitionKey);
if (ruleDefinition == null) {
LOG.debug("Rule {} is enabled on the server, but not available in SonarLint", activeRuleFromStorage.getRuleKey());
continue;
for (ServerActiveRule possiblyDeprecatedActiveRuleFromStorage : ruleSet.getRules()) {
var activeRuleFromStorage = tryConvertDeprecatedKeys(possiblyDeprecatedActiveRuleFromStorage);
SonarLintRuleDefinition ruleOrTemplateDefinition;
if (StringUtils.isNotBlank(activeRuleFromStorage.getTemplateKey())) {
ruleOrTemplateDefinition = analysisContext.get().findRule(activeRuleFromStorage.getTemplateKey()).orElse(null);
if (ruleOrTemplateDefinition == null) {
LOG.debug("Rule {} is enabled on the server, but its template {} is not available in SonarLint", activeRuleFromStorage.getRuleKey(),
activeRuleFromStorage.getTemplateKey());
continue;
}
} else {
ruleOrTemplateDefinition = analysisContext.get().findRule(activeRuleFromStorage.getRuleKey()).orElse(null);
if (ruleOrTemplateDefinition == null) {
LOG.debug("Rule {} is enabled on the server, but not available in SonarLint", activeRuleFromStorage.getRuleKey());
continue;
}
}
analysisRulesContext.includeRule(ruleDefinition, activeRuleFromStorage);
analysisRulesContext.includeRule(ruleOrTemplateDefinition, activeRuleFromStorage);
}
});

allRulesDefinitionsByKey.values().stream()
analysisContext.get().allRulesDefinitionsByKey.values().stream()
.filter(ruleDefinition -> isRuleFromExtraPlugin(ruleDefinition.getLanguage(), globalConfig))
.forEach(analysisRulesContext::includeRule);

return analysisRulesContext;

}

private ServerActiveRule tryConvertDeprecatedKeys(ServerActiveRule possiblyDeprecatedActiveRuleFromStorage) {
SonarLintRuleDefinition ruleOrTemplateDefinition;
if (StringUtils.isNotBlank(possiblyDeprecatedActiveRuleFromStorage.getTemplateKey())) {
ruleOrTemplateDefinition = analysisContext.get().findRule(possiblyDeprecatedActiveRuleFromStorage.getTemplateKey()).orElse(null);
if (ruleOrTemplateDefinition == null) {
// The rule template is not known among our loaded analyzers, so return it untouched, to let calling code take appropriate decision
return possiblyDeprecatedActiveRuleFromStorage;
}
var ruleKeyPossiblyWithDeprecatedRepo = RuleKey.parse(possiblyDeprecatedActiveRuleFromStorage.getRuleKey());
var templateRuleKeyWithCorrectRepo = RuleKey.parse(ruleOrTemplateDefinition.getKey());
var ruleKey = new RuleKey(templateRuleKeyWithCorrectRepo.repository(), ruleKeyPossiblyWithDeprecatedRepo.rule()).toString();
return new ServerActiveRule(ruleKey, possiblyDeprecatedActiveRuleFromStorage.getSeverity(), possiblyDeprecatedActiveRuleFromStorage.getParams(),
ruleOrTemplateDefinition.getKey());
} else {
ruleOrTemplateDefinition = analysisContext.get().findRule(possiblyDeprecatedActiveRuleFromStorage.getRuleKey()).orElse(null);
if (ruleOrTemplateDefinition == null) {
// The rule is not known among our loaded analyzers, so return it untouched, to let calling code take appropriate decision
return possiblyDeprecatedActiveRuleFromStorage;
}
return new ServerActiveRule(ruleOrTemplateDefinition.getKey(), possiblyDeprecatedActiveRuleFromStorage.getSeverity(), possiblyDeprecatedActiveRuleFromStorage.getParams(),
null);
}
}

private static boolean isRuleFromExtraPlugin(Language ruleLanguage, ConnectedGlobalConfiguration config) {
return config.getExtraPluginsPathsByKey().keySet()
.stream().anyMatch(extraPluginKey -> ruleLanguage.getLanguageKey().equals(extraPluginKey));
Expand Down Expand Up @@ -351,26 +386,28 @@ private void restartAnalysisEngine() {

@Override
public CompletableFuture<ConnectedRuleDetails> getActiveRuleDetails(EndpointParams endpoint, HttpClient client, String ruleKey, @Nullable String projectKey) {
var allRulesDefinitionsByKey = analysisContext.get().allRulesDefinitionsByKey;
var ruleDefFromPlugin = allRulesDefinitionsByKey.get(ruleKey);
if (ruleDefFromPlugin != null && (globalConfig.getExtraPluginsPathsByKey().containsKey(ruleDefFromPlugin.getLanguage().getPluginKey()) || projectKey == null)) {
// if no project key, or for rules from extra plugins there will be no rules metadata in the storage
return CompletableFuture.completedFuture(
new DefaultRuleDetails(ruleKey, ruleDefFromPlugin.getName(), ruleDefFromPlugin.getHtmlDescription(), ruleDefFromPlugin.getSeverity(), ruleDefFromPlugin.getType(),
ruleDefFromPlugin.getLanguage(), ""));
var ruleDefFromPluginOpt = analysisContext.get().findRule(ruleKey);
if (ruleDefFromPluginOpt.isPresent()) {
var ruleDefFromPlugin = ruleDefFromPluginOpt.get();
if (globalConfig.getExtraPluginsPathsByKey().containsKey(ruleDefFromPlugin.getLanguage().getPluginKey()) || projectKey == null) {
// if no project key, or for rules from extra plugins there will be no rules metadata in the storage
return CompletableFuture.completedFuture(
new DefaultRuleDetails(ruleKey, ruleDefFromPlugin.getName(), ruleDefFromPlugin.getHtmlDescription(), ruleDefFromPlugin.getSeverity(), ruleDefFromPlugin.getType(),
ruleDefFromPlugin.getLanguage(), ""));
}
}
if (projectKey != null) {
var analyzerConfiguration = projectStorage.getAnalyzerConfiguration(projectKey);
var storageActiveRule = analyzerConfiguration.getRuleSetByLanguageKey().values().stream()
.flatMap(s -> s.getRules().stream())
.filter(r -> r.getRuleKey().equals(ruleKey)).findFirst();
.filter(r -> tryConvertDeprecatedKeys(r).getRuleKey().equals(ruleKey)).findFirst();
if (storageActiveRule.isPresent()) {
var activeRuleFromStorage = storageActiveRule.get();
var serverSeverity = activeRuleFromStorage.getSeverity();
if (StringUtils.isNotBlank(activeRuleFromStorage.getTemplateKey())) {
var templateRuleDefFromPlugin = Optional.ofNullable(allRulesDefinitionsByKey.get(activeRuleFromStorage.getTemplateKey()))
var templateRuleDefFromPlugin = analysisContext.get().findRule(activeRuleFromStorage.getTemplateKey())
.orElseThrow(() -> new IllegalStateException("Unable to find rule definition for rule template " + activeRuleFromStorage.getTemplateKey()));
return new ServerApi(new ServerApiHelper(endpoint, client)).rules().getRule(ruleKey)
return new ServerApi(new ServerApiHelper(endpoint, client)).rules().getRule(activeRuleFromStorage.getRuleKey())
.thenApply(
serverRule -> new DefaultRuleDetails(
ruleKey,
Expand All @@ -381,10 +418,8 @@ public CompletableFuture<ConnectedRuleDetails> getActiveRuleDetails(EndpointPara
templateRuleDefFromPlugin.getLanguage(),
serverRule.getHtmlNote()));
} else {
if (ruleDefFromPlugin == null) {
throw new IllegalStateException("Unable to find rule definition for rule " + ruleKey);
}
return new ServerApi(new ServerApiHelper(endpoint, client)).rules().getRule(ruleKey)
var ruleDefFromPlugin = ruleDefFromPluginOpt.orElseThrow(() -> new IllegalStateException("Unable to find rule definition for rule '" + ruleKey + "'"));
return new ServerApi(new ServerApiHelper(endpoint, client)).rules().getRule(activeRuleFromStorage.getRuleKey())
.thenApply(serverRule -> new DefaultRuleDetails(ruleKey, ruleDefFromPlugin.getName(), ruleDefFromPlugin.getHtmlDescription(),
serverSeverity != null ? serverSeverity : ruleDefFromPlugin.getSeverity(), ruleDefFromPlugin.getType(), ruleDefFromPlugin.getLanguage(),
serverRule.getHtmlNote()));
Expand Down Expand Up @@ -540,12 +575,16 @@ private <T> T wrapErrors(Supplier<T> callable) {
private static class AnalysisContext {
private final Collection<PluginDetails> pluginDetails;
private final Map<String, SonarLintRuleDefinition> allRulesDefinitionsByKey;
private final Map<String, String> deprecatedRuleKeysMapping;
private final AnalysisEngine analysisEngine;

public AnalysisContext(List<PluginDetails> pluginDetails, Map<String, SonarLintRuleDefinition> allRulesDefinitionsByKey, AnalysisEngine analysisEngine) {
this.pluginDetails = pluginDetails;
this.allRulesDefinitionsByKey = allRulesDefinitionsByKey;
this.analysisEngine = analysisEngine;
this.deprecatedRuleKeysMapping = allRulesDefinitionsByKey.values().stream()
.flatMap(r -> r.getDeprecatedKeys().stream().map(dk -> Map.entry(dk, r.getKey())))
.collect(toMap(Map.Entry::getKey, Map.Entry::getValue));
}

public void destroy() {
Expand All @@ -555,6 +594,14 @@ public void destroy() {
public void finishGracefully() {
analysisEngine.finishGracefully();
}

public Optional<SonarLintRuleDefinition> findRule(String ruleKey) {
if (deprecatedRuleKeysMapping.containsKey(ruleKey)) {
return Optional.of(allRulesDefinitionsByKey.get(deprecatedRuleKeysMapping.get(ruleKey)));
}
return Optional.ofNullable(allRulesDefinitionsByKey.get(ruleKey));
}

}

}
91 changes: 90 additions & 1 deletion ...st/java/org/sonarsource/sonarlint/core/mediumtest/ConnectedEmbeddedPluginMediumTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,46 @@
*/
package org.sonarsource.sonarlint.core.mediumtest;

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.api.io.TempDir;
import org.sonarqube.ws.Rules;
import org.sonarqube.ws.Rules.Rule;
import org.sonarsource.sonarlint.core.ConnectedSonarLintEngineImpl;
import org.sonarsource.sonarlint.core.MockWebServerExtensionWithProtobuf;
import org.sonarsource.sonarlint.core.NodeJsHelper;
import org.sonarsource.sonarlint.core.analysis.api.ClientInputFile;
import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue;
import org.sonarsource.sonarlint.core.client.api.connected.ConnectedAnalysisConfiguration;
import org.sonarsource.sonarlint.core.client.api.connected.ConnectedGlobalConfiguration;
import org.sonarsource.sonarlint.core.client.api.connected.ConnectedRuleDetails;
import org.sonarsource.sonarlint.core.commons.Language;
import org.sonarsource.sonarlint.core.mediumtest.ConnectedIssueMediumTests.StoreIssueListener;
import org.sonarsource.sonarlint.core.mediumtest.fixtures.ProjectStorageFixture;
import testutils.PluginLocator;
import testutils.TestUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.tuple;
import static org.sonarsource.sonarlint.core.commons.testutils.MockWebServerExtension.httpClient;
import static org.sonarsource.sonarlint.core.mediumtest.fixtures.StorageFixture.newStorage;

class ConnectedEmbeddedPluginMediumTests {

@RegisterExtension
private final MockWebServerExtensionWithProtobuf mockWebServerExtension = new MockWebServerExtensionWithProtobuf();

private static final String SERVER_ID = StringUtils.repeat("very-long-id", 30);
private static final String JAVA_MODULE_KEY = "test-project-2";
private static ConnectedSonarLintEngineImpl sonarlint;
Expand All @@ -47,8 +69,15 @@ static void prepare(@TempDir Path slHome) throws Exception {
.withJSPlugin()
.withJavaPlugin()
.withProject("test-project")
.withProject(JAVA_MODULE_KEY)
.withProject("stale_module", ProjectStorageFixture.ProjectStorageBuilder::stale)
.withProject(JAVA_MODULE_KEY, project -> project
.withRuleSet("java", ruleSet -> ruleSet
// Emulate server returning a deprecated key for local analyzer
.withActiveRule("squid:S106", "BLOCKER")
// Emulate server returning a deprecated template key
.withCustomActiveRule("squid:myCustomRule", "squid:S124", "MAJOR", Map.of("message", "Needs to be reviewed", "regularExpression", ".*REVIEW.*"))
.withActiveRule("java:S1220", "MINOR")
.withActiveRule("java:S1481", "BLOCKER")))
.create(slHome);

var nodeJsHelper = new NodeJsHelper();
Expand Down Expand Up @@ -100,4 +129,64 @@ void rule_description_come_from_embedded() throws Exception {
+ "</ul>");
}

/**
* SLCORE-365
* The server is only aware of rules squid:S106 and squid:myCustomRule (that is a custom rule based on template rule squid:S124)
* The embedded analyzer is only aware of rules java:S106 and java:S124
*/
@Test
void convert_deprecated_keys_from_server_for_rules_and_templates(@TempDir Path baseDir) throws Exception {
var inputFile = prepareJavaInputFile(baseDir);

final List<Issue> issues = new ArrayList<>();
sonarlint.analyze(ConnectedAnalysisConfiguration.builder()
.setProjectKey(JAVA_MODULE_KEY)
.setBaseDir(baseDir)
.addInputFile(inputFile)
.setModuleKey("key")
.build(),
new StoreIssueListener(issues), null, null);

// Reported issues will refers to new rule keys
assertThat(issues).extracting("ruleKey", "startLine", "inputFile.path", "severity").containsOnly(
tuple("java:S106", 4, inputFile.getPath(), "BLOCKER"),
tuple("java:myCustomRule", 5, inputFile.getPath(), "MAJOR"),
tuple("java:S1220", null, inputFile.getPath(), "MINOR"),
tuple("java:S1481", 3, inputFile.getPath(), "BLOCKER"));

// Requests to the server should be made using deprecated rule keys
mockWebServerExtension.addProtobufResponse("/api/rules/show.protobuf?key=squid:S106",
Rules.ShowResponse.newBuilder().setRule(Rule.newBuilder().setHtmlNote("S106 Extended rule description")).build());
mockWebServerExtension.addProtobufResponse("/api/rules/show.protobuf?key=squid:myCustomRule",
Rules.ShowResponse.newBuilder().setRule(Rule.newBuilder().setHtmlDesc("My custom rule template desc").setHtmlNote("My custom rule extended description")).build());

ConnectedRuleDetails s106RuleDetails = sonarlint.getActiveRuleDetails(mockWebServerExtension.endpointParams(), httpClient(), "java:S106", JAVA_MODULE_KEY).get();
assertThat(s106RuleDetails.getSeverity()).isEqualTo("BLOCKER");
assertThat(s106RuleDetails.getHtmlDescription()).contains("<p>When logging a message there are several important requirements");
assertThat(s106RuleDetails.getExtendedDescription()).isEqualTo("S106 Extended rule description");

ConnectedRuleDetails myCustomRuleDetails = sonarlint.getActiveRuleDetails(mockWebServerExtension.endpointParams(), httpClient(), "java:myCustomRule", JAVA_MODULE_KEY).get();
assertThat(myCustomRuleDetails.getSeverity()).isEqualTo("MAJOR");
assertThat(myCustomRuleDetails.getHtmlDescription()).isEqualTo("My custom rule template desc");
assertThat(myCustomRuleDetails.getExtendedDescription()).isEqualTo("My custom rule extended description");
}

private ClientInputFile prepareJavaInputFile(Path baseDir) throws IOException {
return prepareInputFile(baseDir, "Foo.java",
"public class Foo {\n"
+ " public void foo() {\n"
+ " int x;\n"
+ " System.out.println(\"Foo\");\n"
+ " // TO REVIEW\n"
+ " }\n"
+ "}",
false);
}

private ClientInputFile prepareInputFile(Path baseDir, String relativePath, String content, final boolean isTest) throws IOException {
final var file = new File(baseDir.toFile(), relativePath);
FileUtils.write(file, content, StandardCharsets.UTF_8);
return TestUtils.createInputFile(file.toPath(), relativePath, isTest);
}

}
Loading

0 comments on commit ff48688

Please sign in to comment.