@@ -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()); |
@@ -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); | |||
} | |||
} |
@@ -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); | |||
} |
@@ -67,39 +67,6 @@ public class ModuleIssuesTest { | |||
DefaultInputFile file = new TestInputFileBuilder("foo", "src/Foo.php").initMetadata("Foo\nBar\nBiz\n").build(); | |||
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); | |||
@@ -192,30 +159,6 @@ public class ModuleIssuesTest { | |||
assertThat(argument.getValue().getSeverity()).isEqualTo(org.sonar.scanner.protocol.Constants.Severity.INFO); | |||
} | |||
@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); | |||
@@ -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); | |||
} | |||
} |
@@ -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)); | |||
} | |||