Fix quality issues

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssue.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssue.java
index ee68be1d242d6d15fde7bcdbcb672e399ec8fe70..9118a4a373cbbba267737df5fd30ef171390eb6c 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssue.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssue.java
@@ -72,6 +72,7 @@ public class DefaultExternalIssue extends AbstractDefaultIssue<DefaultExternalIs
     requireNonNull(this.ruleKey, "Rule key is mandatory on external issue");
     checkState(primaryLocation != null, "Primary location is mandatory on every external issue");
     checkState(primaryLocation.inputComponent().isFile(), "External issues must be located in files");
+    checkState(primaryLocation.message() != null, "External issues must have a message");
     checkState(severity != null, "Severity is mandatory on every external issue");
     checkState(type != null, "Type is mandatory on every external issue");
     storage.store(this);

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssueTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssueTest.java
index 079d4995f2b8cd32877a33e2cf4c5e56a9d6e88f..dc77fbc63fba9040eb36ca7b158542a43a0cc1e0 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssueTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultExternalIssueTest.java
@@ -100,6 +100,23 @@ public class DefaultExternalIssueTest {
     exception.expectMessage("External issues must be located in files");
     issue.save();
   }
+  
+  @Test
+  public void fail_to_store_if_primary_location_has_no_message() {
+    SensorStorage storage = mock(SensorStorage.class);
+    DefaultExternalIssue issue = new DefaultExternalIssue(storage)
+      .at(new DefaultIssueLocation()
+        .on(inputFile)
+        .at(inputFile.selectLine(1)))
+      .forRule(RuleKey.of("repo", "rule"))
+      .remediationEffortMinutes(10l)
+      .type(RuleType.BUG)
+      .severity(Severity.BLOCKER);
+
+    exception.expect(IllegalStateException.class);
+    exception.expectMessage("External issues must have a message");
+    issue.save();
+  }
 
   @Test
   public void fail_to_store_if_no_severity() {

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java
index b68fa4c910f7ba421089e0b855d979cf5e1dabbb..7f0d15a042060a0d8006e20d759a0e3942aeacb4 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java
@@ -55,28 +55,8 @@ public class ExternalIssueImporter {
     int issueCount = 0;
 
     for (Issue issue : report.issues) {
-      NewExternalIssue externalIssue = context.newExternalIssue()
-        .forRule(RuleKey.of(issue.engineId, issue.ruleId))
-        .severity(Severity.valueOf(issue.severity))
-        .remediationEffortMinutes(20L)
-        .type(RuleType.valueOf(issue.type));
-
-      NewIssueLocation primary = fillLocation(context, externalIssue.newLocation(), issue.primaryLocation);
-      if (primary != null) {
-        knownFiles.add(issue.primaryLocation.filePath);
-        externalIssue.at(primary);
-        if (issue.secondaryLocations != null) {
-          for (Location l : issue.secondaryLocations) {
-            NewIssueLocation secondary = fillLocation(context, externalIssue.newLocation(), l);
-            if (secondary != null) {
-              externalIssue.addLocation(secondary);
-            }
-          }
-        }
+      if (importIssue(issue)) {
         issueCount++;
-        externalIssue.save();
-      } else {
-        unknownFiles.add(issue.primaryLocation.filePath);
       }
     }
 
@@ -88,6 +68,33 @@ public class ExternalIssueImporter {
     }
   }
 
+  private boolean importIssue(Issue issue) {
+    NewExternalIssue externalIssue = context.newExternalIssue()
+      .forRule(RuleKey.of(issue.engineId, issue.ruleId))
+      .severity(Severity.valueOf(issue.severity))
+      .remediationEffortMinutes(20L)
+      .type(RuleType.valueOf(issue.type));
+
+    NewIssueLocation primary = fillLocation(context, externalIssue.newLocation(), issue.primaryLocation);
+    if (primary != null) {
+      knownFiles.add(issue.primaryLocation.filePath);
+      externalIssue.at(primary);
+      if (issue.secondaryLocations != null) {
+        for (Location l : issue.secondaryLocations) {
+          NewIssueLocation secondary = fillLocation(context, externalIssue.newLocation(), l);
+          if (secondary != null) {
+            externalIssue.addLocation(secondary);
+          }
+        }
+      }
+      externalIssue.save();
+      return true;
+    } else {
+      unknownFiles.add(issue.primaryLocation.filePath);
+      return false;
+    }
+  }
+
   private static String pluralize(String msg, int count) {
     if (count == 1) {
       return msg;
@@ -118,7 +125,7 @@ public class ExternalIssueImporter {
   }
 
   @CheckForNull
-  private InputFile findFile(SensorContext context, String filePath) {
+  private static InputFile findFile(SensorContext context, String filePath) {
     return context.fileSystem().inputFile(context.fileSystem().predicates().hasPath(filePath));
   }
 

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/package-info.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/package-info.java
new file mode 100644 (file)
index 0000000..e58ec06
--- /dev/null
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/package-info.java
@@ -0,0 +1,24 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2018 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+@ParametersAreNonnullByDefault
+package org.sonar.scanner.externalissue;
+
+import javax.annotation.ParametersAreNonnullByDefault;
+

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 1aeefadb7a670e13464828ea176082a3b82ebf84..d064b302f4808a4d320f0608a05674646fb56c13 100644 (file)
--- 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
@@ -112,10 +112,11 @@ public class ModuleIssues {
   }
 
   private static ScannerReport.ExternalIssue createReportExternalIssue(ExternalIssue issue, int componentRef) {
-    String primaryMessage = Strings.isNullOrEmpty(issue.primaryLocation().message()) ? issue.ruleKey().toString() : issue.primaryLocation().message();
+    // primary location of an external issue must have a message
+    String primaryMessage = issue.primaryLocation().message();
     Severity severity = Severity.valueOf(issue.severity().name());
     IssueType issueType = IssueType.valueOf(issue.type().name());
-    
+
     ScannerReport.ExternalIssue.Builder builder = ScannerReport.ExternalIssue.newBuilder();
     ScannerReport.IssueLocation.Builder locationBuilder = IssueLocation.newBuilder();
     ScannerReport.TextRange.Builder textRangeBuilder = ScannerReport.TextRange.newBuilder();