Skip to content

Commit

Permalink
SLCORE-999 Store custom impacts when synchronizing rules
Browse files Browse the repository at this point in the history
  • Loading branch information
eray-felek-sonarsource committed Oct 30, 2024
1 parent 950bf9b commit 576c7a1
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -418,18 +418,16 @@ private ServerActiveRule tryConvertDeprecatedKeys(String connectionId, ServerAct
var ruleKeyPossiblyWithDeprecatedRepo = RuleKey.parse(possiblyDeprecatedActiveRuleFromStorage.getRuleKey());
var templateRuleKeyWithCorrectRepo = RuleKey.parse(ruleOrTemplateDefinition.getKey());
var ruleKey = new RuleKey(templateRuleKeyWithCorrectRepo.repository(), ruleKeyPossiblyWithDeprecatedRepo.rule()).toString();
//TODO Replace empty list with proper one.
return new ServerActiveRule(ruleKey, possiblyDeprecatedActiveRuleFromStorage.getSeverity(), possiblyDeprecatedActiveRuleFromStorage.getParams(),
ruleOrTemplateDefinition.getKey(), Collections.emptyList());
ruleOrTemplateDefinition.getKey(), possiblyDeprecatedActiveRuleFromStorage.getOverriddenImpacts());
} else {
ruleOrTemplateDefinition = rulesRepository.getRule(connectionId, 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;
}
//TODO Replace empty list with proper one.
return new ServerActiveRule(ruleOrTemplateDefinition.getKey(), possiblyDeprecatedActiveRuleFromStorage.getSeverity(), possiblyDeprecatedActiveRuleFromStorage.getParams(),
null, Collections.emptyList());
null, possiblyDeprecatedActiveRuleFromStorage.getOverriddenImpacts());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand All @@ -29,16 +30,17 @@
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import org.jetbrains.annotations.NotNull;
import org.sonarsource.sonarlint.core.commons.VulnerabilityProbability;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.StandaloneRuleConfigDto;
import org.sonarsource.sonarlint.core.commons.CleanCodeAttribute;
import org.sonarsource.sonarlint.core.commons.ImpactSeverity;
import org.sonarsource.sonarlint.core.commons.IssueSeverity;
import org.sonarsource.sonarlint.core.commons.api.SonarLanguage;
import org.sonarsource.sonarlint.core.commons.RuleType;
import org.sonarsource.sonarlint.core.commons.SoftwareQuality;
import org.sonarsource.sonarlint.core.commons.VulnerabilityProbability;
import org.sonarsource.sonarlint.core.commons.api.SonarLanguage;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.StandaloneRuleConfigDto;
import org.sonarsource.sonarlint.core.rule.extractor.SonarLintRuleDefinition;
import org.sonarsource.sonarlint.core.rule.extractor.SonarLintRuleParamDefinition;
import org.sonarsource.sonarlint.core.serverapi.push.parsing.common.ImpactPayload;
import org.sonarsource.sonarlint.core.serverapi.rules.ServerActiveRule;
import org.sonarsource.sonarlint.core.serverapi.rules.ServerRule;

Expand All @@ -54,14 +56,14 @@ public class RuleDetails {
private final IssueSeverity defaultSeverity;
private final RuleType type;
private final CleanCodeAttribute cleanCodeAttribute;
private final Map<SoftwareQuality, ImpactSeverity> defaultImpacts;
private final Map<SoftwareQuality, ImpactSeverity> impacts;
private final Collection<EffectiveRuleParam> params;
private final String extendedDescription;
private final Set<String> educationPrincipleKeys;
private final VulnerabilityProbability vulnerabilityProbability;

public RuleDetails(String key, SonarLanguage language, String name, String htmlDescription, Map<String, List<DescriptionSection>> descriptionSectionsByKey,
IssueSeverity defaultSeverity, RuleType type, @Nullable CleanCodeAttribute cleanCodeAttribute, Map<SoftwareQuality, ImpactSeverity> defaultImpacts,
IssueSeverity defaultSeverity, RuleType type, @Nullable CleanCodeAttribute cleanCodeAttribute, Map<SoftwareQuality, ImpactSeverity> impacts,
@Nullable String extendedDescription, Collection<EffectiveRuleParam> params, Set<String> educationPrincipleKeys,
@Nullable VulnerabilityProbability vulnerabilityProbability) {
this.key = key;
Expand All @@ -72,7 +74,7 @@ public RuleDetails(String key, SonarLanguage language, String name, String htmlD
this.defaultSeverity = defaultSeverity;
this.type = type;
this.cleanCodeAttribute = cleanCodeAttribute;
this.defaultImpacts = defaultImpacts;
this.impacts = impacts;
this.params = params;
this.extendedDescription = extendedDescription;
this.educationPrincipleKeys = educationPrincipleKeys;
Expand Down Expand Up @@ -147,11 +149,24 @@ public static RuleDetails merging(ServerActiveRule activeRuleFromStorage, Server
serverRule.getSeverity(),
templateRuleDefFromPlugin.getType(),
cleanCodeAttribute,
defaultImpacts,
mergeImpacts(defaultImpacts, activeRuleFromStorage.getOverriddenImpacts()),
serverRule.getHtmlNote(),
Collections.emptyList(), templateRuleDefFromPlugin.getEducationPrincipleKeys(), templateRuleDefFromPlugin.getVulnerabilityProbability().orElse(null));
}

public static Map<SoftwareQuality, ImpactSeverity> mergeImpacts(Map<SoftwareQuality, ImpactSeverity> defaultImpacts,
List<ImpactPayload> overriddenImpacts) {
Map<SoftwareQuality, ImpactSeverity> mergedImpacts = new HashMap<>(defaultImpacts);

for (ImpactPayload impact : overriddenImpacts) {
var quality = SoftwareQuality.valueOf(impact.getSoftwareQuality());
var severity = ImpactSeverity.valueOf(impact.getSeverity());
mergedImpacts.computeIfPresent(quality, (k, v) -> severity);
}

return Collections.unmodifiableMap(mergedImpacts);
}

public String getKey() {
return key;
}
Expand Down Expand Up @@ -192,8 +207,8 @@ public Optional<CleanCodeAttribute> getCleanCodeAttribute() {
return Optional.ofNullable(cleanCodeAttribute);
}

public Map<SoftwareQuality, ImpactSeverity> getDefaultImpacts() {
return defaultImpacts;
public Map<SoftwareQuality, ImpactSeverity> getImpacts() {
return impacts;
}

public Collection<EffectiveRuleParam> getParams() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@
import org.sonarsource.sonarlint.core.analysis.api.TextEdit;
import org.sonarsource.sonarlint.core.commons.RuleType;
import org.sonarsource.sonarlint.core.commons.api.SonarLanguage;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.FileEditDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.IssueFlowDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.IssueLocationDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.QuickFixDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.TextEditDto;
import org.sonarsource.sonarlint.core.rpc.protocol.common.Either;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.EffectiveRuleDetailsDto;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.EffectiveRuleParamDto;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.ImpactDto;
Expand All @@ -53,8 +47,14 @@
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.RuleNonContextualSectionDto;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.RuleSplitDescriptionDto;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.VulnerabilityProbability;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.FileEditDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.IssueFlowDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.IssueLocationDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.QuickFixDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.TextEditDto;
import org.sonarsource.sonarlint.core.rpc.protocol.common.CleanCodeAttribute;
import org.sonarsource.sonarlint.core.rpc.protocol.common.CleanCodeAttributeCategory;
import org.sonarsource.sonarlint.core.rpc.protocol.common.Either;
import org.sonarsource.sonarlint.core.rpc.protocol.common.ImpactSeverity;
import org.sonarsource.sonarlint.core.rpc.protocol.common.IssueSeverity;
import org.sonarsource.sonarlint.core.rpc.protocol.common.Language;
Expand Down Expand Up @@ -85,7 +85,7 @@ public static EffectiveRuleDetailsDto transform(RuleDetails ruleDetails, @Nullab
adapt(ruleDetails.getType()),
ruleDetails.getCleanCodeAttribute().map(RuleDetailsAdapter::adapt).orElse(null),
ruleDetails.getCleanCodeAttribute().map(org.sonarsource.sonarlint.core.commons.CleanCodeAttribute::getAttributeCategory).map(RuleDetailsAdapter::adapt).orElse(null),
toDto(ruleDetails.getDefaultImpacts()),
toDto(ruleDetails.getImpacts()),
transformDescriptions(ruleDetails, contextKey),
transform(ruleDetails.getParams()),
adapt(ruleDetails.getLanguage()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,19 +230,17 @@ private ServerActiveRule tryConvertDeprecatedKeys(ServerActiveRule possiblyDepre
var ruleKeyPossiblyWithDeprecatedRepo = RuleKey.parse(possiblyDeprecatedActiveRuleFromStorage.getRuleKey());
var templateRuleKeyWithCorrectRepo = RuleKey.parse(ruleOrTemplateDefinition.get().getKey());
var ruleKey = new RuleKey(templateRuleKeyWithCorrectRepo.repository(), ruleKeyPossiblyWithDeprecatedRepo.rule()).toString();
//TODO Replace empty list with proper one.
return new ServerActiveRule(ruleKey, possiblyDeprecatedActiveRuleFromStorage.getSeverity(), possiblyDeprecatedActiveRuleFromStorage.getParams(),
ruleOrTemplateDefinition.get().getKey(), Collections.emptyList());
ruleOrTemplateDefinition.get().getKey(), possiblyDeprecatedActiveRuleFromStorage.getOverriddenImpacts());
} else {
ruleOrTemplateDefinition = rulesRepository.getRule(connectionId, possiblyDeprecatedActiveRuleFromStorage.getRuleKey());
if (ruleOrTemplateDefinition.isEmpty()) {
// The rule is not known among our loaded analyzers, so return it untouched, to let calling code take appropriate decision
return possiblyDeprecatedActiveRuleFromStorage;
}
//TODO Replace empty list with proper one.
return new ServerActiveRule(ruleOrTemplateDefinition.get().getKey(), possiblyDeprecatedActiveRuleFromStorage.getSeverity(),
possiblyDeprecatedActiveRuleFromStorage.getParams(),
null, Collections.emptyList());
null, possiblyDeprecatedActiveRuleFromStorage.getOverriddenImpacts());
}
}

Expand Down Expand Up @@ -425,7 +423,8 @@ public RuleDetailsForAnalysis getRuleDetailsForConnectedAnalysis(Binding binding
}
var ruleDefinition = ruleDefinitionOpt.get();
return new RuleDetailsForAnalysis(activeRule.getSeverity(), ruleDefinition.getType(),
ruleDefinition.getCleanCodeAttribute().orElse(CONVENTIONAL), ruleDefinition.getDefaultImpacts(),
ruleDefinition.getCleanCodeAttribute().orElse(CONVENTIONAL),
RuleDetails.mergeImpacts(ruleDefinition.getDefaultImpacts(), activeRule.getOverriddenImpacts()),
ruleDefinition.getVulnerabilityProbability().orElse(null));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@
import java.util.Map;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.sonarsource.sonarlint.core.commons.ImpactSeverity;
import org.sonarsource.sonarlint.core.commons.SoftwareQuality;
import org.sonarsource.sonarlint.core.repository.config.ConfigurationRepository;
import org.sonarsource.sonarlint.core.repository.rules.RulesRepository;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.rules.RuleDefinitionDto;
import org.sonarsource.sonarlint.core.serverapi.push.parsing.common.ImpactPayload;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.tuple;
Expand Down Expand Up @@ -58,4 +61,32 @@ void it_should_return_all_embedded_rules_from_the_repository() {
.containsExactly(tuple("repo:ruleKey", "ruleName"));
}

@Test
void it_should_only_override_overridden_impact_quality() {
Map<SoftwareQuality, ImpactSeverity> defaultImpacts = Map.of(
SoftwareQuality.MAINTAINABILITY, ImpactSeverity.LOW,
SoftwareQuality.RELIABILITY, ImpactSeverity.MEDIUM
);

List<ImpactPayload> overriddenImpacts = List.of(
new ImpactPayload("MAINTAINABILITY", "HIGH")
);

Map<SoftwareQuality, ImpactSeverity> result = RuleDetails.mergeImpacts(defaultImpacts, overriddenImpacts);
assertThat(result).containsEntry(SoftwareQuality.MAINTAINABILITY, ImpactSeverity.HIGH);
assertThat(result).containsEntry(SoftwareQuality.RELIABILITY, ImpactSeverity.MEDIUM);
}

@Test
void it_should_work_when_no_overridden_impacts() {
Map<SoftwareQuality, ImpactSeverity> defaultImpacts = Map.of(
SoftwareQuality.MAINTAINABILITY, ImpactSeverity.LOW,
SoftwareQuality.RELIABILITY, ImpactSeverity.MEDIUM
);

Map<SoftwareQuality, ImpactSeverity> result = RuleDetails.mergeImpacts(defaultImpacts, List.of());

assertThat(result).isEqualTo(defaultImpacts);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.sonarsource.sonarlint.core.commons.CleanCodeAttribute;
import org.sonarsource.sonarlint.core.commons.ImpactSeverity;
import org.sonarsource.sonarlint.core.commons.IssueSeverity;
Expand All @@ -40,6 +41,7 @@
import org.sonarsource.sonarlint.core.serverapi.ServerApiHelper;
import org.sonarsource.sonarlint.core.serverapi.UrlUtils;
import org.sonarsource.sonarlint.core.serverapi.proto.sonarqube.ws.Rules;
import org.sonarsource.sonarlint.core.serverapi.push.parsing.common.ImpactPayload;

import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
Expand Down Expand Up @@ -120,8 +122,9 @@ public Collection<ServerActiveRule> getAllActiveRules(String qualityProfileKey,
IssueSeverity.valueOf(ar.getSeverity()),
ar.getParamsList().stream().collect(toMap(Rules.Active.Param::getKey, Rules.Active.Param::getValue)),
ruleTemplatesByRuleKey.get(ruleKey),
//TODO Pass the right value
Collections.emptyList()));
ar.getImpacts().getImpactsList().stream()
.map(impact -> new ImpactPayload(impact.getSoftwareQuality().toString(), impact.getSeverity().name()))
.collect(Collectors.toList())));

},
false,
Expand Down
5 changes: 5 additions & 0 deletions backend/server-api/src/main/proto/sonarqube/ws-rules.proto
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,14 @@ message Active {
optional string qProfile = 1;
optional string severity = 3;
repeated Param params = 5;
optional Impacts impacts = 9;

message Param {
optional string key = 1;
optional string value = 2;
}

message Impacts {
repeated sonarqube.ws.commons.Impact impacts = 1;
}
}

0 comments on commit 576c7a1

Please sign in to comment.