diff options
5 files changed, 50 insertions, 85 deletions
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java index 4da88839471..3e220cc6328 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java @@ -156,7 +156,8 @@ public class TrackerRawInputFactory { private DefaultIssue toIssue(LineHashSequence lineHashSeq, ScannerReport.Issue reportIssue) { DefaultIssue issue = new DefaultIssue(); init(issue); - issue.setRuleKey(RuleKey.of(reportIssue.getRuleRepository(), reportIssue.getRuleKey())); + RuleKey ruleKey = RuleKey.of(reportIssue.getRuleRepository(), reportIssue.getRuleKey()); + issue.setRuleKey(ruleKey); if (reportIssue.hasTextRange()) { int startLine = reportIssue.getTextRange().getStartLine(); issue.setLine(startLine); @@ -166,6 +167,9 @@ public class TrackerRawInputFactory { } if (isNotEmpty(reportIssue.getMsg())) { issue.setMessage(reportIssue.getMsg()); + } else { + Rule rule = ruleRepository.getByKey(ruleKey); + issue.setMessage(rule.getName()); } if (reportIssue.getSeverity() != Severity.UNSET_SEVERITY) { issue.setSeverity(reportIssue.getSeverity().name()); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java index bfc19904810..8b675a0c37e 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java @@ -139,6 +139,33 @@ public class TrackerRawInputFactoryTest { assertThat(issue.effort()).isNull(); } + @Test + public void set_rule_name_as_message_when_issue_message_from_report_is_empty() { + RuleKey ruleKey = RuleKey.of("java", "S001"); + markRuleAsActive(ruleKey); + registerRule(ruleKey, "Rule 1"); + when(issueFilter.accept(any(), eq(FILE))).thenReturn(true); + when(sourceLinesHash.getLineHashesMatchingDBVersion(FILE)).thenReturn(Collections.singletonList("line")); + ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() + .setRuleRepository(ruleKey.repository()) + .setRuleKey(ruleKey.rule()) + .setMsg("") + .build(); + reportReader.putIssues(FILE.getReportAttributes().getRef(), singletonList(reportIssue)); + Input<DefaultIssue> input = underTest.create(FILE); + + Collection<DefaultIssue> issues = input.getIssues(); + assertThat(issues).hasSize(1); + DefaultIssue issue = Iterators.getOnlyElement(issues.iterator()); + + // fields set by analysis report + assertThat(issue.ruleKey()).isEqualTo(ruleKey); + + // fields set by compute engine + assertInitializedIssue(issue); + assertThat(issue.message()).isEqualTo("Rule 1"); + } + // SONAR-10781 @Test public void load_issues_from_report_missing_secondary_location_component() { @@ -369,4 +396,12 @@ public class TrackerRawInputFactoryTest { private void markRuleAsActive(RuleKey ruleKey) { activeRulesHolder.put(new ActiveRule(ruleKey, Severity.CRITICAL, emptyMap(), 1_000L, null, "qp1")); } + + private void registerRule(RuleKey ruleKey, String name) { + DumbRule dumbRule = new DumbRule(ruleKey); + dumbRule.setName(name); + ruleRepository.add(dumbRule); + } + + } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/ModuleIssues.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/ModuleIssues.java index a0b8974443d..cd07af80188 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/ModuleIssues.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/ModuleIssues.java @@ -19,7 +19,6 @@ */ package org.sonar.scanner.issue; -import com.google.common.base.Strings; import java.util.Collection; import java.util.function.Consumer; import javax.annotation.concurrent.ThreadSafe; @@ -27,19 +26,17 @@ import org.sonar.api.batch.fs.TextRange; import org.sonar.api.batch.fs.internal.DefaultInputComponent; import org.sonar.api.batch.rule.ActiveRule; import org.sonar.api.batch.rule.ActiveRules; -import org.sonar.api.batch.rule.Rule; -import org.sonar.api.batch.rule.Rules; import org.sonar.api.batch.sensor.issue.ExternalIssue; import org.sonar.api.batch.sensor.issue.Issue; import org.sonar.api.batch.sensor.issue.Issue.Flow; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.utils.MessageException; import org.sonar.scanner.protocol.Constants.Severity; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReport.IssueLocation; import org.sonar.scanner.protocol.output.ScannerReport.IssueType; import org.sonar.scanner.report.ReportPublisher; +import static com.google.common.base.Strings.nullToEmpty; + /** * Initialize the issues raised during scan. */ @@ -47,13 +44,11 @@ import org.sonar.scanner.report.ReportPublisher; public class ModuleIssues { private final ActiveRules activeRules; - private final Rules rules; private final IssueFilters filters; private final ReportPublisher reportPublisher; - public ModuleIssues(ActiveRules activeRules, Rules rules, IssueFilters filters, ReportPublisher reportPublisher) { + public ModuleIssues(ActiveRules activeRules, IssueFilters filters, ReportPublisher reportPublisher) { this.activeRules = activeRules; - this.rules = rules; this.filters = filters; this.reportPublisher = reportPublisher; } @@ -61,14 +56,13 @@ public class ModuleIssues { public boolean initAndAddIssue(Issue issue) { DefaultInputComponent inputComponent = (DefaultInputComponent) issue.primaryLocation().inputComponent(); - Rule rule = validateRule(issue); ActiveRule activeRule = activeRules.find(issue.ruleKey()); if (activeRule == null) { // rule does not exist or is not enabled -> ignore the issue return false; } - ScannerReport.Issue rawIssue = createReportIssue(issue, inputComponent.batchId(), rule.name(), activeRule.severity()); + ScannerReport.Issue rawIssue = createReportIssue(issue, inputComponent.batchId(), activeRule.severity()); if (filters.accept(inputComponent.key(), rawIssue)) { write(inputComponent.batchId(), rawIssue); @@ -83,8 +77,8 @@ public class ModuleIssues { write(inputComponent.batchId(), rawExternalIssue); } - private static ScannerReport.Issue createReportIssue(Issue issue, int componentRef, String ruleName, String activeRuleSeverity) { - String primaryMessage = Strings.isNullOrEmpty(issue.primaryLocation().message()) ? ruleName : issue.primaryLocation().message(); + private static ScannerReport.Issue createReportIssue(Issue issue, int componentRef, String activeRuleSeverity) { + String primaryMessage = nullToEmpty(issue.primaryLocation().message()); org.sonar.api.batch.rule.Severity overriddenSeverity = issue.overriddenSeverity(); Severity severity = overriddenSeverity != null ? Severity.valueOf(overriddenSeverity.name()) : Severity.valueOf(activeRuleSeverity); @@ -176,18 +170,6 @@ public class ModuleIssues { return textRangeBuilder.build(); } - private Rule validateRule(Issue issue) { - RuleKey ruleKey = issue.ruleKey(); - Rule rule = rules.find(ruleKey); - if (rule == null) { - throw MessageException.of(String.format("The rule '%s' does not exist.", ruleKey)); - } - if (Strings.isNullOrEmpty(rule.name()) && Strings.isNullOrEmpty(issue.primaryLocation().message())) { - throw MessageException.of(String.format("The rule '%s' has no name and the related issue has no message.", ruleKey)); - } - return rule; - } - public void write(int batchId, ScannerReport.Issue rawIssue) { reportPublisher.getWriter().appendComponentIssue(batchId, rawIssue); } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/ModuleIssuesTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/ModuleIssuesTest.java index 0353fe23282..cf22300454f 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/ModuleIssuesTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/ModuleIssuesTest.java @@ -68,39 +68,6 @@ public class ModuleIssuesTest { ReportPublisher reportPublisher = mock(ReportPublisher.class, RETURNS_DEEP_STUBS); @Test - public void fail_on_unknown_rule() { - initModuleIssues(); - DefaultIssue issue = new DefaultIssue() - .at(new DefaultIssueLocation().on(file).at(file.selectLine(3)).message("Foo")) - .forRule(SQUID_RULE_KEY); - try { - moduleIssues.initAndAddIssue(issue); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(MessageException.class); - } - - verifyZeroInteractions(reportPublisher); - } - - @Test - public void fail_if_rule_has_no_name_and_issue_has_no_message() { - ruleBuilder.add(SQUID_RULE_KEY).setInternalKey(SQUID_RULE_KEY.rule()); - initModuleIssues(); - DefaultIssue issue = new DefaultIssue() - .at(new DefaultIssueLocation().on(file).at(file.selectLine(3)).message("")) - .forRule(SQUID_RULE_KEY); - try { - moduleIssues.initAndAddIssue(issue); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(MessageException.class); - } - - verifyZeroInteractions(reportPublisher); - } - - @Test public void ignore_null_active_rule() { ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); initModuleIssues(); @@ -193,30 +160,6 @@ public class ModuleIssuesTest { } @Test - public void use_rule_name_if_no_message() { - ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); - activeRulesBuilder.addRule(new NewActiveRule.Builder() - .setRuleKey(SQUID_RULE_KEY) - .setSeverity(Severity.INFO) - .setName(SQUID_RULE_NAME) - .setQProfileKey("qp-1") - .build()); - initModuleIssues(); - - DefaultIssue issue = new DefaultIssue() - .at(new DefaultIssueLocation().on(file).at(file.selectLine(3)).message("")) - .forRule(SQUID_RULE_KEY); - when(filters.accept(anyString(), any(ScannerReport.Issue.class))).thenReturn(true); - - boolean added = moduleIssues.initAndAddIssue(issue); - - assertThat(added).isTrue(); - ArgumentCaptor<ScannerReport.Issue> argument = ArgumentCaptor.forClass(ScannerReport.Issue.class); - verify(reportPublisher.getWriter()).appendComponentIssue(eq(file.batchId()), argument.capture()); - assertThat(argument.getValue().getMsg()).isEqualTo("Avoid Cycle"); - } - - @Test public void filter_issue() { ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); activeRulesBuilder.addRule(new NewActiveRule.Builder() @@ -242,7 +185,7 @@ public class ModuleIssuesTest { * Every rules and active rules has to be added in builders before creating ModuleIssues */ private void initModuleIssues() { - moduleIssues = new ModuleIssues(activeRulesBuilder.build(), ruleBuilder.build(), filters, reportPublisher); + moduleIssues = new ModuleIssues(activeRulesBuilder.build(), filters, reportPublisher); } } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/ChecksMediumTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/ChecksMediumTest.java index 89149482bbb..30db74ece2e 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/ChecksMediumTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/ChecksMediumTest.java @@ -79,11 +79,12 @@ public class ChecksMediumTest { .execute(); List<Issue> issues = result.issuesFor(result.inputFile("src/sample.xoo")); + // If the message is the rule name. it's set by the CE. See SONAR-11531 assertThat(issues) .extracting("msg", "textRange.startLine") .containsOnly( - tuple("A template rule", 1), - tuple("Another template rule", 2)); + tuple("", 1), + tuple("", 2)); } |