From 9dfee8d62d2ecbd021148a9695c8fac508db5a92 Mon Sep 17 00:00:00 2001 From: Nicolas QUINQUENEL Date: Mon, 11 Nov 2024 14:58:52 +0100 Subject: [PATCH] SLCORE-1023 Parsing impact severity with INFO and BLOCKER should work (#1155) --- .../core/commons/ImpactSeverity.java | 12 +++++- .../sonarlint/core/rules/RuleDetails.java | 12 +----- .../core/serverapi/rules/RulesApi.java | 4 +- .../TaintIssueDownloader.java | 2 +- .../TaintIssueDownloaderTests.java | 42 +++++++++---------- 5 files changed, 35 insertions(+), 37 deletions(-) diff --git a/backend/commons/src/main/java/org/sonarsource/sonarlint/core/commons/ImpactSeverity.java b/backend/commons/src/main/java/org/sonarsource/sonarlint/core/commons/ImpactSeverity.java index 493fada7a1..ef57920783 100644 --- a/backend/commons/src/main/java/org/sonarsource/sonarlint/core/commons/ImpactSeverity.java +++ b/backend/commons/src/main/java/org/sonarsource/sonarlint/core/commons/ImpactSeverity.java @@ -24,5 +24,15 @@ public enum ImpactSeverity { LOW, MEDIUM, HIGH, - BLOCKER + BLOCKER; + + public static ImpactSeverity mapSeverity(String severity) { + if ("BLOCKER".equals(severity) || "ImpactSeverity_BLOCKER".equals(severity)) { + return ImpactSeverity.BLOCKER; + } else if ("INFO".equals(severity) || "ImpactSeverity_INFO".equals(severity)) { + return ImpactSeverity.INFO; + } else { + return ImpactSeverity.valueOf(severity); + } + } } diff --git a/backend/core/src/main/java/org/sonarsource/sonarlint/core/rules/RuleDetails.java b/backend/core/src/main/java/org/sonarsource/sonarlint/core/rules/RuleDetails.java index 02abed07e0..0690e7bb70 100644 --- a/backend/core/src/main/java/org/sonarsource/sonarlint/core/rules/RuleDetails.java +++ b/backend/core/src/main/java/org/sonarsource/sonarlint/core/rules/RuleDetails.java @@ -163,23 +163,13 @@ public static Map mergeImpacts(Map severity); } return Collections.unmodifiableMap(mergedImpacts); } - private static ImpactSeverity mapSeverity(String severity) { - if ("BLOCKER".equals(severity) || "ImpactSeverity_BLOCKER".equals(severity)) { - return ImpactSeverity.BLOCKER; - } else if ("INFO".equals(severity) || "ImpactSeverity_INFO".equals(severity)) { - return ImpactSeverity.INFO; - } else { - return ImpactSeverity.valueOf(severity); - } - } - public static RuleDetails merging(RuleDetails serverActiveRuleDetails, RaisedFindingDto raisedFindingDto) { var isMQRMode = raisedFindingDto.getSeverityMode().isRight(); var softwareImpacts = new EnumMap(SoftwareQuality.class); diff --git a/backend/server-api/src/main/java/org/sonarsource/sonarlint/core/serverapi/rules/RulesApi.java b/backend/server-api/src/main/java/org/sonarsource/sonarlint/core/serverapi/rules/RulesApi.java index d15b52b96d..b952fb90eb 100644 --- a/backend/server-api/src/main/java/org/sonarsource/sonarlint/core/serverapi/rules/RulesApi.java +++ b/backend/server-api/src/main/java/org/sonarsource/sonarlint/core/serverapi/rules/RulesApi.java @@ -77,7 +77,7 @@ public Optional getRule(String ruleKey, SonarLintCancelMonitor cance var cleanCodeAttribute = Enums.getIfPresent(CleanCodeAttribute.class, rule.getCleanCodeAttribute().name()).orNull(); var impacts = rule.getImpacts().getImpactsList().stream().collect(toMap( impact -> SoftwareQuality.valueOf(impact.getSoftwareQuality().name()), - impact -> ImpactSeverity.valueOf(impact.getSeverity().name()))); + impact -> ImpactSeverity.mapSeverity(impact.getSeverity().name()))); return Optional.of(new ServerRule(rule.getName(), IssueSeverity.valueOf(rule.getSeverity()), RuleType.valueOf(rule.getType().name()), rule.getLang(), rule.getHtmlDesc(), convertDescriptionSections(rule), rule.getHtmlNote(), Set.copyOf(rule.getEducationPrinciples().getEducationPrinciplesList()), cleanCodeAttribute, impacts)); @@ -123,7 +123,7 @@ public Collection getAllActiveRules(String qualityProfileKey, ar.getParamsList().stream().collect(toMap(Rules.Active.Param::getKey, Rules.Active.Param::getValue)), ruleTemplatesByRuleKey.get(ruleKey), ar.getImpacts().getImpactsList().stream() - .map(impact -> new ImpactPayload(impact.getSoftwareQuality().toString(), impact.getSeverity().name())) + .map(impact -> new ImpactPayload(impact.getSoftwareQuality().toString(), ImpactSeverity.mapSeverity(impact.getSeverity().name()).name())) .collect(Collectors.toList()))); }, diff --git a/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloader.java b/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloader.java index eb1bbabf6b..e68338a60a 100644 --- a/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloader.java +++ b/backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloader.java @@ -177,7 +177,7 @@ static ImpactSeverity parseProtoImpactSeverity(Common.Impact protoImpact) { if (!protoImpact.hasSeverity() || protoImpact.getSeverity() == Common.ImpactSeverity.UNKNOWN_IMPACT_SEVERITY) { throw new IllegalArgumentException("Unknown or missing impact severity"); } - return ImpactSeverity.valueOf(protoImpact.getSeverity().name()); + return ImpactSeverity.mapSeverity(protoImpact.getSeverity().name()); } private static List convertFlows(SourceApi sourceApi, List flowsList, Map componentPathsByKey, diff --git a/backend/server-connection/src/test/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloaderTests.java b/backend/server-connection/src/test/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloaderTests.java index 867664332b..ac20e497e5 100644 --- a/backend/server-connection/src/test/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloaderTests.java +++ b/backend/server-connection/src/test/java/org/sonarsource/sonarlint/core/serverconnection/TaintIssueDownloaderTests.java @@ -350,45 +350,43 @@ void parse_clean_code_attribute_from_lite_stream_unknown() { assertThat(cleanCodeAttribute).isNull(); } - void parse_software_quality() { - var impact = Common.Impact.newBuilder().setSoftwareQuality(Common.SoftwareQuality.SECURITY).build(); + @Test + void parse_software_quality_and_impact_severity() { + var impact = Common.Impact.newBuilder().setSoftwareQuality(Common.SoftwareQuality.SECURITY).setSeverity(Common.ImpactSeverity.MEDIUM).build(); assertThat(parseProtoSoftwareQuality(impact)).isEqualTo(SoftwareQuality.SECURITY); + assertThat(parseProtoImpactSeverity(impact)).isEqualTo(ImpactSeverity.MEDIUM); } - void parse_software_quality_missing() { - var impact = Common.Impact.newBuilder().build(); - assertThatThrownBy(() -> parseProtoSoftwareQuality(impact)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Unknown or missing software quality"); + @Test + void parse_software_quality_and_impact_severity_info() { + var impact = Common.Impact.newBuilder().setSoftwareQuality(Common.SoftwareQuality.SECURITY).setSeverity(Common.ImpactSeverity.ImpactSeverity_INFO).build(); + assertThat(parseProtoSoftwareQuality(impact)).isEqualTo(SoftwareQuality.SECURITY); + assertThat(parseProtoImpactSeverity(impact)).isEqualTo(ImpactSeverity.INFO); + } + + @Test + void parse_software_quality_and_impact_severity_blocker() { + var impact = Common.Impact.newBuilder().setSoftwareQuality(Common.SoftwareQuality.SECURITY).setSeverity(Common.ImpactSeverity.ImpactSeverity_BLOCKER).build(); + assertThat(parseProtoSoftwareQuality(impact)).isEqualTo(SoftwareQuality.SECURITY); + assertThat(parseProtoImpactSeverity(impact)).isEqualTo(ImpactSeverity.BLOCKER); } + @Test void parse_software_quality_unknown() { - var impact = Common.Impact.newBuilder().setSoftwareQuality(Common.SoftwareQuality.UNKNOWN_IMPACT_QUALITY).build(); + var impact = Common.Impact.newBuilder().setSoftwareQuality(Common.SoftwareQuality.UNKNOWN_IMPACT_QUALITY).setSeverity(Common.ImpactSeverity.HIGH).build(); assertThatThrownBy(() -> parseProtoSoftwareQuality(impact)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Unknown or missing software quality"); } - void parse_impact_severity() { - var impact = Common.Impact.newBuilder().setSeverity(Common.ImpactSeverity.LOW).build(); - assertThat(parseProtoImpactSeverity(impact)).isEqualTo(ImpactSeverity.LOW); - } - - void parse_impact_severity_missing() { - var impact = Common.Impact.newBuilder().build(); - assertThatThrownBy(() -> parseProtoImpactSeverity(impact)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Unknown or missing impact severity"); - } - + @Test void parse_impact_severity_unknown() { - var impact = Common.Impact.newBuilder().setSeverity(Common.ImpactSeverity.UNKNOWN_IMPACT_SEVERITY).build(); + var impact = Common.Impact.newBuilder().setSeverity(Common.ImpactSeverity.UNKNOWN_IMPACT_SEVERITY).setSoftwareQuality(Common.SoftwareQuality.MAINTAINABILITY).build(); assertThatThrownBy(() -> parseProtoImpactSeverity(impact)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Unknown or missing impact severity"); } - private static void assertTextRange(@Nullable TextRangeWithHash textRangeWithHash, int startLine, int startLineOffset, int endLine, int endLineOffset, String hash) { assertThat(textRangeWithHash).isNotNull();