SONAR-6116 Validate line is correct when creating issue on file

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java
index 585ebcf647a57b7db7b460eeaf41cc202567ed69..280502d23b7209e2ef004f6a0ab59f959543c706 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java
@@ -19,14 +19,13 @@
  */
 package org.sonar.api.batch.sensor.issue.internal;
 
-import org.sonar.api.batch.sensor.internal.SensorStorage;
-
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import org.sonar.api.batch.fs.InputDir;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.InputPath;
 import org.sonar.api.batch.sensor.internal.DefaultStorable;
+import org.sonar.api.batch.sensor.internal.SensorStorage;
 import org.sonar.api.batch.sensor.issue.Issue;
 import org.sonar.api.batch.sensor.issue.NewIssue;
 import org.sonar.api.rule.RuleKey;
@@ -94,7 +93,10 @@ public class DefaultIssue extends DefaultStorable implements Issue, NewIssue {
 
   @Override
   public DefaultIssue atLine(int line) {
-    Preconditions.checkState(this.path != null && this.path instanceof InputFile, "atLine should be called after onFile");
+    Preconditions.checkState(this.path != null && this.path instanceof InputFile, "atLine should be called after onFile.");
+    Preconditions.checkArgument(line > 0, "line starts at 1, invalid value " + line + ".");
+    int lines = ((InputFile) path).lines();
+    Preconditions.checkArgument(line <= lines, "File " + path + " has " + lines + " lines. Unable to create issue at line " + line + ".");
     this.line = line;
     return this;
   }

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java
index 86be3bd947fdc3f41d953e6bec2fa845f53baf70..098266cff6f25c43388163b52efc160be15fd7e9 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java
@@ -19,13 +19,15 @@
  */
 package org.sonar.api.batch.sensor.issue.internal;
 
-import org.sonar.api.batch.sensor.internal.SensorStorage;
-
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.sonar.api.batch.fs.internal.DefaultInputDir;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
+import org.sonar.api.batch.sensor.internal.SensorStorage;
+import org.sonar.api.batch.sensor.issue.Issue.Severity;
 import org.sonar.api.rule.RuleKey;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -39,7 +41,7 @@ public class DefaultIssueTest {
   public void build_file_issue() {
     SensorStorage storage = mock(SensorStorage.class);
     DefaultIssue issue = new DefaultIssue(storage)
-      .onFile(new DefaultInputFile("foo", "src/Foo.php"))
+      .onFile(new DefaultInputFile("foo", "src/Foo.php").setLines(3))
       .forRule(RuleKey.of("repo", "rule"))
       .atLine(1)
       .effortToFix(10.0)
@@ -56,6 +58,25 @@ public class DefaultIssueTest {
     verify(storage).store(issue);
   }
 
+  @Test
+  public void build_directory_issue() {
+    SensorStorage storage = mock(SensorStorage.class);
+    DefaultIssue issue = new DefaultIssue(storage)
+      .onDir(new DefaultInputDir("foo", "src"))
+      .forRule(RuleKey.of("repo", "rule"))
+      .overrideSeverity(Severity.BLOCKER)
+      .message("Wrong way!");
+
+    assertThat(issue.inputPath()).isEqualTo(new DefaultInputDir("foo", "src"));
+    assertThat(issue.ruleKey()).isEqualTo(RuleKey.of("repo", "rule"));
+    assertThat(issue.message()).isEqualTo("Wrong way!");
+    assertThat(issue.overridenSeverity()).isEqualTo(Severity.BLOCKER);
+
+    issue.save();
+
+    verify(storage).store(issue);
+  }
+
   @Test
   public void build_project_issue() {
     SensorStorage storage = mock(SensorStorage.class);
@@ -89,4 +110,24 @@ public class DefaultIssueTest {
       .message("Wrong way!");
   }
 
+  @Test
+  public void line_is_positive() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("line starts at 1, invalid value 0.");
+    new DefaultIssue()
+      .onFile(new DefaultInputFile("foo", "src/Foo.php").setLines(3))
+      .forRule(RuleKey.of("repo", "rule"))
+      .atLine(0);
+  }
+
+  @Test
+  public void not_allowed_to_create_issues_on_unexisting_line() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("File [moduleKey=foo, relative=src/Foo.php, basedir=null] has 3 lines. Unable to create issue at line 5.");
+    new DefaultIssue()
+      .onFile(new DefaultInputFile("foo", "src/Foo.php").setLines(3))
+      .forRule(RuleKey.of("repo", "rule"))
+      .atLine(5);
+  }
+
 }