SONAR-4785 When a plugin creates an issue without a message, the issue message should...

diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
index 9e5d7f4e95f46ecc772f024cde6a90c5a6c2c84a..22f35ae38370abecc0e0475d4bfb39dd460f833e 100644 (file)
--- a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
@@ -19,12 +19,16 @@
  */
 package org.sonar.batch.issue;
 
+import com.google.common.base.Strings;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.profiles.RulesProfile;
 import org.sonar.api.resources.Project;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.ActiveRule;
+import org.sonar.api.rules.Rule;
+import org.sonar.api.rules.RuleFinder;
 import org.sonar.api.rules.Violation;
+import org.sonar.api.utils.MessageException;
 import org.sonar.batch.technicaldebt.TechnicalDebtCalculator;
 import org.sonar.core.issue.DefaultIssueBuilder;
 
@@ -40,13 +44,15 @@ public class ModuleIssues {
   private final Project project;
   private final IssueFilters filters;
   private final TechnicalDebtCalculator technicalDebtCalculator;
+  private final RuleFinder ruleFinder;
 
-  public ModuleIssues(RulesProfile qProfile, IssueCache cache, Project project, IssueFilters filters, TechnicalDebtCalculator technicalDebtCalculator) {
+  public ModuleIssues(RulesProfile qProfile, IssueCache cache, Project project, IssueFilters filters, TechnicalDebtCalculator technicalDebtCalculator, RuleFinder ruleFinder) {
     this.qProfile = qProfile;
     this.cache = cache;
     this.project = project;
     this.filters = filters;
     this.technicalDebtCalculator = technicalDebtCalculator;
+    this.ruleFinder = ruleFinder;
   }
 
   public boolean initAndAddIssue(DefaultIssue issue) {
@@ -70,13 +76,20 @@ public class ModuleIssues {
   }
 
   private boolean initAndAddIssue(DefaultIssue issue, @Nullable Violation violation) {
-    // TODO fail fast : if rule does not exist
-
-    ActiveRule activeRule = qProfile.getActiveRule(issue.ruleKey().repository(), issue.ruleKey().rule());
+    RuleKey ruleKey = issue.ruleKey();
+    Rule rule = ruleFinder.findByKey(ruleKey);
+    if (rule == null) {
+      throw MessageException.of(String.format("The rule '%s' does not exists.", ruleKey));
+    }
+    ActiveRule activeRule = qProfile.getActiveRule(ruleKey.repository(), ruleKey.rule());
     if (activeRule == null || activeRule.getRule() == null) {
       // rule does not exist or is not enabled -> ignore the issue
       return false;
     }
+
+    if (Strings.isNullOrEmpty(issue.message())) {
+      issue.setMessage(rule.getName());
+    }
     issue.setCreationDate(project.getAnalysisDate());
     issue.setUpdateDate(project.getAnalysisDate());
     if (issue.severity() == null) {
@@ -90,4 +103,5 @@ public class ModuleIssues {
     }
     return false;
   }
+
 }

diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java
index 2b3ae1c86d6af2eb1a4395119b03468fd73da3f4..712c49f80d85aa42829472eaf8429c03c4b7fa67 100644 (file)
--- a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java
@@ -22,7 +22,10 @@ package org.sonar.batch.issue;
 import org.apache.commons.lang.time.DateUtils;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.issue.internal.WorkDayDuration;
 import org.sonar.api.profiles.RulesProfile;
@@ -31,41 +34,72 @@ import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
-import org.sonar.api.rules.ActiveRule;
-import org.sonar.api.rules.Rule;
-import org.sonar.api.rules.RulePriority;
-import org.sonar.api.rules.Violation;
+import org.sonar.api.rules.*;
+import org.sonar.api.utils.MessageException;
 import org.sonar.batch.technicaldebt.TechnicalDebtCalculator;
 
 import java.util.Calendar;
 import java.util.Date;
 
 import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.*;
 
+@RunWith(MockitoJUnitRunner.class)
 public class ModuleIssuesTest {
 
   static final RuleKey SQUID_RULE_KEY = RuleKey.of("squid", "AvoidCycle");
 
-  IssueCache cache = mock(IssueCache.class);
-  RulesProfile qProfile = mock(RulesProfile.class);
-  Project project = mock(Project.class);
-  IssueFilters filters = mock(IssueFilters.class);
-  TechnicalDebtCalculator technicalDebtCalculator = mock(TechnicalDebtCalculator.class);
-  ModuleIssues moduleIssues = new ModuleIssues(qProfile, cache, project, filters, technicalDebtCalculator);
+  @Mock
+  IssueCache cache;
+
+  @Mock
+  RulesProfile qProfile;
+
+  @Mock
+  Project project;
+
+  @Mock
+  IssueFilters filters;
+
+  @Mock
+  TechnicalDebtCalculator technicalDebtCalculator;
+
+  @Mock
+  RuleFinder ruleFinder;
+
+  ModuleIssues moduleIssues;
 
   @Before
   public void setUp() {
     when(project.getAnalysisDate()).thenReturn(new Date());
     when(project.getEffectiveKey()).thenReturn("org.apache:struts-core");
+
+    moduleIssues = new ModuleIssues(qProfile, cache, project, filters, technicalDebtCalculator, ruleFinder);
+  }
+
+  @Test
+  public void fail_on_unknown_rule() throws Exception {
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(null);
+    DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY);
+
+    try {
+      moduleIssues.initAndAddIssue(issue);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(MessageException.class);
+    }
+
+    verifyZeroInteractions(cache);
   }
 
   @Test
   public void ignore_null_active_rule() throws Exception {
     when(qProfile.getActiveRule(anyString(), anyString())).thenReturn(null);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle"));
 
     DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY);
     boolean added = moduleIssues.initAndAddIssue(issue);
@@ -79,6 +113,7 @@ public class ModuleIssuesTest {
     ActiveRule activeRule = mock(ActiveRule.class);
     when(activeRule.getRule()).thenReturn(null);
     when(qProfile.getActiveRule(anyString(), anyString())).thenReturn(activeRule);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle"));
 
     DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY);
     boolean added = moduleIssues.initAndAddIssue(issue);
@@ -94,6 +129,7 @@ public class ModuleIssuesTest {
     when(activeRule.getRule()).thenReturn(rule);
     when(activeRule.getSeverity()).thenReturn(RulePriority.INFO);
     when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle"));
 
     Date analysisDate = new Date();
     when(project.getAnalysisDate()).thenReturn(analysisDate);
@@ -120,6 +156,7 @@ public class ModuleIssuesTest {
     when(activeRule.getRule()).thenReturn(rule);
     when(activeRule.getSeverity()).thenReturn(RulePriority.INFO);
     when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle"));
 
     Date analysisDate = new Date();
     when(project.getAnalysisDate()).thenReturn(analysisDate);
@@ -134,6 +171,33 @@ public class ModuleIssuesTest {
     assertThat(argument.getValue().creationDate()).isEqualTo(DateUtils.truncate(analysisDate, Calendar.SECOND));
   }
 
+  @Test
+  public void use_rule_name_if_no_message() throws Exception {
+    Rule rule = Rule.create("squid", "AvoidCycle");
+    ActiveRule activeRule = mock(ActiveRule.class);
+    when(activeRule.getRule()).thenReturn(rule);
+    when(activeRule.getSeverity()).thenReturn(RulePriority.INFO);
+    when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle").setName("Avoid Cycle"));
+
+    Date analysisDate = new Date();
+    when(project.getAnalysisDate()).thenReturn(analysisDate);
+
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      .setRuleKey(SQUID_RULE_KEY)
+      .setSeverity(Severity.CRITICAL)
+      .setMessage("");
+    when(filters.accept(issue, null)).thenReturn(true);
+
+    boolean added = moduleIssues.initAndAddIssue(issue);
+
+    assertThat(added).isTrue();
+    ArgumentCaptor<DefaultIssue> argument = ArgumentCaptor.forClass(DefaultIssue.class);
+    verify(cache).put(argument.capture());
+    assertThat(argument.getValue().message()).isEqualTo("Avoid Cycle");
+  }
+
   @Test
   public void add_deprecated_violation() throws Exception {
     Rule rule = Rule.create("squid", "AvoidCycle");
@@ -147,6 +211,7 @@ public class ModuleIssuesTest {
     when(activeRule.getRule()).thenReturn(rule);
     when(activeRule.getSeverity()).thenReturn(RulePriority.INFO);
     when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle"));
     when(filters.accept(any(DefaultIssue.class), eq(violation))).thenReturn(true);
 
     boolean added = moduleIssues.initAndAddViolation(violation);
@@ -170,6 +235,7 @@ public class ModuleIssuesTest {
     when(activeRule.getRule()).thenReturn(rule);
     when(activeRule.getSeverity()).thenReturn(RulePriority.INFO);
     when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle"));
 
     DefaultIssue issue = new DefaultIssue()
       .setKey("ABCDE")
@@ -191,6 +257,7 @@ public class ModuleIssuesTest {
     when(activeRule.getRule()).thenReturn(rule);
     when(activeRule.getSeverity()).thenReturn(RulePriority.INFO);
     when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule);
+    when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle"));
 
     Date analysisDate = new Date();
     when(project.getAnalysisDate()).thenReturn(analysisDate);