SONAR-7595 Issues are not closed in some block move conditions

5 files changed, 60 insertions, 19 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java
index abda1ae3bf5..c7570f9f67c 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java
@@ -75,21 +75,23 @@ class BlockRecognizer<RAW extends Trackable, BASE extends Trackable> {
       }
     }
 
-    // Check if remaining number of lines exceeds threshold
-    if (basesByLine.keySet().size() * rawsByLine.keySet().size() < 250000) {
-      List<LinePair> possibleLinePairs = Lists.newArrayList();
-      for (Integer baseLine : basesByLine.keySet()) {
-        for (Integer rawLine : rawsByLine.keySet()) {
-          int weight = lengthOfMaximalBlock(baseInput.getLineHashSequence(), baseLine, rawInput.getLineHashSequence(), rawLine);
-          possibleLinePairs.add(new LinePair(baseLine, rawLine, weight));
-        }
-      }
-      Collections.sort(possibleLinePairs, LinePairComparator.INSTANCE);
-      for (LinePair linePair : possibleLinePairs) {
-        // High probability that baseLine has been moved to rawLine, so we can map all Issues on baseLine to all Issues on rawLine
-        map(rawsByLine.get(linePair.rawLine), basesByLine.get(linePair.baseLine), tracking);
+    // Check if remaining number of lines exceeds threshold. It avoids processing too many combinations.
+    if (basesByLine.keySet().size() * rawsByLine.keySet().size() >= 250_000) {
+      return;
+    }
+
+    List<LinePair> possibleLinePairs = Lists.newArrayList();
+    for (Integer baseLine : basesByLine.keySet()) {
+      for (Integer rawLine : rawsByLine.keySet()) {
+        int weight = lengthOfMaximalBlock(baseInput.getLineHashSequence(), baseLine, rawInput.getLineHashSequence(), rawLine);
+        possibleLinePairs.add(new LinePair(baseLine, rawLine, weight));
       }
     }
+    Collections.sort(possibleLinePairs, LinePairComparator.INSTANCE);
+    for (LinePair linePair : possibleLinePairs) {
+      // High probability that baseLine has been moved to rawLine, so we can map all issues on baseLine to all issues on rawLine
+      map(rawsByLine.get(linePair.rawLine), basesByLine.get(linePair.baseLine), tracking);
+    }
   }
 
   /**
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java
index 1c5d6f067e0..33e3351254b 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java
@@ -31,6 +31,9 @@ public interface Trackable {
   @CheckForNull
   Integer getLine();
 
+  /**
+   * Trimmed message of issue
+   */
   String getMessage();
 
   @CheckForNull
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java
index 214bf7afc79..2f7e326a5c6 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java
@@ -88,8 +88,10 @@ public class Tracking<RAW extends Trackable, BASE extends Trackable> {
   }
 
   void match(RAW raw, BASE base) {
-    rawToBase.put(raw, base);
-    baseToRaw.put(base, raw);
+    if (!rawToBase.containsKey(raw)) {
+      rawToBase.put(raw, base);
+      baseToRaw.put(base, raw);
+    }
   }
 
   boolean isComplete() {
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java
index b55d2fdd8b8..ef87a41dcfb 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java
@@ -30,6 +30,7 @@ import org.junit.rules.ExpectedException;
 import org.sonar.api.rule.RuleKey;
 
 import static java.util.Arrays.asList;
+import static org.apache.commons.lang.StringUtils.trim;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class TrackerTest {
@@ -38,6 +39,8 @@ public class TrackerTest {
   public static final RuleKey RULE_UNUSED_LOCAL_VARIABLE = RuleKey.of("java", "UnusedLocalVariable");
   public static final RuleKey RULE_UNUSED_PRIVATE_METHOD = RuleKey.of("java", "UnusedPrivateMethod");
   public static final RuleKey RULE_NOT_DESIGNED_FOR_EXTENSION = RuleKey.of("java", "NotDesignedForExtension");
+  public static final RuleKey RULE_USE_DIAMOND = RuleKey.of("java", "UseDiamond");
+
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
@@ -348,6 +351,36 @@ public class TrackerTest {
     assertThat(tracking.getUnmatchedBases()).containsOnly(base2);
   }
 
+  /**
+   * https://jira.sonarsource.com/browse/SONAR-7595
+   */
+  @Test
+  public void match_only_one_issue_when_multiple_blocks_match_the_same_block() {
+    FakeInput baseInput = FakeInput.createForSourceLines(
+      "public class Toto {",
+      "  private final Deque<Set<DataItem>> one = new ArrayDeque<Set<DataItem>>();",
+      "  private final Deque<Set<DataItem>> two = new ArrayDeque<Set<DataItem>>();",
+      "  private final Deque<Integer> three = new ArrayDeque<Integer>();",
+      "  private final Deque<Set<Set<DataItem>>> four = new ArrayDeque<Set<DataItem>>();");
+    Issue base1 = baseInput.createIssueOnLine(2, RULE_USE_DIAMOND, "Use diamond");
+    baseInput.createIssueOnLine(3, RULE_USE_DIAMOND, "Use diamond");
+    baseInput.createIssueOnLine(4, RULE_USE_DIAMOND, "Use diamond");
+    baseInput.createIssueOnLine(5, RULE_USE_DIAMOND, "Use diamond");
+
+    FakeInput rawInput = FakeInput.createForSourceLines(
+      "public class Toto {",
+      "  // move all lines",
+      "  private final Deque<Set<DataItem>> one = new ArrayDeque<Set<DataItem>>();",
+      "  private final Deque<Set<DataItem>> two = new ArrayDeque<>();",
+      "  private final Deque<Integer> three = new ArrayDeque<>();",
+      "  private final Deque<Set<Set<DataItem>>> four = new ArrayDeque<>();");
+    Issue raw1 = rawInput.createIssueOnLine(3, RULE_USE_DIAMOND, "Use diamond");
+
+    Tracking<Issue, Issue> tracking = tracker.track(rawInput, baseInput);
+    assertThat(tracking.getUnmatchedBases()).hasSize(3);
+    assertThat(tracking.baseFor(raw1)).isEqualTo(base1);
+  }
+
   private static class Issue implements Trackable {
     private final RuleKey ruleKey;
     private final Integer line;
@@ -357,7 +390,7 @@ public class TrackerTest {
       this.line = line;
       this.lineHash = lineHash;
       this.ruleKey = ruleKey;
-      this.message = message;
+      this.message = trim(message);
     }
 
     @Override
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/ServerIssueFromWs.java b/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/ServerIssueFromWs.java
index 7898d5c88eb..03937e63028 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/ServerIssueFromWs.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/ServerIssueFromWs.java
@@ -20,9 +20,10 @@
 package org.sonar.batch.issue.tracking;
 
 import javax.annotation.CheckForNull;
-
-import org.sonar.core.issue.tracking.Trackable;
 import org.sonar.api.rule.RuleKey;
+import org.sonar.core.issue.tracking.Trackable;
+
+import static org.apache.commons.lang.StringUtils.trim;
 
 public class ServerIssueFromWs implements Trackable {
 
@@ -59,7 +60,7 @@ public class ServerIssueFromWs implements Trackable {
 
   @Override
   public String getMessage() {
-    return dto.hasMsg() ? dto.getMsg() : "";
+    return dto.hasMsg() ? trim(dto.getMsg()) : "";
   }
 
 }