From ac73ac876fae3d027b9ffc83a79b8d324d409a04 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 5 May 2016 10:15:04 +0200 Subject: [PATCH] SONAR-7595 Issues are not closed in some block move conditions --- .../core/issue/tracking/BlockRecognizer.java | 28 ++++++++------- .../sonar/core/issue/tracking/Trackable.java | 3 ++ .../sonar/core/issue/tracking/Tracking.java | 6 ++-- .../core/issue/tracking/TrackerTest.java | 35 ++++++++++++++++++- .../issue/tracking/ServerIssueFromWs.java | 7 ++-- 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 { } } - // Check if remaining number of lines exceeds threshold - if (basesByLine.keySet().size() * rawsByLine.keySet().size() < 250000) { - List 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 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 { } 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> one = new ArrayDeque>();", + " private final Deque> two = new ArrayDeque>();", + " private final Deque three = new ArrayDeque();", + " private final Deque>> four = new ArrayDeque>();"); + 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> one = new ArrayDeque>();", + " private final Deque> two = new ArrayDeque<>();", + " private final Deque three = new ArrayDeque<>();", + " private final Deque>> four = new ArrayDeque<>();"); + Issue raw1 = rawInput.createIssueOnLine(3, RULE_USE_DIAMOND, "Use diamond"); + + Tracking 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()) : ""; } } -- 2.39.5