Skip to content

Commit

Permalink
SONAR-6175 Validate that offsets provided for highlighting and symbol…
Browse files Browse the repository at this point in the history
… reference are val
  • Loading branch information
henryju committed Feb 20, 2015
1 parent 237bc20 commit 4434c2a
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void testNoExecutionIfNoSyntaxFile() {
public void testExecution() throws IOException {
File symbol = new File(baseDir, "src/foo.xoo.highlighting");
FileUtils.write(symbol, "1:4:k\n12:15:cppd\n\n#comment");
DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setLanguage("xoo");
DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setLanguage("xoo").setLastValidOffset(100);
context.fileSystem().add(inputFile);

sensor.execute(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ void eof() {
private static class LineOffsetCounter extends CharHandler {
private int currentOriginalOffset = 0;
private List<Integer> originalLineOffsets = new ArrayList<Integer>();
private int lastValidOffset = 0;

public LineOffsetCounter() {
originalLineOffsets.add(0);
Expand All @@ -215,10 +216,19 @@ void newLine() {
originalLineOffsets.add(currentOriginalOffset);
}

@Override
void eof() {
lastValidOffset = currentOriginalOffset;
}

public List<Integer> getOriginalLineOffsets() {
return originalLineOffsets;
}

public int getLastValidOffset() {
return lastValidOffset;
}

}

/**
Expand All @@ -236,6 +246,7 @@ Metadata read(File file, Charset encoding) {
scanFile(file, encoding, lineCounter, fileHashComputer);
}
return new Metadata(lineCounter.lines(), lineCounter.nonBlankLines(), fileHashComputer.getHash(), lineOffsetCounter.getOriginalLineOffsets(),
lineOffsetCounter.getLastValidOffset(),
lineCounter.isEmpty());
}

Expand Down Expand Up @@ -288,14 +299,16 @@ static class Metadata {
final int nonBlankLines;
final String hash;
final int[] originalLineOffsets;
final int lastValidOffset;
final boolean empty;

private Metadata(int lines, int nonBlankLines, String hash, List<Integer> originalLineOffsets, boolean empty) {
private Metadata(int lines, int nonBlankLines, String hash, List<Integer> originalLineOffsets, int lastValidOffset, boolean empty) {
this.lines = lines;
this.nonBlankLines = nonBlankLines;
this.hash = hash;
this.empty = empty;
this.originalLineOffsets = Ints.toArray(originalLineOffsets);
this.lastValidOffset = lastValidOffset;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ InputFileMetadata completeAndComputeMetadata(DeprecatedDefaultInputFile inputFil

FileMetadata.Metadata metadata = fileMetadata.read(inputFile.file(), fs.encoding());
inputFile.setLines(metadata.lines);
inputFile.setLastValidOffset(metadata.lastValidOffset);

result.setNonBlankLines(metadata.nonBlankLines);
result.setHash(metadata.hash);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ private File getFile(InputFile file) {
public void store(org.sonar.api.batch.sensor.dependency.Dependency dep) {
BatchResource fromBatchResource = resourceCache.get(dep.fromKey());
BatchResource toBatchResource = resourceCache.get(dep.toKey());
Preconditions.checkNotNull(fromBatchResource, "Unable to find from resource " + dep.fromKey());
Preconditions.checkNotNull(toBatchResource, "Unable to find from resource " + dep.toKey());
Preconditions.checkNotNull(fromBatchResource, "Unable to find origin resource " + dep.fromKey());
Preconditions.checkNotNull(toBatchResource, "Unable to find destination resource " + dep.toKey());
File fromResource = (File) fromBatchResource.resource();
File toResource = (File) toBatchResource.resource();
if (sonarIndex.getEdge(fromResource, toResource) != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/
package org.sonar.batch.mediumtest.highlighting;

import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import org.apache.commons.io.FileUtils;
import org.junit.After;
Expand Down Expand Up @@ -102,8 +103,8 @@ public void computeSyntaxHighlightingOnBigFile() throws IOException {

File xooFile = new File(srcDir, "sample.xoo");
File xoohighlightingFile = new File(srcDir, "sample.xoo.highlighting");
FileUtils.write(xooFile, "Sample xoo\ncontent");
int chunkSize = 100000;
FileUtils.write(xooFile, Strings.repeat("a", chunkSize));
StringBuilder sb = new StringBuilder(16 * chunkSize);
for (int i = 0; i < chunkSize; i++) {
sb.append(i).append(":").append(i + 1).append(":s\n");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void testCheckWithTemplate() throws IOException {
srcDir.mkdir();

File xooFile = new File(srcDir, "sample.xoo");
FileUtils.write(xooFile, "foo");
FileUtils.write(xooFile, "foo\nbar");

TaskResult result = tester.newTask()
.properties(ImmutableMap.<String, String>builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public void empty_file() throws Exception {
assertThat(metadata.nonBlankLines).isEqualTo(0);
assertThat(metadata.hash).isNotEmpty();
assertThat(metadata.originalLineOffsets).containsOnly(0);
assertThat(metadata.lastValidOffset).isEqualTo(0);
assertThat(metadata.empty).isTrue();
}

Expand All @@ -67,6 +68,7 @@ public void windows_without_latest_eol() throws Exception {
assertThat(metadata.nonBlankLines).isEqualTo(3);
assertThat(metadata.hash).isEqualTo(md5Hex("foo\nbar\nbaz"));
assertThat(metadata.originalLineOffsets).containsOnly(0, 5, 10);
assertThat(metadata.lastValidOffset).isEqualTo(13);
assertThat(metadata.empty).isFalse();
}

Expand Down Expand Up @@ -115,6 +117,7 @@ public void unix_without_latest_eol() throws Exception {
assertThat(metadata.nonBlankLines).isEqualTo(3);
assertThat(metadata.hash).isEqualTo(md5Hex("foo\nbar\nbaz"));
assertThat(metadata.originalLineOffsets).containsOnly(0, 4, 8);
assertThat(metadata.lastValidOffset).isEqualTo(11);
}

@Test
Expand All @@ -127,6 +130,7 @@ public void unix_with_latest_eol() throws Exception {
assertThat(metadata.nonBlankLines).isEqualTo(3);
assertThat(metadata.hash).isEqualTo(md5Hex("foo\nbar\nbaz\n"));
assertThat(metadata.originalLineOffsets).containsOnly(0, 4, 8, 12);
assertThat(metadata.lastValidOffset).isEqualTo(12);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public void shouldSaveProjectMeasureToSensorContext() {

@Test
public void shouldAddIssueOnFile() {
InputFile file = new DefaultInputFile("foo", "src/Foo.php");
InputFile file = new DefaultInputFile("foo", "src/Foo.php").setLines(4);

ArgumentCaptor<Issue> argumentCaptor = ArgumentCaptor.forClass(Issue.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class DefaultInputFile implements InputFile, Serializable {
private Status status;
private int lines;
private Charset charset;
private int lastValidOffset;

public DefaultInputFile(String moduleKey, String relativePath) {
this.moduleKey = moduleKey;
Expand Down Expand Up @@ -144,6 +145,15 @@ public DefaultInputFile setCharset(Charset charset) {
return this;
}

public int lastValidOffset() {
return lastValidOffset;
}

public DefaultInputFile setLastValidOffset(int lastValidOffset) {
this.lastValidOffset = lastValidOffset;
return this;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.fs.internal.DefaultInputFile;
import org.sonar.api.batch.sensor.highlighting.NewHighlighting;
import org.sonar.api.batch.sensor.highlighting.TypeOfText;
import org.sonar.api.batch.sensor.internal.DefaultStorable;
Expand Down Expand Up @@ -91,12 +92,21 @@ public InputFile inputFile() {
@Override
public DefaultHighlighting highlight(int startOffset, int endOffset, TypeOfText typeOfText) {
Preconditions.checkState(inputFile != null, "Call onFile() first");
int maxValidOffset = ((DefaultInputFile) inputFile).lastValidOffset();
checkOffset(startOffset, maxValidOffset, "startOffset");
checkOffset(endOffset, maxValidOffset, "endOffset");
Preconditions.checkArgument(startOffset < endOffset, "startOffset (" + startOffset + ") should be < endOffset (" + endOffset + ") for file " + inputFile + ".");
SyntaxHighlightingRule syntaxHighlightingRule = SyntaxHighlightingRule.create(startOffset, endOffset,
typeOfText);
this.syntaxHighlightingRuleSet.add(syntaxHighlightingRule);
return this;
}

private void checkOffset(int offset, int maxValidOffset, String label) {
Preconditions.checkArgument(offset >= 0 && offset <= maxValidOffset, "Invalid " + label + " " + offset + ". Should be >= 0 and <= " + maxValidOffset
+ " for file " + inputFile);
}

@Override
protected void doSave() {
Preconditions.checkState(inputFile != null, "Call onFile() first");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class DefaultHighlightingTest {
public void setUpSampleRules() {

DefaultHighlighting highlightingDataBuilder = new DefaultHighlighting()
.onFile(new DefaultInputFile("foo", "src/Foo.java"))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100))
.highlight(0, 10, COMMENT)
.highlight(10, 12, KEYWORD)
.highlight(24, 38, KEYWORD)
Expand All @@ -71,7 +71,7 @@ public void should_order_by_start_then_end_offset() throws Exception {
@Test
public void should_suport_overlapping() throws Exception {
new DefaultHighlighting(mock(SensorStorage.class))
.onFile(new DefaultInputFile("foo", "src/Foo.java"))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100))
.highlight(0, 15, KEYWORD)
.highlight(8, 12, CPP_DOC)
.save();
Expand All @@ -83,10 +83,46 @@ public void should_prevent_boudaries_overlapping() throws Exception {
throwable.expectMessage("Cannot register highlighting rule for characters from 8 to 15 as it overlaps at least one existing rule");

new DefaultHighlighting(mock(SensorStorage.class))
.onFile(new DefaultInputFile("foo", "src/Foo.java"))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100))
.highlight(0, 10, KEYWORD)
.highlight(8, 15, KEYWORD)
.save();
}

@Test
public void should_prevent_invalid_offset() throws Exception {
throwable.expect(IllegalArgumentException.class);
throwable.expectMessage("Invalid endOffset 15. Should be >= 0 and <= 10 for file [moduleKey=foo, relative=src/Foo.java, basedir=null]");

new DefaultHighlighting(mock(SensorStorage.class))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(10))
.highlight(0, 10, KEYWORD)
.highlight(8, 15, KEYWORD)
.save();
}

@Test
public void positive_offset() throws Exception {
throwable.expect(IllegalArgumentException.class);
throwable.expectMessage("Invalid startOffset -8. Should be >= 0 and <= 10 for file [moduleKey=foo, relative=src/Foo.java, basedir=null]");

new DefaultHighlighting(mock(SensorStorage.class))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(10))
.highlight(0, 10, KEYWORD)
.highlight(-8, 15, KEYWORD)
.save();
}

@Test
public void should_prevent_invalid_offset_order() throws Exception {
throwable.expect(IllegalArgumentException.class);
throwable.expectMessage("startOffset (18) should be < endOffset (15) for file [moduleKey=foo, relative=src/Foo.java, basedir=null].");

new DefaultHighlighting(mock(SensorStorage.class))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(20))
.highlight(0, 10, KEYWORD)
.highlight(18, 15, KEYWORD)
.save();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ public void testIssues() {
assertThat(tester.issues("foo:src/Foo.java")).isEmpty();
assertThat(tester.allIssues()).isEmpty();
tester.newIssue()
.onFile(new DefaultInputFile("foo", "src/Foo.java"))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLines(10))
.forRule(RuleKey.of("repo", "rule"))
.atLine(1)
.save();
tester.newIssue()
.onFile(new DefaultInputFile("foo", "src/Foo.java"))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLines(10))
.forRule(RuleKey.of("repo", "rule"))
.atLine(3)
.save();
Expand Down Expand Up @@ -144,7 +144,7 @@ public void testMeasures() {
public void testHighlighting() {
assertThat(tester.highlightingTypeFor("foo:src/Foo.java", 3)).isEmpty();
tester.newHighlighting()
.onFile(new DefaultInputFile("foo", "src/Foo.java"))
.onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100))
.highlight(0, 4, TypeOfText.ANNOTATION)
.highlight(8, 10, TypeOfText.CONSTANT)
.highlight(9, 10, TypeOfText.COMMENT)
Expand Down

0 comments on commit 4434c2a

Please sign in to comment.