Fix some quality flaws

6 files changed, 96 insertions, 76 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java
index a5a33733993..863bdc4c4b0 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java
@@ -20,14 +20,15 @@
 
 package org.sonar.server.computation.source;
 
+import org.sonar.batch.protocol.output.BatchReport;
+import org.sonar.db.protobuf.DbFileSources;
+
 import java.io.Serializable;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import org.sonar.batch.protocol.output.BatchReport;
-import org.sonar.db.protobuf.DbFileSources;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
@@ -82,10 +83,10 @@ public class DuplicationLineReader implements LineReader {
     return (range.getEndLine() - range.getStartLine()) + 1;
   }
 
-  private Map<BatchReport.TextRange, Integer> createDuplicationIdsByRange(List<BatchReport.Duplication> duplications) {
+  private static Map<BatchReport.TextRange, Integer> createDuplicationIdsByRange(List<BatchReport.Duplication> duplications) {
     Map<BatchReport.TextRange, Integer> map = newHashMap();
     int blockId = 1;
-    for (BatchReport.Duplication duplication : this.duplications) {
+    for (BatchReport.Duplication duplication : duplications) {
       map.put(duplication.getOriginPosition(), blockId);
       blockId++;
       for (BatchReport.Duplicate duplicate : duplication.getDuplicateList()) {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java
index 064b298eee5..3286c6d3c0b 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java
@@ -21,15 +21,20 @@
 package org.sonar.server.computation.source;
 
 import com.google.common.collect.ImmutableMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import javax.annotation.CheckForNull;
 import org.sonar.batch.protocol.Constants;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.db.protobuf.DbFileSources;
 
+import javax.annotation.CheckForNull;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
 import static com.google.common.collect.Lists.newArrayList;
+import static org.sonar.server.computation.source.RangeOffsetHelper.OFFSET_SEPARATOR;
+import static org.sonar.server.computation.source.RangeOffsetHelper.SYMBOLS_SEPARATOR;
+import static org.sonar.server.computation.source.RangeOffsetHelper.offsetToString;
 
 public class HighlightingLineReader implements LineReader {
 
@@ -62,30 +67,36 @@ public class HighlightingLineReader implements LineReader {
 
     incrementHighlightingListMatchingLine(line);
     for (Iterator<BatchReport.SyntaxHighlighting> syntaxHighlightingIterator = highlightingList.iterator(); syntaxHighlightingIterator.hasNext();) {
-      BatchReport.SyntaxHighlighting syntaxHighlighting = syntaxHighlightingIterator.next();
-      BatchReport.TextRange range = syntaxHighlighting.getRange();
-      if (range.getStartLine() <= line) {
-        String offsets = RangeOffsetHelper.offsetToString(syntaxHighlighting.getRange(), line, lineBuilder.getSource().length());
-        if (!offsets.isEmpty()) {
-          if (highlighting.length() > 0) {
-            highlighting.append(RangeOffsetHelper.SYMBOLS_SEPARATOR);
-          }
-          highlighting.append(offsets)
-            .append(RangeOffsetHelper.OFFSET_SEPARATOR)
-            .append(getCssClass(syntaxHighlighting.getType()));
-          if (range.getEndLine() == line) {
-            syntaxHighlightingIterator.remove();
-          }
-        } else {
-          syntaxHighlightingIterator.remove();
-        }
-      }
+      processHighlighting(syntaxHighlightingIterator, highlighting, lineBuilder);
     }
     if (highlighting.length() > 0) {
       lineBuilder.setHighlighting(highlighting.toString());
     }
   }
 
+  private static void processHighlighting(Iterator<BatchReport.SyntaxHighlighting> syntaxHighlightingIterator, StringBuilder highlighting,
+    DbFileSources.Line.Builder lineBuilder) {
+    BatchReport.SyntaxHighlighting syntaxHighlighting = syntaxHighlightingIterator.next();
+    int line = lineBuilder.getLine();
+    BatchReport.TextRange range = syntaxHighlighting.getRange();
+    if (range.getStartLine() <= line) {
+      String offsets = offsetToString(syntaxHighlighting.getRange(), line, lineBuilder.getSource().length());
+      if (!offsets.isEmpty()) {
+        if (highlighting.length() > 0) {
+          highlighting.append(SYMBOLS_SEPARATOR);
+        }
+        highlighting.append(offsets)
+          .append(OFFSET_SEPARATOR)
+          .append(getCssClass(syntaxHighlighting.getType()));
+        if (range.getEndLine() == line) {
+          syntaxHighlightingIterator.remove();
+        }
+      } else {
+        syntaxHighlightingIterator.remove();
+      }
+    }
+  }
+
   private static String getCssClass(Constants.HighlightingType type) {
     String cssClass = cssClassByType.get(type);
     if (cssClass != null) {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetHelper.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetConverter.java
index 35402dace4c..368320b6932 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetHelper.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetConverter.java
@@ -22,12 +22,14 @@ package org.sonar.server.computation.source;
 
 import org.sonar.batch.protocol.output.BatchReport;
 
-public class RangeOffsetHelper {
+import static java.lang.String.format;
+
+public class RangeOffsetConverter {
 
   static final String OFFSET_SEPARATOR = ",";
   static final String SYMBOLS_SEPARATOR = ";";
 
-  private RangeOffsetHelper() {
+  private RangeOffsetConverter() {
     // Only static methods
   }
 
@@ -51,19 +53,19 @@ public class RangeOffsetHelper {
 
   private static void validateOffsetOrder(BatchReport.TextRange range, int line) {
     if (range.getStartLine() == range.getEndLine() && range.getStartOffset() > range.getEndOffset()) {
-      throw new IllegalArgumentException(String.format("End offset %s cannot be defined before start offset %s on line %s", range.getEndOffset(), range.getStartOffset(), line));
+      throw new IllegalArgumentException(format("End offset %s cannot be defined before start offset %s on line %s", range.getEndOffset(), range.getStartOffset(), line));
     }
   }
 
   private static void validateStartOffsetNotGreaterThanLineLength(BatchReport.TextRange range, int lineLength, int line) {
     if (range.getStartLine() == line && range.getStartOffset() > lineLength) {
-      throw new IllegalArgumentException(String.format("Start offset %s is defined outside the length (%s) of the line %s", range.getStartOffset(), lineLength, line));
+      throw new IllegalArgumentException(format("Start offset %s is defined outside the length (%s) of the line %s", range.getStartOffset(), lineLength, line));
     }
   }
 
   private static void validateEndOffsetNotGreaterThanLineLength(BatchReport.TextRange range, int lineLength, int line) {
     if (range.getEndLine() == line && range.getEndOffset() > lineLength) {
-      throw new IllegalArgumentException(String.format("End offset %s is defined outside the length (%s) of the line %s", range.getEndOffset(), lineLength, line));
+      throw new IllegalArgumentException(format("End offset %s is defined outside the length (%s) of the line %s", range.getEndOffset(), lineLength, line));
     }
   }
 
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java
index ae980a7c02a..83586978b91 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java
@@ -21,6 +21,9 @@
 package org.sonar.server.computation.source;
 
 import com.google.common.collect.Lists;
+import org.sonar.batch.protocol.output.BatchReport;
+import org.sonar.db.protobuf.DbFileSources;
+
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -31,8 +34,10 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import org.sonar.batch.protocol.output.BatchReport;
-import org.sonar.db.protobuf.DbFileSources;
+
+import static org.sonar.server.computation.source.RangeOffsetHelper.OFFSET_SEPARATOR;
+import static org.sonar.server.computation.source.RangeOffsetHelper.SYMBOLS_SEPARATOR;
+import static org.sonar.server.computation.source.RangeOffsetHelper.offsetToString;
 
 public class SymbolsLineReader implements LineReader {
 
@@ -69,13 +74,13 @@ public class SymbolsLineReader implements LineReader {
 
   private static void appendSymbol(StringBuilder lineSymbol, BatchReport.TextRange range, int line, int symbolId, String sourceLine) {
     if (matchLine(range, line)) {
-      String offsets = RangeOffsetHelper.offsetToString(range, line, sourceLine.length());
+      String offsets = offsetToString(range, line, sourceLine.length());
       if (!offsets.isEmpty()) {
         if (lineSymbol.length() > 0) {
-          lineSymbol.append(RangeOffsetHelper.SYMBOLS_SEPARATOR);
+          lineSymbol.append(SYMBOLS_SEPARATOR);
         }
         lineSymbol.append(offsets)
-          .append(RangeOffsetHelper.OFFSET_SEPARATOR)
+          .append(OFFSET_SEPARATOR)
           .append(symbolId);
       }
     }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java
index 21ff8a5aee8..dcbec7dfebc 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java
@@ -20,19 +20,21 @@
 
 package org.sonar.server.computation.source;
 
-import java.util.Collections;
 import org.junit.Test;
 import org.sonar.batch.protocol.Constants;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.db.protobuf.DbFileSources;
 
+import java.util.Collections;
+
 import static com.google.common.collect.Lists.newArrayList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown;
+import static org.sonar.db.protobuf.DbFileSources.Data.newBuilder;
 
 public class HighlightingLineReaderTest {
 
-  DbFileSources.Data.Builder sourceData = DbFileSources.Data.newBuilder();
+  DbFileSources.Data.Builder sourceData = newBuilder();
   DbFileSources.Line.Builder line1 = sourceData.addLinesBuilder().setSource("line1").setLine(1);
   DbFileSources.Line.Builder line2 = sourceData.addLinesBuilder().setSource("line2").setLine(2);
   DbFileSources.Line.Builder line3 = sourceData.addLinesBuilder().setSource("line3").setLine(3);
@@ -42,7 +44,7 @@ public class HighlightingLineReaderTest {
   public void nothing_to_read() {
     HighlightingLineReader highlightingLineReader = new HighlightingLineReader(Collections.<BatchReport.SyntaxHighlighting>emptyList().iterator());
 
-    DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1);
+    DbFileSources.Line.Builder lineBuilder = newBuilder().addLinesBuilder().setLine(1);
     highlightingLineReader.read(lineBuilder);
 
     assertThat(lineBuilder.hasHighlighting()).isFalse();
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetHelperTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetConverterTest.java
index a2d9a7ca80e..0a7171be0c7 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetHelperTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetConverterTest.java
@@ -20,17 +20,22 @@
 
 package org.sonar.server.computation.source;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.sonar.batch.protocol.output.BatchReport;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Fail.failBecauseExceptionWasNotThrown;
+import static org.sonar.server.computation.source.RangeOffsetConverter.offsetToString;
 
-public class RangeOffsetHelperTest {
+public class RangeOffsetConverterTest {
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
 
   @Test
   public void append_range() {
-    assertThat(RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
+    assertThat(offsetToString(BatchReport.TextRange.newBuilder()
       .setStartLine(1).setEndLine(1)
       .setStartOffset(2).setEndOffset(3)
       .build(), 1, 5)).isEqualTo("2,3");
@@ -38,7 +43,7 @@ public class RangeOffsetHelperTest {
 
   @Test
   public void append_range_not_finishing_in_current_line() {
-    assertThat(RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
+    assertThat(offsetToString(BatchReport.TextRange.newBuilder()
       .setStartLine(1).setEndLine(3)
       .setStartOffset(2).setEndOffset(3)
       .build(), 1, 5)).isEqualTo("2,5");
@@ -46,7 +51,7 @@ public class RangeOffsetHelperTest {
 
   @Test
   public void append_range_that_began_in_previous_line_and_finish_in_current_line() {
-    assertThat(RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
+    assertThat(offsetToString(BatchReport.TextRange.newBuilder()
       .setStartLine(1).setEndLine(3)
       .setStartOffset(2).setEndOffset(3)
       .build(), 3, 5)).isEqualTo("0,3");
@@ -54,7 +59,7 @@ public class RangeOffsetHelperTest {
 
   @Test
   public void append_range_that_began_in_previous_line_and_not_finishing_in_current_line() {
-    assertThat(RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
+    assertThat(offsetToString(BatchReport.TextRange.newBuilder()
       .setStartLine(1).setEndLine(3)
       .setStartOffset(2).setEndOffset(3)
       .build(), 2, 5)).isEqualTo("0,5");
@@ -62,7 +67,7 @@ public class RangeOffsetHelperTest {
 
   @Test
   public void do_nothing_if_offset_is_empty() {
-    assertThat(RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
+    assertThat(offsetToString(BatchReport.TextRange.newBuilder()
       .setStartLine(1).setEndLine(1)
       .setStartOffset(0).setEndOffset(0)
       .build(), 1, 5)).isEmpty();
@@ -70,43 +75,37 @@ public class RangeOffsetHelperTest {
 
   @Test
   public void fail_when_end_offset_is_before_start_offset() {
-    try {
-      RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
-        .setStartLine(1).setEndLine(1)
-        .setStartOffset(4).setEndOffset(2)
-        .build(),
-        1, 5);
-      failBecauseExceptionWasNotThrown(IllegalArgumentException.class);
-    } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("End offset 2 cannot be defined before start offset 4 on line 1");
-    }
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("End offset 2 cannot be defined before start offset 4 on line 1");
+
+    offsetToString(BatchReport.TextRange.newBuilder()
+      .setStartLine(1).setEndLine(1)
+      .setStartOffset(4).setEndOffset(2)
+      .build(),
+      1, 5);
   }
 
   @Test
   public void fail_when_end_offset_is_higher_than_line_length() {
-    try {
-      RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
-        .setStartLine(1).setEndLine(1)
-        .setStartOffset(4).setEndOffset(10)
-        .build(),
-        1, 5);
-      failBecauseExceptionWasNotThrown(IllegalArgumentException.class);
-    } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("End offset 10 is defined outside the length (5) of the line 1");
-    }
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("End offset 10 is defined outside the length (5) of the line 1");
+
+    offsetToString(BatchReport.TextRange.newBuilder()
+      .setStartLine(1).setEndLine(1)
+      .setStartOffset(4).setEndOffset(10)
+      .build(),
+      1, 5);
   }
 
   @Test
   public void fail_when_start_offset_is_higher_than_line_length() {
-    try {
-      RangeOffsetHelper.offsetToString(BatchReport.TextRange.newBuilder()
-        .setStartLine(1).setEndLine(1)
-        .setStartOffset(10).setEndOffset(11)
-        .build(),
-        1, 5);
-      failBecauseExceptionWasNotThrown(IllegalArgumentException.class);
-    } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Start offset 10 is defined outside the length (5) of the line 1");
-    }
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Start offset 10 is defined outside the length (5) of the line 1");
+
+    offsetToString(BatchReport.TextRange.newBuilder()
+      .setStartLine(1).setEndLine(1)
+      .setStartOffset(10).setEndOffset(11)
+      .build(),
+      1, 5);
   }
 }