From 466427e0c3f5c5a5635f5525b85d3e4c41208d40 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 24 Mar 2016 12:31:43 +0100 Subject: [PATCH] SONAR-7472 Drop manual issues in issue tracking --- .../issue/IntegrateIssuesVisitor.java | 8 +- .../computation/issue/IssueLifecycle.java | 5 -- .../issue/IntegrateIssuesVisitorTest.java | 22 +---- .../computation/issue/IssueLifecycleTest.java | 9 --- .../sonar/core/issue/tracking/Tracker.java | 47 +---------- .../sonar/core/issue/tracking/Tracking.java | 20 ----- .../core/issue/tracking/TrackerTest.java | 81 ++----------------- .../issue/tracking/LocalIssueTracking.java | 18 ++--- 8 files changed, 17 insertions(+), 193 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IntegrateIssuesVisitor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IntegrateIssuesVisitor.java index a21617d5d51..1a1ca8f3f25 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IntegrateIssuesVisitor.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IntegrateIssuesVisitor.java @@ -43,7 +43,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { private final List componentIssues = new ArrayList<>(); public IntegrateIssuesVisitor(TrackerExecution tracker, IssueCache issueCache, IssueLifecycle issueLifecycle, IssueVisitors issueVisitors, - ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues, MutableComponentIssuesRepository componentIssuesRepository) { + ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues, MutableComponentIssuesRepository componentIssuesRepository) { super(CrawlerDepthLimit.FILE, POST_ORDER); this.tracker = tracker; this.issueCache = issueCache; @@ -91,12 +91,6 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { issueLifecycle.mergeExistingOpenIssue(raw, base); process(component, raw, cacheAppender); } - for (Map.Entry entry : tracking.getOpenManualIssuesByLine().entries()) { - Integer line = entry.getKey(); - DefaultIssue manualIssue = entry.getValue(); - issueLifecycle.moveOpenManualIssue(manualIssue, line); - process(component, manualIssue, cacheAppender); - } } private void closeUnmatchedBaseIssues(Component component, Tracking tracking, DiskCache.DiskAppender cacheAppender) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java index 2acca806baa..caba1df318d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java @@ -21,7 +21,6 @@ package org.sonar.server.computation.issue; import com.google.common.annotations.VisibleForTesting; import java.util.Date; -import javax.annotation.Nullable; import org.sonar.api.issue.Issue; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; @@ -96,10 +95,6 @@ public class IssueLifecycle { raw.setSelectedAt(base.selectedAt()); } - public void moveOpenManualIssue(DefaultIssue manualIssue, @Nullable Integer toLine) { - updater.setLine(manualIssue, toLine); - } - public void doAutomaticTransition(DefaultIssue issue) { workflow.doAutomaticTransition(issue, changeContext); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java index 83b89c79d96..8e4f582da52 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java @@ -110,7 +110,7 @@ public class IntegrateIssuesVisitorTest { IssueLifecycle issueLifecycle = mock(IssueLifecycle.class); IssueVisitor issueVisitor = mock(IssueVisitor.class); - IssueVisitors issueVisitors = new IssueVisitors(new IssueVisitor[]{issueVisitor}); + IssueVisitors issueVisitors = new IssueVisitors(new IssueVisitor[] {issueVisitor}); ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues = new ComponentsWithUnprocessedIssues(); TypeAwareVisitor underTest; @@ -182,26 +182,6 @@ public class IntegrateIssuesVisitorTest { assertThat(componentsWithUnprocessedIssues.getUuids()).isEmpty(); } - @Test - public void process_manual_issue() throws Exception { - componentsWithUnprocessedIssues.setUuids(newHashSet(FILE_UUID)); - - RuleKey ruleKey = RuleKey.of(RuleKey.MANUAL_REPOSITORY_KEY, "architecture"); - addBaseIssue(ruleKey); - - underTest.visitAny(FILE); - - verify(issueLifecycle).moveOpenManualIssue(defaultIssueCaptor.capture(), eq((Integer) null)); - assertThat(defaultIssueCaptor.getValue().ruleKey()).isEqualTo(ruleKey); - - verify(issueLifecycle).doAutomaticTransition(defaultIssueCaptor.capture()); - assertThat(defaultIssueCaptor.getValue().ruleKey()).isEqualTo(ruleKey); - List issues = newArrayList(issueCache.traverse()); - assertThat(issues).hasSize(1); - - assertThat(componentsWithUnprocessedIssues.getUuids()).isEmpty(); - } - @Test public void execute_issue_visitors() throws Exception { componentsWithUnprocessedIssues.setUuids(Collections.emptySet()); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java index 0beeaebbb06..edb79bbb0b9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java @@ -72,15 +72,6 @@ public class IssueLifecycleTest { assertThat(issue.debt()).isEqualTo(DEFAULT_DURATION); } - @Test - public void moveOpenManualIssue() throws Exception { - DefaultIssue issue = new DefaultIssue(); - - underTest.moveOpenManualIssue(issue, 1); - - verify(updater).setLine(issue, 1); - } - @Test public void doAutomaticTransition() throws Exception { DefaultIssue issue = new DefaultIssue(); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java index dae576cdcd0..9b57dd27efb 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java @@ -19,22 +19,15 @@ */ package org.sonar.core.issue.tracking; -import org.sonar.api.batch.BatchSide; -import org.sonar.api.batch.InstantiationStrategy; -import com.google.common.base.Predicate; -import com.google.common.base.Strings; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Multimap; - import java.util.Collection; import java.util.Objects; -import java.util.Set; - import javax.annotation.Nonnull; - import org.apache.commons.lang.StringUtils; +import org.sonar.api.batch.BatchSide; +import org.sonar.api.batch.InstantiationStrategy; import org.sonar.api.rule.RuleKey; -import static com.google.common.collect.FluentIterable.from; @InstantiationStrategy(InstantiationStrategy.PER_BATCH) @BatchSide @@ -43,8 +36,6 @@ public class Tracker { public Tracking track(Input rawInput, Input baseInput) { Tracking tracking = new Tracking<>(rawInput, baseInput); - relocateManualIssues(rawInput, baseInput, tracking); - // 1. match issues with same rule, same line and same line hash, but not necessarily with same message match(tracking, LineAndLineHashKeyFactory.INSTANCE); @@ -93,40 +84,6 @@ public class Tracker { } } - private void relocateManualIssues(Input rawInput, Input baseInput, Tracking tracking) { - Iterable manualIssues = from(tracking.getUnmatchedBases()).filter(IsManual.INSTANCE); - for (BASE base : manualIssues) { - if (base.getLine() == null) { - // no need to relocate. Location is unchanged. - tracking.keepManualIssueOpen(base, null); - } else { - String baseHash = base.getLineHash(); - if (Strings.isNullOrEmpty(baseHash)) { - baseHash = baseInput.getLineHashSequence().getHashForLine(base.getLine()); - } - if (!Strings.isNullOrEmpty(baseHash)) { - Set rawLines = rawInput.getLineHashSequence().getLinesForHash(baseHash); - if (rawLines.size() == 1) { - tracking.keepManualIssueOpen(base, rawLines.iterator().next()); - } else if (rawLines.isEmpty() && rawInput.getLineHashSequence().hasLine(base.getLine())) { - // still valid (???). We didn't manage to correctly detect code move, so the - // issue is kept at the same location, even if code changes - tracking.keepManualIssueOpen(base, base.getLine()); - } - // TODO if hash found multiple times, pick the closest line - } - } - } - } - - private enum IsManual implements Predicate { - INSTANCE; - @Override - public boolean apply(Trackable input) { - return input.getRuleKey().isManual(); - } - } - private interface SearchKey { } 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 d79c292a0e3..214bf7afc79 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 @@ -21,15 +21,12 @@ package org.sonar.core.issue.tracking; import com.google.common.base.Objects; import com.google.common.base.Predicate; -import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Iterables; -import com.google.common.collect.Multimap; import java.util.Collection; import java.util.IdentityHashMap; import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; -import javax.annotation.Nullable; public class Tracking { @@ -56,13 +53,6 @@ public class Tracking { } }; - /** - * The manual issues that are still valid (related source code still exists). They - * are grouped by line. Lines start with 1. The key 0 references the manual - * issues that do not relate to a line. - */ - private final Multimap openManualIssuesByLine = ArrayListMultimap.create(); - public Tracking(Input rawInput, Input baseInput) { this.raws = rawInput.getIssues(); this.bases = baseInput.getIssues(); @@ -106,15 +96,6 @@ public class Tracking { return rawToBase.size() == raws.size(); } - public Multimap getOpenManualIssuesByLine() { - return openManualIssuesByLine; - } - - void keepManualIssueOpen(BASE manualIssue, @Nullable Integer line) { - openManualIssuesByLine.put(line, manualIssue); - baseToRaw.put(manualIssue, null); - } - @Override public String toString() { return Objects.toStringHelper(this) @@ -122,7 +103,6 @@ public class Tracking { .add("baseToRaw", baseToRaw) .add("raws", raws) .add("bases", bases) - .add("openManualIssuesByLine", openManualIssuesByLine) .toString(); } } 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 7a00ba8f4cf..b55d2fdd8b8 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 @@ -19,14 +19,11 @@ */ package org.sonar.core.issue.tracking; -import com.google.common.collect.Iterables; -import com.google.common.collect.Multimap; import java.util.ArrayList; import java.util.Collection; import java.util.List; import javax.annotation.Nullable; import org.apache.commons.codec.digest.DigestUtils; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -41,7 +38,6 @@ 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_MANUAL = RuleKey.of(RuleKey.MANUAL_REPOSITORY_KEY, "CodeReview"); @Rule public ExpectedException thrown = ExpectedException.none(); @@ -211,8 +207,7 @@ public class TrackerTest { " public void doSomethingElse() {", " // doSomethingElse", " }", - "}" - ); + "}"); Issue base1 = baseInput.createIssueOnLine(7, RULE_SYSTEM_PRINT, "Indentation"); Issue base2 = baseInput.createIssueOnLine(11, RULE_SYSTEM_PRINT, "Indentation"); @@ -238,8 +233,7 @@ public class TrackerTest { " public void doSomethingElse() {", " // doSomethingElse", " }", - "}" - ); + "}"); Issue raw1 = rawInput.createIssueOnLine(9, RULE_SYSTEM_PRINT, "Indentation"); Issue raw2 = rawInput.createIssueOnLine(13, RULE_SYSTEM_PRINT, "Indentation"); Issue raw3 = rawInput.createIssueOnLine(17, RULE_SYSTEM_PRINT, "Indentation"); @@ -265,8 +259,7 @@ public class TrackerTest { " void method1() {", " System.out.println(\"toto\");", " }", - "}" - ); + "}"); Issue base1 = baseInput.createIssueOnLine(5, RULE_SYSTEM_PRINT, "SystemPrintln"); FakeInput rawInput = FakeInput.createForSourceLines( @@ -285,8 +278,7 @@ public class TrackerTest { " void method3() {", " System.out.println(\"toto\");", " }", - "}" - ); + "}"); Issue raw1 = rawInput.createIssueOnLine(6, RULE_SYSTEM_PRINT, "SystemPrintln"); Issue raw2 = rawInput.createIssueOnLine(10, RULE_SYSTEM_PRINT, "SystemPrintln"); Issue raw3 = rawInput.createIssueOnLine(14, RULE_SYSTEM_PRINT, "SystemPrintln"); @@ -315,8 +307,7 @@ public class TrackerTest { "\tprivate String myMethod() {", // UnusedPrivateMethod "\t\treturn \"hello\";", "\t}", - "}" - ); + "}"); Issue base1 = baseInput.createIssueOnLine(6, RULE_UNUSED_LOCAL_VARIABLE, "Avoid unused local variables such as 'j'."); Issue base2 = baseInput.createIssueOnLine(13, RULE_UNUSED_PRIVATE_METHOD, "Avoid unused private methods such as 'myMethod()'."); Issue base3 = baseInput.createIssueOnLine(9, RULE_NOT_DESIGNED_FOR_EXTENSION, @@ -342,8 +333,7 @@ public class TrackerTest { " public void newIssue() {", " String msg = myMethod();", // new issue UnusedLocalVariable " }", - "}" - ); + "}"); Issue newRaw = rawInput.createIssueOnLine(18, RULE_UNUSED_LOCAL_VARIABLE, "Avoid unused local variables such as 'msg'."); Issue rawSameAsBase1 = rawInput.createIssueOnLine(6, RULE_UNUSED_LOCAL_VARIABLE, "Avoid unused local variables such as 'j'."); @@ -358,65 +348,6 @@ public class TrackerTest { assertThat(tracking.getUnmatchedBases()).containsOnly(base2); } - @Test - public void move_manual_issue_to_line_with_same_hash() { - FakeInput baseInput = new FakeInput("H1", "H2"); - Issue issue = baseInput.createIssueOnLine(1, RULE_MANUAL, "message"); - FakeInput rawInput = new FakeInput("H3", "H4", "H1"); - - Tracking tracking = tracker.track(rawInput, baseInput); - - assertThat(tracking.getUnmatchedBases()).isEmpty(); - Multimap openManualIssues = tracking.getOpenManualIssuesByLine(); - assertThat(openManualIssues.keySet()).containsOnly(3); - assertThat(Iterables.getOnlyElement(openManualIssues.get(3))).isSameAs(issue); - } - - @Test - public void do_not_move_manual_issue_if_line_hash_not_found_in_raw() { - FakeInput baseInput = new FakeInput("H1", "H2"); - Issue issue = baseInput.createIssueOnLine(1, RULE_MANUAL, "message"); - FakeInput rawInput = new FakeInput("H3", "H4", "H5"); - - Tracking tracking = tracker.track(rawInput, baseInput); - - assertThat(tracking.getUnmatchedBases()).isEmpty(); - Multimap openManualIssues = tracking.getOpenManualIssuesByLine(); - assertThat(openManualIssues.keySet()).containsOnly(1); - assertThat(Iterables.getOnlyElement(openManualIssues.get(1))).isSameAs(issue); - } - - @Test - public void do_not_match_manual_issue_if_hash_and_line_do_not_exist() { - // manual issue is on line 3 (hash H3) but this hash does not exist - // anymore nor the line 3. - FakeInput baseInput = new FakeInput("H1", "H2", "H3"); - Issue issue = baseInput.createIssueOnLine(3, RULE_MANUAL, "message"); - FakeInput rawInput = new FakeInput("H4"); - - Tracking tracking = tracker.track(rawInput, baseInput); - - assertThat(tracking.getUnmatchedBases()).containsOnly(issue); - assertThat(tracking.getOpenManualIssuesByLine().isEmpty()).isTrue(); - } - - @Test - @Ignore("not implemented yet") - public void move_to_closest_line_if_manual_issue_matches_multiple_hashes() { - // manual issue is on line 3 (hash H3) but this hash does not exist - // anymore nor the line 3. - FakeInput baseInput = new FakeInput("H1", "H2"); - Issue issue = baseInput.createIssueOnLine(1, RULE_MANUAL, "message"); - FakeInput rawInput = new FakeInput("H1", "H3", "H1"); - - Tracking tracking = tracker.track(rawInput, baseInput); - - assertThat(tracking.getUnmatchedBases()).isEmpty(); - Multimap openManualIssues = tracking.getOpenManualIssuesByLine(); - assertThat(openManualIssues.keySet()).containsOnly(1); - assertThat(Iterables.getOnlyElement(openManualIssues.get(1))).isSameAs(issue); - } - private static class Issue implements Trackable { private final RuleKey ruleKey; private final Integer line; diff --git a/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/LocalIssueTracking.java b/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/LocalIssueTracking.java index 634326771d4..21a89cdb95c 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/LocalIssueTracking.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/batch/issue/tracking/LocalIssueTracking.java @@ -19,15 +19,7 @@ */ package org.sonar.batch.issue.tracking; -import org.sonar.core.issue.tracking.Tracking; -import org.sonar.scanner.protocol.output.ScannerReport; -import org.sonar.core.issue.tracking.Input; -import org.sonar.core.issue.tracking.Tracker; -import org.sonar.batch.issue.IssueTransformer; -import org.sonar.api.batch.fs.InputFile.Status; -import org.sonar.batch.analysis.DefaultAnalysisMode; import com.google.common.annotations.VisibleForTesting; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -35,18 +27,23 @@ import java.util.Date; import java.util.LinkedList; import java.util.List; import java.util.Map; - import javax.annotation.CheckForNull; import javax.annotation.Nullable; - import org.sonar.api.batch.BatchSide; +import org.sonar.api.batch.fs.InputFile.Status; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.rule.ActiveRule; import org.sonar.api.batch.rule.ActiveRules; import org.sonar.api.issue.Issue; import org.sonar.api.resources.ResourceUtils; +import org.sonar.batch.analysis.DefaultAnalysisMode; import org.sonar.batch.index.BatchComponent; +import org.sonar.batch.issue.IssueTransformer; import org.sonar.batch.repository.ProjectRepositories; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.Tracker; +import org.sonar.core.issue.tracking.Tracking; +import org.sonar.scanner.protocol.output.ScannerReport; @BatchSide public class LocalIssueTracking { @@ -94,7 +91,6 @@ public class LocalIssueTracking { Tracking track = tracker.track(rawIssues, baseIssues); addUnmatchedFromServer(track.getUnmatchedBases(), sourceHashHolder, trackedIssues); - addUnmatchedFromServer(track.getOpenManualIssuesByLine().values(), sourceHashHolder, trackedIssues); mergeMatched(track, trackedIssues, rIssues); addUnmatchedFromReport(track.getUnmatchedRaws(), trackedIssues, analysisDate); } -- 2.39.5