SONAR-17287 Fix quality issues and adopt changes to API

3 files changed, 55 insertions, 24 deletions

diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/AbstractDefaultIssue.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/AbstractDefaultIssue.java
index 2fa3235e7c5..cc77d90a02f 100644
--- a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/AbstractDefaultIssue.java
+++ b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/AbstractDefaultIssue.java
@@ -34,7 +34,7 @@ import org.sonar.api.batch.sensor.internal.DefaultStorable;
 import org.sonar.api.batch.sensor.internal.SensorStorage;
 import org.sonar.api.batch.sensor.issue.Issue.Flow;
 import org.sonar.api.batch.sensor.issue.IssueLocation;
-import org.sonar.api.batch.sensor.issue.NewIssue;
+import org.sonar.api.batch.sensor.issue.NewIssue.FlowType;
 import org.sonar.api.batch.sensor.issue.NewIssueLocation;
 import org.sonar.api.utils.PathUtils;
 
@@ -72,25 +72,21 @@ public abstract class AbstractDefaultIssue<T extends AbstractDefaultIssue> exten
   }
 
   public T addLocation(NewIssueLocation secondaryLocation) {
-    flows.add(new DefaultIssueFlow(List.of(rewriteLocation((DefaultIssueLocation) secondaryLocation)), DefaultIssueFlow.Type.UNDEFINED, null));
+    flows.add(new DefaultIssueFlow(List.of(rewriteLocation((DefaultIssueLocation) secondaryLocation)), FlowType.UNDEFINED, null));
     return (T) this;
   }
 
   public T addFlow(Iterable<NewIssueLocation> locations) {
-    return addFlow(locations, DefaultIssueFlow.Type.UNDEFINED, null);
+    return addFlow(locations, FlowType.UNDEFINED, null);
   }
 
-  public T addFlow(Iterable<NewIssueLocation> flowLocations, NewIssue.FlowType flowType, @Nullable String flowDescription) {
-    return addFlow(flowLocations, DefaultIssueFlow.Type.valueOf(flowType.name()), flowDescription);
-  }
-
-  private T addFlow(Iterable<NewIssueLocation> locations, DefaultIssueFlow.Type type, @Nullable String description) {
+  public T addFlow(Iterable<NewIssueLocation> flowLocations, FlowType type, @Nullable String flowDescription) {
     checkArgument(type != null, "Type can't be null");
     List<IssueLocation> flowAsList = new ArrayList<>();
-    for (NewIssueLocation issueLocation : locations) {
+    for (NewIssueLocation issueLocation : flowLocations) {
       flowAsList.add(rewriteLocation((DefaultIssueLocation) issueLocation));
     }
-    flows.add(new DefaultIssueFlow(flowAsList, type, description));
+    flows.add(new DefaultIssueFlow(flowAsList, type, flowDescription));
     return (T) this;
   }
 
diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueFlow.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueFlow.java
index e4aabcc85da..76a47751c51 100644
--- a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueFlow.java
+++ b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueFlow.java
@@ -24,14 +24,15 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonar.api.batch.sensor.issue.Issue;
 import org.sonar.api.batch.sensor.issue.IssueLocation;
+import org.sonar.api.batch.sensor.issue.NewIssue.FlowType;
 
 public class DefaultIssueFlow implements Issue.Flow {
   private final List<IssueLocation> locations;
-  private final Type type;
+  private final FlowType type;
   @Nullable
   private final String description;
 
-  public DefaultIssueFlow(List<IssueLocation> locations, Type type, @Nullable String description) {
+  public DefaultIssueFlow(List<IssueLocation> locations, FlowType type, @Nullable String description) {
     this.locations = locations;
     this.type = type;
     this.description = description;
@@ -42,16 +43,14 @@ public class DefaultIssueFlow implements Issue.Flow {
     return locations;
   }
 
-  public Type getType() {
+  @Override
+  public FlowType type() {
     return type;
   }
 
   @CheckForNull
-  public String getDescription() {
+  @Override
+  public String description() {
     return description;
   }
-
-  public enum Type {
-    UNDEFINED, DATA, EXECUTION;
-  }
 }
diff --git a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java
index 515380b0e25..1f874295e48 100644
--- a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java
+++ b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java
@@ -37,10 +37,14 @@ import org.sonar.api.batch.fs.internal.DefaultTextRange;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
 import org.sonar.api.batch.rule.Severity;
 import org.sonar.api.batch.sensor.internal.SensorStorage;
+import org.sonar.api.batch.sensor.issue.Issue;
+import org.sonar.api.batch.sensor.issue.Issue.Flow;
 import org.sonar.api.batch.sensor.issue.NewIssue;
+import org.sonar.api.batch.sensor.issue.NewIssue.FlowType;
 import org.sonar.api.rule.RuleKey;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.tuple;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -94,16 +98,35 @@ public class DefaultIssueTest {
 
     DefaultIssue issue = new DefaultIssue(project, storage)
       .at(new DefaultIssueLocation().on(inputFile))
-      .addFlow(List.of(new DefaultIssueLocation().message("loc1").on(inputFile)), NewIssue.FlowType.DATA, "desc")
+      .addFlow(List.of(new DefaultIssueLocation().message("loc1").on(inputFile)), FlowType.DATA, "desc")
       .addFlow(List.of(new DefaultIssueLocation().message("loc1").on(inputFile).at(range1), new DefaultIssueLocation().message("loc1").on(inputFile).at(range2)))
       .forRule(RULE_KEY);
 
-    assertThat(issue.flows)
-      .extracting(DefaultIssueFlow::getType, DefaultIssueFlow::getDescription)
-      .containsExactly(tuple(DefaultIssueFlow.Type.DATA, "desc"), tuple(DefaultIssueFlow.Type.UNDEFINED, null));
+    assertThat(issue.flows())
+      .extracting(Flow::type, Flow::description)
+      .containsExactly(tuple(FlowType.DATA, "desc"), tuple(FlowType.UNDEFINED, null));
 
-    assertThat(issue.flows.get(0).locations()).hasSize(1);
-    assertThat(issue.flows.get(1).locations()).hasSize(2);
+    assertThat(issue.flows().get(0).locations()).hasSize(1);
+    assertThat(issue.flows().get(1).locations()).hasSize(2);
+  }
+
+  @Test
+  public void build_issue_with_secondary_locations() {
+    TextRange range1 = new DefaultTextRange(new DefaultTextPointer(1, 1), new DefaultTextPointer(1, 2));
+    TextRange range2 = new DefaultTextRange(new DefaultTextPointer(2, 1), new DefaultTextPointer(2, 2));
+
+    DefaultIssue issue = new DefaultIssue(project, storage)
+      .at(new DefaultIssueLocation().on(inputFile))
+      .addLocation(new DefaultIssueLocation().on(inputFile).at(range1).message("loc1"))
+      .addLocation(new DefaultIssueLocation().on(inputFile).at(range2).message("loc2"))
+      .forRule(RULE_KEY);
+
+    assertThat(issue.flows())
+      .extracting(Flow::type, Flow::description)
+      .containsExactly(tuple(FlowType.UNDEFINED, null), tuple(FlowType.UNDEFINED, null));
+
+    assertThat(issue.flows().get(0).locations()).hasSize(1);
+    assertThat(issue.flows().get(1).locations()).hasSize(1);
   }
 
   @Test
@@ -178,6 +201,19 @@ public class DefaultIssueTest {
   }
 
   @Test
+  public void at_fails_if_called_twice() {
+    DefaultIssueLocation loc = new DefaultIssueLocation().on(inputFile);
+    DefaultIssue issue = new DefaultIssue(project, storage).at(loc);
+    assertThatThrownBy(() -> issue.at(loc)).isInstanceOf(IllegalStateException.class);
+  }
+
+  @Test
+  public void at_fails_if_location_is_null() {
+    DefaultIssue issue = new DefaultIssue(project, storage);
+    assertThatThrownBy(() -> issue.at(null)).isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
   public void default_issue_has_no_quickfix() {
     DefaultIssue issue = new DefaultIssue(project, storage);