Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analysis should not fail when highlighting has invalid text range #4604

Merged
merged 3 commits into from Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -191,10 +191,16 @@ private void saveIssues(List<Issue> issues) {
private void saveHighlights(BridgeServer.Highlight[] highlights) {
NewHighlighting highlighting = context.newHighlighting().onFile(file);
for (BridgeServer.Highlight highlight : highlights) {
highlighting.highlight(
highlight.location.toTextRange(file),
TypeOfText.valueOf(highlight.textType)
);
try {
highlighting.highlight(
highlight.location.toTextRange(file),
TypeOfText.valueOf(highlight.textType)
);
} catch (IllegalArgumentException e) {
LOG.warn("Failed to save highlight in {} at {}", file.uri(), highlight.location);
LOG.warn("Exception cause", e);
// continue processing other highlights
}
}
highlighting.save();
}
Expand All @@ -203,19 +209,30 @@ private void saveHighlightedSymbols(BridgeServer.HighlightedSymbol[] highlighted
NewSymbolTable symbolTable = context.newSymbolTable().onFile(file);
for (BridgeServer.HighlightedSymbol highlightedSymbol : highlightedSymbols) {
BridgeServer.Location declaration = highlightedSymbol.declaration;
NewSymbol newSymbol = symbolTable.newSymbol(
declaration.startLine,
declaration.startCol,
declaration.endLine,
declaration.endCol
);
NewSymbol newSymbol;
try {
newSymbol =
symbolTable.newSymbol(
declaration.startLine,
declaration.startCol,
declaration.endLine,
declaration.endCol
);
} catch (IllegalArgumentException e) {
LOG.warn("Failed to create symbol declaration in {} at {}", file.uri(), declaration);
continue;
}
for (BridgeServer.Location reference : highlightedSymbol.references) {
newSymbol.newReference(
reference.startLine,
reference.startCol,
reference.endLine,
reference.endCol
);
try {
newSymbol.newReference(
reference.startLine,
reference.startCol,
reference.endLine,
reference.endCol
);
} catch (IllegalArgumentException e) {
LOG.warn("Failed to create symbol reference in {} at {}", file.uri(), reference);
}
}
}
symbolTable.save();
Expand Down
Expand Up @@ -195,6 +195,15 @@ class Location {
int endLine;
int endCol;

public Location() {}

Location(int startLine, int startCol, int endLine, int endCol) {
this.startLine = startLine;
this.startCol = startCol;
this.endLine = endLine;
this.endCol = endCol;
}

public int getStartLine() {
return startLine;
}
Expand Down Expand Up @@ -230,6 +239,11 @@ public void setEndCol(int endCol) {
TextRange toTextRange(InputFile inputFile) {
return inputFile.newRange(this.startLine, this.startCol, this.endLine, this.endCol);
}

@Override
public String toString() {
return String.format("%d:%d-%d:%d", startLine, startCol, endLine, endCol);
}
}

class Metrics {
Expand Down
@@ -0,0 +1,70 @@
package org.sonar.plugins.javascript.bridge;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.nio.file.Path;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
import org.sonar.api.batch.sensor.internal.SensorContextTester;
import org.sonar.api.issue.NoSonarFilter;
import org.sonar.api.measures.FileLinesContext;
import org.sonar.api.measures.FileLinesContextFactory;
import org.sonar.api.testfixtures.log.LogTesterJUnit5;

class AnalysisProcessorTest {

@TempDir
Path baseDir;

@org.junit.jupiter.api.extension.RegisterExtension
LogTesterJUnit5 logTester = new LogTesterJUnit5();

@Test
void should_not_fail_when_invalid_range() {
var fileLinesContextFactory = mock(FileLinesContextFactory.class);
when(fileLinesContextFactory.createFor(any())).thenReturn(mock(FileLinesContext.class));
var processor = new AnalysisProcessor(mock(NoSonarFilter.class), fileLinesContextFactory);
var context = SensorContextTester.create(baseDir);
var file = TestInputFileBuilder
.create("moduleKey", "file.js")
.setContents("var x = 1;")
.build();
var response = new BridgeServer.AnalysisResponse();
var highlight = new BridgeServer.Highlight();
highlight.location = new BridgeServer.Location(1, 2, 1, 1); // invalid range startCol > endCol
response.highlights = new BridgeServer.Highlight[] { highlight };
processor.processResponse(context, mock(JsTsChecks.class), file, response);
assertThat(logTester.logs())
.contains("Failed to save highlight in " + file.uri() + " at 1:2-1:1");
}

@Test
void should_not_fail_when_invalid_symbol() {
var fileLinesContextFactory = mock(FileLinesContextFactory.class);
when(fileLinesContextFactory.createFor(any())).thenReturn(mock(FileLinesContext.class));
var processor = new AnalysisProcessor(mock(NoSonarFilter.class), fileLinesContextFactory);
var context = SensorContextTester.create(baseDir);
var file = TestInputFileBuilder
.create("moduleKey", "file.js")
.setContents("var x = 1;")
.build();
var response = new BridgeServer.AnalysisResponse();
var symbol = new BridgeServer.HighlightedSymbol();
symbol.declaration = new BridgeServer.Location(1, 2, 1, 1); // invalid range startCol > endCol
response.highlightedSymbols = new BridgeServer.HighlightedSymbol[] { symbol };
processor.processResponse(context, mock(JsTsChecks.class), file, response);
assertThat(logTester.logs())
.contains("Failed to create symbol declaration in " + file.uri() + " at 1:2-1:1");

context = SensorContextTester.create(baseDir);
symbol.declaration = new BridgeServer.Location(1, 1, 1, 2);
symbol.references = new BridgeServer.Location[] { new BridgeServer.Location(2, 2, 2, 1) };
processor.processResponse(context, mock(JsTsChecks.class), file, response);
assertThat(logTester.logs())
.contains("Failed to create symbol reference in " + file.uri() + " at 2:2-2:1");
}
}