SONAR-4683 First naive implementation of manual issues relocation

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java
index 23a618289531a6801d1ba776cb048ec362c9fb8c..75e4531d249611e921c01c185592ecc6ab351dc7 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java
@@ -19,16 +19,18 @@
  */
 package org.sonar.plugins.core.issue;
 
-import org.sonar.api.batch.SonarIndex;
-import org.sonar.batch.scan.LastSnapshots;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
+import org.apache.commons.lang.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sonar.api.batch.Decorator;
 import org.sonar.api.batch.DecoratorBarriers;
 import org.sonar.api.batch.DecoratorContext;
 import org.sonar.api.batch.DependedUpon;
 import org.sonar.api.batch.DependsUpon;
+import org.sonar.api.batch.SonarIndex;
 import org.sonar.api.component.ResourcePerspectives;
 import org.sonar.api.issue.Issuable;
 import org.sonar.api.issue.Issue;
@@ -43,6 +45,7 @@ import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RuleFinder;
 import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.batch.issue.IssueCache;
+import org.sonar.batch.scan.LastSnapshots;
 import org.sonar.core.issue.IssueUpdater;
 import org.sonar.core.issue.db.IssueDto;
 import org.sonar.core.issue.workflow.IssueWorkflow;
@@ -116,7 +119,7 @@ public class IssueTrackingDecorator implements Decorator {
     IssueTrackingResult trackingResult = tracking.track(sourceHashHolder, dbOpenIssues, issues);
 
     // unmatched = issues that have been resolved + issues on disabled/removed rules + manual issues
-    addUnmatched(trackingResult.unmatched(), issues);
+    addUnmatched(trackingResult.unmatched(), sourceHashHolder, issues);
 
     mergeMatched(trackingResult);
 
@@ -171,9 +174,12 @@ public class IssueTrackingDecorator implements Decorator {
     }
   }
 
-  private void addUnmatched(Collection<IssueDto> unmatchedIssues, Collection<DefaultIssue> issues) {
+  private void addUnmatched(Collection<IssueDto> unmatchedIssues, SourceHashHolder sourceHashHolder, Collection<DefaultIssue> issues) {
     for (IssueDto unmatchedDto : unmatchedIssues) {
       DefaultIssue unmatched = unmatchedDto.toDefaultIssue();
+      if (StringUtils.isNotBlank(unmatchedDto.getReporter())) {
+        relocateManualIssue(unmatched, unmatchedDto, sourceHashHolder);
+      }
       updateUnmatchedIssue(unmatched, false /* manual issues can be kept open */);
       issues.add(unmatched);
     }
@@ -204,4 +210,21 @@ public class IssueTrackingDecorator implements Decorator {
       issue.setOnDisabledRule(activeRule == null || rule == null || Rule.STATUS_REMOVED.equals(rule.getStatus()));
     }
   }
+
+  private void relocateManualIssue(DefaultIssue newIssue, IssueDto oldIssue, SourceHashHolder sourceHashHolder) {
+    Logger logger = LoggerFactory.getLogger(IssueTrackingDecorator.class);
+    logger.debug("Trying to relocate manual issue {}", oldIssue.getKee());
+
+    Collection<Integer> newLinesWithSameHash = sourceHashHolder.getNewLinesMatching(oldIssue.getLine());
+    logger.debug("Found the following lines with same hash: {}", newLinesWithSameHash);
+    if (newLinesWithSameHash.size() == 1) {
+      Integer newLine = newLinesWithSameHash.iterator().next();
+      logger.debug("Relocating issue to line {}", newLine);
+
+      newIssue.setLine(newLine);
+      updater.setPastLine(newIssue, oldIssue.getLine());
+      updater.setPastMessage(newIssue, oldIssue.getMessage(), changeContext);
+      updater.setPastEffortToFix(newIssue, oldIssue.getEffortToFix(), changeContext);
+    }
+  }
 }

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java
index 15633db972e6730dcb1e8abc893859201e93d50e..e8b6c18ee6bd92c66d24a2bca1f47067e28ffbda 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.plugins.core.issue;
 
+import java.util.Collection;
+
 import org.sonar.api.batch.SonarIndex;
 import org.sonar.api.resources.Resource;
 import org.sonar.batch.scan.LastSnapshots;
@@ -49,8 +51,8 @@ public class SourceHashHolder {
   }
 
   private void initHashes() {
-    hashedReference = HashedSequence.wrap(new StringText(referenceSource), StringTextComparator.IGNORE_WHITESPACE);
-    hashedSource = HashedSequence.wrap(new StringText(source), StringTextComparator.IGNORE_WHITESPACE);
+    hashedReference = HashedSequence.wrap(new StringText(getReferenceSource()), StringTextComparator.IGNORE_WHITESPACE);
+    hashedSource = HashedSequence.wrap(new StringText(getSource()), StringTextComparator.IGNORE_WHITESPACE);
   }
 
   public HashedSequence<StringText> getHashedReference() {
@@ -90,5 +92,9 @@ public class SourceHashHolder {
       initHashes();
     }
   }
+
+  public Collection<Integer> getNewLinesMatching(Integer originLine) {
+    return getHashedSource().getLinesForHash(getHashedReference().getHash(originLine));
+  }
 }
 

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/tracking/HashedSequence.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/tracking/HashedSequence.java
index 4edb4a8500b1e63012598a2e6a62f9bf57bbef02..9659ca21f478426671ba8248f11a6a78290a4852 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/tracking/HashedSequence.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/tracking/HashedSequence.java
@@ -19,6 +19,11 @@
  */
 package org.sonar.plugins.core.issue.tracking;
 
+import java.util.Collection;
+
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Multimap;
+
 /**
  * Wraps a {@link Sequence} to assign hash codes to elements.
  */
@@ -26,23 +31,34 @@ public final class HashedSequence<S extends Sequence> implements Sequence {
 
   final S base;
   final int[] hashes;
+  final Multimap<Integer, Integer> linesByHash;
 
   public static <S extends Sequence> HashedSequence<S> wrap(S base, SequenceComparator<S> cmp) {
     int size = base.length();
     int[] hashes = new int[size];
+    Multimap<Integer, Integer> linesByHash = LinkedHashMultimap.create();
     for (int i = 0; i < size; i++) {
       hashes[i] = cmp.hash(base, i);
+      linesByHash.put(hashes[i], i + 1); // indices in array are shifted one line before
     }
-    return new HashedSequence<S>(base, hashes);
+    return new HashedSequence<S>(base, hashes, linesByHash);
   }
 
-  private HashedSequence(S base, int[] hashes) {
+  private HashedSequence(S base, int[] hashes, Multimap<Integer, Integer> linesByHash) {
     this.base = base;
     this.hashes = hashes;
+    this.linesByHash = linesByHash;
   }
 
   public int length() {
     return base.length();
   }
 
+  public Collection<Integer> getLinesForHash(Integer hash) {
+    return linesByHash.get(hash);
+  }
+
+  public Integer getHash(Integer line) {
+    return hashes[line - 1]; // indices in array are shifted one line before
+  }
 }

diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java
index 8c742eaef33a50d060addbd9a2f0b878c132ccc8..ecf13a44d90e92c1f13a0467a86777a9ce79f834 100644 (file)
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java
@@ -150,17 +150,130 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void manual_issues_should_be_kept_open() throws Exception {
+  public void manual_issues_should_be_moved_if_matching_line_found() throws Exception {
     // "Unmatched" issues existed in previous scan but not in current one -> they have to be closed
     Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123);
 
     // INPUT : one issue existing during previous scan
-    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
+    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(6).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
     when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance"));
 
     IssueTrackingResult trackingResult = new IssueTrackingResult();
     trackingResult.addUnmatched(unmatchedIssue);
 
+    String originalSource = "public interface Action {\n"
+      + "   void method1();\n"
+      + "   void method2();\n"
+      + "   void method3();\n"
+      + "   void method4();\n"
+      + "   void method5();\n" // Original issue here
+      + "}";
+    String newSource = "public interface Action {\n"
+      + "   void method5();\n" // New issue here
+      + "   void method1();\n"
+      + "   void method2();\n"
+      + "   void method3();\n"
+      + "   void method4();\n"
+      + "}";
+    when(index.getSource(file)).thenReturn(newSource);
+    when(lastSnapshots.getSource(file)).thenReturn(originalSource);
+
+    when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult);
+
+    decorator.doDecorate(file);
+
+    verify(workflow, times(1)).doAutomaticTransition(any(DefaultIssue.class), any(IssueChangeContext.class));
+    verify(handlers, times(1)).execute(any(DefaultIssue.class), any(IssueChangeContext.class));
+
+    ArgumentCaptor<DefaultIssue> argument = ArgumentCaptor.forClass(DefaultIssue.class);
+    verify(issueCache).put(argument.capture());
+
+    DefaultIssue issue = argument.getValue();
+    assertThat(issue.line()).isEqualTo(2);
+    assertThat(issue.key()).isEqualTo("ABCDE");
+    assertThat(issue.isNew()).isFalse();
+    assertThat(issue.isEndOfLife()).isFalse();
+    assertThat(issue.isOnDisabledRule()).isFalse();
+  }
+
+  @Test
+  public void manual_issues_should_be_kept_if_matching_line_not_found() throws Exception {
+    // "Unmatched" issues existed in previous scan but not in current one -> they have to be closed
+    Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123);
+
+    // INPUT : one issue existing during previous scan
+    final int issueOnLine = 6;
+    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(issueOnLine).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
+    when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance"));
+
+    IssueTrackingResult trackingResult = new IssueTrackingResult();
+    trackingResult.addUnmatched(unmatchedIssue);
+
+    String originalSource = "public interface Action {\n"
+      + "   void method1();\n"
+      + "   void method2();\n"
+      + "   void method3();\n"
+      + "   void method4();\n"
+      + "   void method5();\n" // Original issue here
+      + "}";
+    String newSource = "public interface Action {\n"
+      + "   void method1();\n"
+      + "   void method2();\n"
+      + "   void method3();\n"
+      + "   void method4();\n"
+      + "   void method6();\n" // Poof, no method5 anymore
+      + "}";
+    when(index.getSource(file)).thenReturn(newSource);
+    when(lastSnapshots.getSource(file)).thenReturn(originalSource);
+
+    when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult);
+
+    decorator.doDecorate(file);
+
+    verify(workflow, times(1)).doAutomaticTransition(any(DefaultIssue.class), any(IssueChangeContext.class));
+    verify(handlers, times(1)).execute(any(DefaultIssue.class), any(IssueChangeContext.class));
+
+    ArgumentCaptor<DefaultIssue> argument = ArgumentCaptor.forClass(DefaultIssue.class);
+    verify(issueCache).put(argument.capture());
+
+    DefaultIssue issue = argument.getValue();
+    assertThat(issue.line()).isEqualTo(issueOnLine);
+    assertThat(issue.key()).isEqualTo("ABCDE");
+    assertThat(issue.isNew()).isFalse();
+    assertThat(issue.isEndOfLife()).isFalse();
+    assertThat(issue.isOnDisabledRule()).isFalse();
+  }
+
+  @Test
+  public void manual_issues_should_be_kept_if_multiple_matching_lines_found() throws Exception {
+    // "Unmatched" issues existed in previous scan but not in current one -> they have to be closed
+    Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123);
+
+    // INPUT : one issue existing during previous scan
+    final int issueOnLine = 3;
+    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(issueOnLine).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
+    when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance"));
+
+    IssueTrackingResult trackingResult = new IssueTrackingResult();
+    trackingResult.addUnmatched(unmatchedIssue);
+
+    String originalSource = "public class Action {\n"
+      + "   void method1() {\n"
+      + "     notify();\n" // initial issue
+      + "   }\n"
+      + "}";
+    String newSource = "public class Action {\n"
+      + "   \n"
+      + "   void method1() {\n" // new issue will appear here
+      + "     notify();\n"
+      + "   }\n"
+      + "   void method2() {\n"
+      + "     notify();\n"
+      + "   }\n"
+      + "}";
+    when(index.getSource(file)).thenReturn(newSource);
+    when(lastSnapshots.getSource(file)).thenReturn(originalSource);
+
     when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult);
 
     decorator.doDecorate(file);
@@ -172,24 +285,30 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase {
     verify(issueCache).put(argument.capture());
 
     DefaultIssue issue = argument.getValue();
+    assertThat(issue.line()).isEqualTo(issueOnLine);
     assertThat(issue.key()).isEqualTo("ABCDE");
     assertThat(issue.isNew()).isFalse();
     assertThat(issue.isEndOfLife()).isFalse();
     assertThat(issue.isOnDisabledRule()).isFalse();
   }
 
+
   @Test
   public void manual_issues_should_be_closed_if_manual_rule_is_removed() throws Exception {
     // "Unmatched" issues existed in previous scan but not in current one -> they have to be closed
     Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123);
 
     // INPUT : one issue existing during previous scan
-    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
+    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
     when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance").setStatus(Rule.STATUS_REMOVED));
 
     IssueTrackingResult trackingResult = new IssueTrackingResult();
     trackingResult.addUnmatched(unmatchedIssue);
 
+    String source = "public interface Action {}";
+    when(index.getSource(file)).thenReturn(source);
+    when(lastSnapshots.getSource(file)).thenReturn(source);
+
     when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult);
 
     decorator.doDecorate(file);
@@ -213,12 +332,59 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase {
     Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123);
 
     // INPUT : one issue existing during previous scan
-    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
+    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
+    when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(null);
+
+    IssueTrackingResult trackingResult = new IssueTrackingResult();
+    trackingResult.addUnmatched(unmatchedIssue);
+
+    String source = "public interface Action {}";
+    when(index.getSource(file)).thenReturn(source);
+    when(lastSnapshots.getSource(file)).thenReturn(source);
+
+    when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult);
+
+    decorator.doDecorate(file);
+
+    verify(workflow, times(1)).doAutomaticTransition(any(DefaultIssue.class), any(IssueChangeContext.class));
+    verify(handlers, times(1)).execute(any(DefaultIssue.class), any(IssueChangeContext.class));
+
+    ArgumentCaptor<DefaultIssue> argument = ArgumentCaptor.forClass(DefaultIssue.class);
+    verify(issueCache).put(argument.capture());
+
+    DefaultIssue issue = argument.getValue();
+    assertThat(issue.key()).isEqualTo("ABCDE");
+    assertThat(issue.isNew()).isFalse();
+    assertThat(issue.isEndOfLife()).isTrue();
+    assertThat(issue.isOnDisabledRule()).isTrue();
+  }
+
+  @Test
+  public void manual_issues_should_be_closed_if_new_source_is_shorter() throws Exception {
+    // "Unmatched" issues existed in previous scan but not in current one -> they have to be closed
+    Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123);
+
+    // INPUT : one issue existing during previous scan
+    IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(6).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance");
     when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(null);
 
     IssueTrackingResult trackingResult = new IssueTrackingResult();
     trackingResult.addUnmatched(unmatchedIssue);
 
+    String originalSource = "public interface Action {\n"
+      + "   void method1();\n"
+      + "   void method2();\n"
+      + "   void method3();\n"
+      + "   void method4();\n"
+      + "   void method5();\n"
+      + "}";
+    String newSource = "public interface Action {\n"
+      + "   void method1();\n"
+      + "   void method2();\n"
+      + "}";
+    when(index.getSource(file)).thenReturn(newSource);
+    when(lastSnapshots.getSource(file)).thenReturn(originalSource);
+
     when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult);
 
     decorator.doDecorate(file);