SONAR-7509 Highlighting API should work with ranges (line/offset)

5 files changed, 78 insertions, 12 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java
index cf4ebb418b6..c45cdd97564 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java
@@ -235,7 +235,10 @@ public class DefaultInputFile extends DefaultInputComponent implements InputFile
 
   @Override
   public TextRange newRange(int startLine, int startLineOffset, int endLine, int endLineOffset) {
-    return newRangeValidPointers(newPointer(startLine, startLineOffset), newPointer(endLine, endLineOffset));
+    TextPointer start = newPointer(startLine, startLineOffset);
+    TextPointer end = newPointer(endLine, endLineOffset);
+    Preconditions.checkArgument(start.compareTo(end) < 0, "Start pointer %s should be before end pointer %s", start, end);
+    return newRangeValidPointers(start, end);
   }
 
   @Override
@@ -259,6 +262,7 @@ public class DefaultInputFile extends DefaultInputComponent implements InputFile
    * Create Range from global offsets. Used for backward compatibility with older API.
    */
   public TextRange newRange(int startOffset, int endOffset) {
+    Preconditions.checkArgument(startOffset < endOffset, "Start offset %s should be strictly before end offset %s", startOffset, endOffset);
     return newRangeValidPointers(newPointer(startOffset), newPointer(endOffset));
   }
 
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java
index 0a9c84c11d0..af4c145713f 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java
@@ -21,6 +21,7 @@ package org.sonar.api.batch.sensor.highlighting;
 
 import com.google.common.annotations.Beta;
 import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.batch.fs.TextRange;
 
 /**
  * This builder is used to define syntax highlighting (aka code coloration) on files.
@@ -43,6 +44,21 @@ public interface NewHighlighting {
   NewHighlighting highlight(int startOffset, int endOffset, TypeOfText typeOfText);
 
   /**
+   * Call this method to indicate the type of text in a range.
+   * @param range Range of text to highlight. See for example {@link InputFile#newRange(int, int, int, int)}.
+   * @param typeOfText see {@link TypeOfText} values.
+   * @since 5.6
+   */
+  NewHighlighting highlight(TextRange range, TypeOfText typeOfText);
+
+  /**
+   * Shortcut to avoid calling {@link InputFile#newRange(int, int, int, int)}
+   * @param typeOfText see {@link TypeOfText} values.
+   * @since 5.6
+   */
+  NewHighlighting highlight(int startLine, int startLineOffset, int endLine, int endLineOffset, TypeOfText typeOfText);
+
+  /**
    * Call this method only once when your are done with defining highlighting of the file.
    * @throws IllegalStateException if you have defined overlapping highlighting
    */
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java
index e9cdc68c2d4..6ccc541dad3 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java
@@ -76,22 +76,38 @@ public class DefaultHighlighting extends DefaultStorable implements NewHighlight
 
   @Override
   public DefaultHighlighting highlight(int startOffset, int endOffset, TypeOfText typeOfText) {
-    Preconditions.checkState(inputFile != null, "Call onFile() first");
+    checkInputFileNotNull();
     TextRange newRange;
     try {
-      Preconditions.checkArgument(startOffset < endOffset, "start offset should be strictly before end offset");
       newRange = inputFile.newRange(startOffset, endOffset);
     } catch (Exception e) {
-      throw new IllegalArgumentException("Unable to highlight file " + inputFile + " from offset " + startOffset + " to offset " + endOffset, e);
+      throw new IllegalArgumentException("Unable to highlight file " + inputFile, e);
+    }
+    return highlight(newRange, typeOfText);
+  }
+
+  @Override
+  public DefaultHighlighting highlight(int startLine, int startLineOffset, int endLine, int endLineOffset, TypeOfText typeOfText) {
+    checkInputFileNotNull();
+    TextRange newRange;
+    try {
+      newRange = inputFile.newRange(startLine, startLineOffset, endLine, endLineOffset);
+    } catch (Exception e) {
+      throw new IllegalArgumentException("Unable to highlight file " + inputFile, e);
     }
-    SyntaxHighlightingRule syntaxHighlightingRule = SyntaxHighlightingRule.create(newRange, typeOfText);
+    return highlight(newRange, typeOfText);
+  }
+
+  @Override
+  public DefaultHighlighting highlight(TextRange range, TypeOfText typeOfText) {
+    SyntaxHighlightingRule syntaxHighlightingRule = SyntaxHighlightingRule.create(range, typeOfText);
     this.syntaxHighlightingRules.add(syntaxHighlightingRule);
     return this;
   }
 
   @Override
   protected void doSave() {
-    Preconditions.checkState(inputFile != null, "Call onFile() first");
+    checkInputFileNotNull();
     // Sort rules to avoid variation during consecutive runs
     Collections.sort(syntaxHighlightingRules, new Comparator<SyntaxHighlightingRule>() {
       @Override
@@ -106,4 +122,8 @@ public class DefaultHighlighting extends DefaultStorable implements NewHighlight
     checkOverlappingBoudaries();
     storage.store(this);
   }
+
+  private void checkInputFileNotNull() {
+    Preconditions.checkState(inputFile != null, "Call onFile() first");
+  }
 }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java
index 59d939ab128..14cae212525 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java
@@ -54,7 +54,7 @@ public class DefaultHighlightingTest {
     DefaultHighlighting highlightingDataBuilder = new DefaultHighlighting(mock(SensorStorage.class))
       .onFile(INPUT_FILE)
       .highlight(0, 10, COMMENT)
-      .highlight(10, 12, KEYWORD)
+      .highlight(1, 10, 1, 12, KEYWORD)
       .highlight(24, 38, KEYWORD)
       .highlight(42, 50, KEYWORD)
       .highlight(24, 65, CPP_DOC)
@@ -76,17 +76,18 @@ public class DefaultHighlightingTest {
 
   @Test
   public void should_order_by_start_then_end_offset() {
-    assertThat(highlightingRules).extracting("range", TextRange.class).containsExactly(rangeOf(1, 0, 1, 10),
+    assertThat(highlightingRules).extracting("range", TextRange.class).containsExactly(
+      rangeOf(1, 0, 1, 10),
       rangeOf(1, 10, 1, 12),
       rangeOf(1, 12, 1, 20),
       rangeOf(1, 24, 2, 15),
       rangeOf(1, 24, 1, 38),
       rangeOf(1, 42, 2, 0));
-    assertThat(highlightingRules).extracting("textType").containsOnly(COMMENT, KEYWORD, COMMENT, KEYWORD, CPP_DOC, KEYWORD);
+    assertThat(highlightingRules).extracting("textType").containsExactly(COMMENT, KEYWORD, COMMENT, CPP_DOC, KEYWORD, KEYWORD);
   }
 
   @Test
-  public void should_suport_overlapping() {
+  public void should_support_overlapping() {
     new DefaultHighlighting(mock(SensorStorage.class))
       .onFile(INPUT_FILE)
       .highlight(0, 15, KEYWORD)
@@ -107,6 +108,18 @@ public class DefaultHighlightingTest {
   }
 
   @Test
+  public void should_prevent_start_equal_end2() {
+    throwable.expect(IllegalArgumentException.class);
+    throwable
+      .expectMessage("Unable to highlight file");
+
+    new DefaultHighlighting(mock(SensorStorage.class))
+      .onFile(INPUT_FILE)
+      .highlight(1, 10, 1, 10, KEYWORD)
+      .save();
+  }
+
+  @Test
   public void should_prevent_boudaries_overlapping() {
     throwable.expect(IllegalStateException.class);
     throwable
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/batch/sensor/noop/NoOpNewHighlighting.java b/sonar-scanner-engine/src/main/java/org/sonar/batch/sensor/noop/NoOpNewHighlighting.java
index 7ead32f7f04..c286107ffe7 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/batch/sensor/noop/NoOpNewHighlighting.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/batch/sensor/noop/NoOpNewHighlighting.java
@@ -20,6 +20,7 @@
 package org.sonar.batch.sensor.noop;
 
 import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.batch.fs.TextRange;
 import org.sonar.api.batch.sensor.highlighting.NewHighlighting;
 import org.sonar.api.batch.sensor.highlighting.TypeOfText;
 
@@ -30,13 +31,25 @@ public class NoOpNewHighlighting implements NewHighlighting {
   }
 
   @Override
-  public NewHighlighting onFile(InputFile inputFile) {
+  public NoOpNewHighlighting onFile(InputFile inputFile) {
     // Do nothing
     return this;
   }
 
   @Override
-  public NewHighlighting highlight(int startOffset, int endOffset, TypeOfText typeOfText) {
+  public NoOpNewHighlighting highlight(int startOffset, int endOffset, TypeOfText typeOfText) {
+    // Do nothing
+    return this;
+  }
+
+  @Override
+  public NoOpNewHighlighting highlight(int startLine, int startLineOffset, int endLine, int endLineOffset, TypeOfText typeOfText) {
+    // Do nothing
+    return this;
+  }
+
+  @Override
+  public NoOpNewHighlighting highlight(TextRange range, TypeOfText typeOfText) {
     // Do nothing
     return this;
   }