SONAR-5827 Fix corner cases with highligthing offset conversion

sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilder.java

@@ -19,11 +19,10 @@
  */
 package org.sonar.batch.highlighting;
 
-import com.google.common.collect.Sets;
-import org.sonar.api.batch.sensor.highlighting.TypeOfText;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Ordering;
+import com.google.common.collect.Sets;
+import org.sonar.api.batch.sensor.highlighting.TypeOfText;
 
 import javax.annotation.Nullable;
 
@@ -38,10 +37,10 @@ public class SyntaxHighlightingDataBuilder {
     syntaxHighlightingRuleSet = Sets.newTreeSet(new Ordering<SyntaxHighlightingRule>() {
       @Override
       public int compare(@Nullable SyntaxHighlightingRule left,
-                         @Nullable SyntaxHighlightingRule right) {
+        @Nullable SyntaxHighlightingRule right) {
         int result = left.getStartPosition() - right.getStartPosition();
         if (result == 0) {
-          result = left.getEndPosition() - right.getEndPosition();
+          result = right.getEndPosition() - left.getEndPosition();
         }
         return result;
       }

sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingRule.java

@@ -50,4 +50,9 @@ public class SyntaxHighlightingRule implements Serializable {
   public TypeOfText getTextType() {
     return textType;
   }
+
+  @Override
+  public String toString() {
+    return "" + startPosition + "," + endPosition + "," + textType.cssClass();
+  }
 }

sonar-batch/src/main/java/org/sonar/batch/index/SourcePersister.java

@@ -113,28 +113,8 @@ public class SourcePersister implements ScanPersister {
 
       for (InputPath inputPath : inputPathCache.all()) {
         if (inputPath instanceof InputFile) {
-          DefaultInputFile inputFile = (DefaultInputFile) inputPath;
-          org.sonar.api.resources.File file = (org.sonar.api.resources.File) resourceCache.get(inputFile.key());
-          String fileUuid = file.getUuid();
-          FileSourceDto previous = mapper.select(fileUuid);
-          String newData = getSourceData(inputFile);
-          String dataHash = newData != null ? DigestUtils.md5Hex(newData) : "0";
-          Date now = system2.newDate();
-          if (previous == null) {
-            FileSourceDto newFileSource = new FileSourceDto().setProjectUuid(projectTree.getRootProject().getUuid()).setFileUuid(fileUuid).setData(newData).setDataHash(dataHash)
-              .setCreatedAt(now)
-              .setUpdatedAt(now);
-            mapper.insert(newFileSource);
-          } else {
-            if (dataHash.equals(previous.getDataHash())) {
-              continue;
-            } else {
-              previous.setData(newData).setDataHash(dataHash).setUpdatedAt(now);
-              mapper.update(previous);
-            }
-          }
+          persist(session, mapper, inputPath);
         }
-        session.commit();
       }
     } catch (Exception e) {
       throw new IllegalStateException("Unable to save file sources", e);
@@ -144,6 +124,29 @@ public class SourcePersister implements ScanPersister {
 
   }
 
+  private void persist(DbSession session, FileSourceMapper mapper, InputPath inputPath) {
+    DefaultInputFile inputFile = (DefaultInputFile) inputPath;
+    org.sonar.api.resources.File file = (org.sonar.api.resources.File) resourceCache.get(inputFile.key());
+    String fileUuid = file.getUuid();
+    FileSourceDto previous = mapper.select(fileUuid);
+    String newData = getSourceData(inputFile);
+    String dataHash = newData != null ? DigestUtils.md5Hex(newData) : "0";
+    Date now = system2.newDate();
+    if (previous == null) {
+      FileSourceDto newFileSource = new FileSourceDto().setProjectUuid(projectTree.getRootProject().getUuid()).setFileUuid(fileUuid).setData(newData).setDataHash(dataHash)
+        .setCreatedAt(now)
+        .setUpdatedAt(now);
+      mapper.insert(newFileSource);
+      session.commit();
+    } else {
+      if (!dataHash.equals(previous.getDataHash())) {
+        previous.setData(newData).setDataHash(dataHash).setUpdatedAt(now);
+        mapper.update(previous);
+        session.commit();
+      }
+    }
+  }
+
   @CheckForNull
   String getSourceData(DefaultInputFile file) {
     if (file.lines() == 0) {
@@ -176,36 +179,50 @@ public class SourcePersister implements ScanPersister {
   }
 
   String[] computeHighlightingPerLine(DefaultInputFile file, @Nullable SyntaxHighlightingData highlighting) {
-    String[] highlightingPerLine = new String[file.lines()];
+    String[] result = new String[file.lines()];
     if (highlighting == null) {
-      return highlightingPerLine;
+      return result;
     }
     Iterable<SyntaxHighlightingRule> rules = highlighting.syntaxHighlightingRuleSet();
     int currentLineIdx = 1;
-    StringBuilder currentLineSb = new StringBuilder();
+    StringBuilder[] highlightingPerLine = new StringBuilder[file.lines()];
     for (SyntaxHighlightingRule rule : rules) {
-      long ruleStartOffset = rule.getStartPosition();
-      long ruleEndOffset = rule.getEndPosition();
-      while (currentLineIdx < file.lines() && ruleStartOffset >= file.originalLineOffsets()[currentLineIdx]) {
+      while (currentLineIdx < file.lines() && rule.getStartPosition() >= file.originalLineOffsets()[currentLineIdx]) {
         // This rule starts on another line so advance
-        saveLineHighlighting(highlightingPerLine, currentLineIdx, currentLineSb);
         currentLineIdx++;
       }
       // Now we know current rule starts on current line
-      long ruleStartOffsetCurrentLine = ruleStartOffset;
-      while (currentLineIdx < file.lines() && ruleEndOffset >= file.originalLineOffsets()[currentLineIdx]) {
-        // rule continue on next line so write current line and continue on next line with same rule
-        writeRule(currentLineSb, ruleStartOffsetCurrentLine - file.originalLineOffsets()[currentLineIdx - 1], file.originalLineOffsets()[currentLineIdx] - 1, rule.getTextType());
-        saveLineHighlighting(highlightingPerLine, currentLineIdx, currentLineSb);
-        currentLineIdx++;
-        ruleStartOffsetCurrentLine = file.originalLineOffsets()[currentLineIdx];
-      }
-      // Rule ends on current line
-      writeRule(currentLineSb, ruleStartOffsetCurrentLine - file.originalLineOffsets()[currentLineIdx - 1], ruleEndOffset - file.originalLineOffsets()[currentLineIdx - 1],
+      writeRule(file, rule, highlightingPerLine, currentLineIdx);
+    }
+    for (int i = 0; i < file.lines(); i++) {
+      result[i] = highlightingPerLine[i] != null ? highlightingPerLine[i].toString() : null;
+    }
+    return result;
+  }
+
+  private void writeRule(DefaultInputFile file, SyntaxHighlightingRule rule, StringBuilder[] highlightingPerLine, int currentLineIdx) {
+    // We know current rule starts on current line
+    long ruleStartOffsetCurrentLine = rule.getStartPosition();
+    while (currentLineIdx < file.lines() && rule.getEndPosition() >= file.originalLineOffsets()[currentLineIdx]) {
+      // rule continue on next line so write current line and continue on next line with same rule
+      writeRule(highlightingPerLine, currentLineIdx, ruleStartOffsetCurrentLine - file.originalLineOffsets()[currentLineIdx - 1], file.originalLineOffsets()[currentLineIdx]
+        - file.originalLineOffsets()[currentLineIdx - 1],
         rule.getTextType());
+      currentLineIdx++;
+      ruleStartOffsetCurrentLine = file.originalLineOffsets()[currentLineIdx - 1];
     }
-    saveLineHighlighting(highlightingPerLine, currentLineIdx, currentLineSb);
-    return highlightingPerLine;
+    // Rule ends on current line
+    writeRule(highlightingPerLine, currentLineIdx, ruleStartOffsetCurrentLine - file.originalLineOffsets()[currentLineIdx - 1], rule.getEndPosition()
+      - file.originalLineOffsets()[currentLineIdx - 1],
+      rule.getTextType());
+  }
+
+  private void writeRule(StringBuilder[] highlightingPerLine, int currentLineIdx, long startLineOffset, long endLineOffset, TypeOfText textType) {
+    if (highlightingPerLine[currentLineIdx - 1] == null) {
+      highlightingPerLine[currentLineIdx - 1] = new StringBuilder();
+    }
+    StringBuilder currentLineSb = highlightingPerLine[currentLineIdx - 1];
+    writeRule(currentLineSb, startLineOffset, endLineOffset, textType);
   }
 
   private void writeRule(StringBuilder currentLineSb, long startLineOffset, long endLineOffset, TypeOfText textType) {
@@ -219,11 +236,6 @@ public class SourcePersister implements ScanPersister {
       .append(textType.cssClass());
   }
 
-  private void saveLineHighlighting(String[] highlightingPerLine, int currentLineIdx, StringBuilder currentLineSb) {
-    highlightingPerLine[currentLineIdx - 1] = currentLineSb.toString();
-    currentLineSb.setLength(0);
-  }
-
   private Map<Integer, String> getLineMetric(DefaultInputFile file, String metricKey) {
     Map<Integer, String> authorsByLine;
     Iterator<Measure> authorsIt = measureCache.byMetric(file.key(), metricKey).iterator();

sonar-batch/src/test/java/org/sonar/batch/index/SourcePersisterTest.java

@@ -37,7 +37,7 @@ import org.sonar.api.utils.DateUtils;
 import org.sonar.api.utils.System2;
 import org.sonar.batch.ProjectTree;
 import org.sonar.batch.highlighting.SyntaxHighlightingData;
-import org.sonar.batch.highlighting.SyntaxHighlightingRule;
+import org.sonar.batch.highlighting.SyntaxHighlightingDataBuilder;
 import org.sonar.batch.scan.filesystem.InputPathCache;
 import org.sonar.batch.scan.measure.MeasureCache;
 import org.sonar.core.persistence.AbstractDaoTestCase;
@@ -48,6 +48,7 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 
+import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.mock;
@@ -188,11 +189,11 @@ public class SourcePersisterTest extends AbstractDaoTestCase {
     when(measureCache.byMetric(PROJECT_KEY + ":" + relativePathNew, CoreMetrics.SCM_REVISIONS_BY_LINE_KEY))
       .thenReturn(Arrays.asList(new Measure(CoreMetrics.SCM_REVISIONS_BY_LINE, "1=123;2=234;3=345")));
 
-    SyntaxHighlightingData highlighting = new SyntaxHighlightingData(Arrays.asList(
-      SyntaxHighlightingRule.create(0, 3, TypeOfText.ANNOTATION),
-      SyntaxHighlightingRule.create(4, 5, TypeOfText.COMMENT),
-      SyntaxHighlightingRule.create(7, 16, TypeOfText.CONSTANT)
-      ));
+    SyntaxHighlightingData highlighting = new SyntaxHighlightingDataBuilder()
+      .registerHighlightingRule(0, 3, TypeOfText.ANNOTATION)
+      .registerHighlightingRule(4, 5, TypeOfText.COMMENT)
+      .registerHighlightingRule(7, 16, TypeOfText.CONSTANT)
+      .build();
     when(componentDataCache.getData(PROJECT_KEY + ":" + relativePathNew, SnapshotDataTypes.SYNTAX_HIGHLIGHTING))
       .thenReturn(highlighting);
 
@@ -200,6 +201,76 @@ public class SourcePersisterTest extends AbstractDaoTestCase {
     checkTables("testPersistNewFileWithScmAndHighlighting", "file_sources");
   }
 
+  @Test
+  public void testSimpleConversionOfHighlightingOffset() {
+    DefaultInputFile file = new DefaultInputFile(PROJECT_KEY, "src/foo.java")
+      .setLines(3)
+      .setOriginalLineOffsets(new long[] {0, 4, 7});
+
+    SyntaxHighlightingData highlighting = new SyntaxHighlightingDataBuilder()
+      .registerHighlightingRule(0, 3, TypeOfText.ANNOTATION)
+      .registerHighlightingRule(4, 5, TypeOfText.COMMENT)
+      .registerHighlightingRule(7, 16, TypeOfText.CONSTANT)
+      .build();
+
+    String[] highlightingPerLine = sourcePersister.computeHighlightingPerLine(file, highlighting);
+
+    assertThat(highlightingPerLine).containsOnly("0,3,a", "0,1,cd", "0,9,c");
+  }
+
+  @Test
+  public void testConversionOfHighlightingOffsetMultiLine() {
+    DefaultInputFile file = new DefaultInputFile(PROJECT_KEY, "src/foo.java")
+      .setLines(3)
+      .setOriginalLineOffsets(new long[] {0, 4, 7});
+
+    SyntaxHighlightingData highlighting = new SyntaxHighlightingDataBuilder()
+      .registerHighlightingRule(0, 3, TypeOfText.ANNOTATION)
+      .registerHighlightingRule(4, 9, TypeOfText.COMMENT)
+      .registerHighlightingRule(10, 16, TypeOfText.CONSTANT)
+      .build();
+
+    String[] highlightingPerLine = sourcePersister.computeHighlightingPerLine(file, highlighting);
+
+    assertThat(highlightingPerLine).containsOnly("0,3,a", "0,3,cd", "0,2,cd;3,9,c");
+  }
+
+  @Test
+  public void testConversionOfHighlightingNestedRules() {
+    DefaultInputFile file = new DefaultInputFile(PROJECT_KEY, "src/foo.java")
+      .setLines(3)
+      .setOriginalLineOffsets(new long[] {0, 4, 7});
+
+    SyntaxHighlightingData highlighting = new SyntaxHighlightingDataBuilder()
+      .registerHighlightingRule(0, 3, TypeOfText.ANNOTATION)
+      .registerHighlightingRule(4, 6, TypeOfText.COMMENT)
+      .registerHighlightingRule(7, 16, TypeOfText.CONSTANT)
+      .registerHighlightingRule(8, 15, TypeOfText.KEYWORD)
+      .build();
+
+    String[] highlightingPerLine = sourcePersister.computeHighlightingPerLine(file, highlighting);
+
+    assertThat(highlightingPerLine).containsOnly("0,3,a", "0,2,cd", "0,9,c;1,8,k");
+  }
+
+  @Test
+  public void testConversionOfHighlightingNestedRulesMultiLine() {
+    DefaultInputFile file = new DefaultInputFile(PROJECT_KEY, "src/foo.java")
+      .setLines(3)
+      .setOriginalLineOffsets(new long[] {0, 4, 7});
+
+    SyntaxHighlightingData highlighting = new SyntaxHighlightingDataBuilder()
+      .registerHighlightingRule(0, 3, TypeOfText.ANNOTATION)
+      .registerHighlightingRule(4, 6, TypeOfText.COMMENT)
+      .registerHighlightingRule(4, 16, TypeOfText.CONSTANT)
+      .registerHighlightingRule(8, 15, TypeOfText.KEYWORD)
+      .build();
+
+    String[] highlightingPerLine = sourcePersister.computeHighlightingPerLine(file, highlighting);
+
+    assertThat(highlightingPerLine).containsOnly("0,3,a", "0,3,c;0,2,cd", "0,9,c;1,8,k");
+  }
+
   private void mockResourceCache(String relativePathEmpty, String projectKey, String uuid) {
     File sonarFile = File.create(relativePathEmpty);
     sonarFile.setUuid(uuid);