SONAR-3387 Fix quality flaws

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 42d1013dc1c0047b29a11e66a3e7c9b2a06110a8..a8fe508840355d5eddfd98b0c15982b8733e5516 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
@@ -215,15 +215,21 @@ public class IssueTrackingDecorator implements Decorator {
     Logger logger = LoggerFactory.getLogger(IssueTrackingDecorator.class);
     logger.debug("Trying to relocate manual issue {}", oldIssue.getKee());
 
-    Collection<Integer> newLinesWithSameHash = sourceHashHolder.getNewLinesMatching(oldIssue.getLine());
+    Integer previousLine = oldIssue.getLine();
+    if (previousLine == null) {
+      logger.debug("Cannot relocate issue at resource level");
+      return;
+    }
+
+    Collection<Integer> newLinesWithSameHash = sourceHashHolder.getNewLinesMatching(previousLine);
     logger.debug("Found the following lines with same hash: {}", newLinesWithSameHash);
-    if (newLinesWithSameHash.size() == 0) {
-      if (oldIssue.getLine() > sourceHashHolder.getHashedSource().length()) {
-        logger.debug("Old issue line {} is out of new source, closing and removing line number", oldIssue.getLine());
+    if (newLinesWithSameHash.isEmpty()) {
+      if (previousLine > sourceHashHolder.getHashedSource().length()) {
+        logger.debug("Old issue line {} is out of new source, closing and removing line number", previousLine);
         newIssue.setLine(null);
         updater.setStatus(newIssue, Issue.STATUS_CLOSED, changeContext);
         updater.setResolution(newIssue, Issue.RESOLUTION_REMOVED, changeContext);
-        updater.setPastLine(newIssue, oldIssue.getLine());
+        updater.setPastLine(newIssue, previousLine);
         updater.setPastMessage(newIssue, oldIssue.getMessage(), changeContext);
         updater.setPastEffortToFix(newIssue, oldIssue.getEffortToFix(), changeContext);
       }
@@ -232,7 +238,7 @@ public class IssueTrackingDecorator implements Decorator {
       logger.debug("Relocating issue to line {}", newLine);
 
       newIssue.setLine(newLine);
-      updater.setPastLine(newIssue, oldIssue.getLine());
+      updater.setPastLine(newIssue, previousLine);
       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/tracking/HashedSequence.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/tracking/HashedSequence.java
index 9659ca21f478426671ba8248f11a6a78290a4852..d8a19ca599717e7126d58af564150124327a5da7 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
@@ -39,7 +39,8 @@ public final class HashedSequence<S extends Sequence> implements Sequence {
     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
+      // indices in array are shifted one line before
+      linesByHash.put(hashes[i], i + 1);
     }
     return new HashedSequence<S>(base, hashes, linesByHash);
   }
@@ -59,6 +60,7 @@ public final class HashedSequence<S extends Sequence> implements Sequence {
   }
 
   public Integer getHash(Integer line) {
-    return hashes[line - 1]; // indices in array are shifted one line before
+    // indices in array are shifted one line before
+    return hashes[line - 1];
   }
 }

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 8e75b331c92f02f7ca8255bf8c4db6449d8bfc06..c81eb042b52c6729e53722b41f7c498b398dccdf 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
@@ -241,6 +241,40 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase {
     assertThat(issue.status()).isEqualTo("CLOSED");
   }
 
+  @Test
+  public void manual_issues_should_be_untouched_if_line_is_null() throws Exception {
+    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(null).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 {}";
+    when(index.getSource(file)).thenReturn(originalSource);
+    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(null);
+    assertThat(issue.key()).isEqualTo("ABCDE");
+    assertThat(issue.isNew()).isFalse();
+    assertThat(issue.isEndOfLife()).isFalse();
+    assertThat(issue.isOnDisabledRule()).isFalse();
+    assertThat(issue.status()).isEqualTo("OPEN");
+  }
+
   @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