Skip to content

Commit

Permalink
SONARJAVA-4963 Line and column positions are wrong after text blocks …
Browse files Browse the repository at this point in the history
…using '\' line continuations (#4864)
  • Loading branch information
alban-auzeill authored Sep 18, 2024
1 parent d543e1d commit 03e34a9
Show file tree
Hide file tree
Showing 11 changed files with 270 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ void invalid_file_should_make_verifier_fail() {

assertThat(e)
.isInstanceOf(AssertionError.class)
.hasMessage("Should not fail analysis (Parse error at line 1 column 8: Syntax error, insert \"}\" to complete ClassBody)");
.hasMessage("Should not fail analysis (Parse error at line 1 column 9: Syntax error, insert \"}\" to complete ClassBody)");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ void invalid_file_should_make_verifier_fail() {

assertThat(e)
.isInstanceOf(AssertionError.class)
.hasMessage("Should not fail analysis (Parse error at line 1 column 8: Syntax error, insert \"}\" to complete ClassBody)");
.hasMessage("Should not fail analysis (Parse error at line 1 column 9: Syntax error, insert \"}\" to complete ClassBody)");
}

@Test
Expand Down
62 changes: 24 additions & 38 deletions java-frontend/src/main/java/org/sonar/java/model/JParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,13 @@ public static JavaTree.CompilationUnitTreeImpl parse(ASTParser astParser, String
static JavaTree.CompilationUnitTreeImpl convert(String version, String unitName, String source, CompilationUnit astNode) {
List<IProblem> errors = Stream.of(astNode.getProblems()).filter(IProblem::isError).toList();
Optional<IProblem> possibleSyntaxError = errors.stream().filter(IS_SYNTAX_ERROR).findFirst();
LineColumnConverter lineColumnConverter = new LineColumnConverter(source);
if (possibleSyntaxError.isPresent()) {
IProblem syntaxError = possibleSyntaxError.get();
int line = syntaxError.getSourceLineNumber();
int column = astNode.getColumnNumber(syntaxError.getSourceStart());
String message = String.format("Parse error at line %d column %d: %s", line, column, syntaxError.getMessage());
LineColumnConverter.Pos pos = lineColumnConverter.toPos(syntaxError.getSourceStart());
String message = String.format("Parse error at line %d column %d: %s", pos.line(), pos.columnOffset() + 1, syntaxError.getMessage());
// interrupt parsing
throw new RecognitionException(line, message);
throw new RecognitionException(pos.line(), message);
}

Set<JProblem> undefinedTypes = errors.stream()
Expand All @@ -303,10 +303,11 @@ static JavaTree.CompilationUnitTreeImpl convert(String version, String unitName,
converter.sema.undefinedTypes.addAll(undefinedTypes);
converter.compilationUnit = astNode;
converter.tokenManager = new TokenManager(lex(version, unitName, source.toCharArray()), source, new DefaultCodeFormatterOptions(new HashMap<>()));
converter.lineColumnConverter = lineColumnConverter;

JavaTree.CompilationUnitTreeImpl tree = converter.convertCompilationUnit(astNode);
tree.sema = converter.sema;
JWarning.Mapper.warningsFor(astNode).mappedInto(tree);
JWarning.Mapper.warningsFor(astNode, converter.lineColumnConverter).mappedInto(tree);

ASTUtils.mayTolerateMissingType(astNode.getAST());

Expand Down Expand Up @@ -362,6 +363,7 @@ private static List<Token> lex(String version, String unitName, char[] sourceCha
private CompilationUnit compilationUnit;

private TokenManager tokenManager;
private LineColumnConverter lineColumnConverter;

private JSema sema;

Expand Down Expand Up @@ -444,43 +446,26 @@ private InternalSyntaxToken lastTokenIn(ASTNode e, int tokenType) {

private InternalSyntaxToken createSyntaxToken(int tokenIndex) {
Token t = tokenManager.get(tokenIndex);
String value;
boolean isEOF;
if (t.tokenType == TerminalTokens.TokenNameEOF) {
if (t.originalStart == 0) {
return new InternalSyntaxToken(1, 0, "", collectComments(tokenIndex), true);
}
final int position = t.originalStart - 1;
final char c = tokenManager.getSource().charAt(position);
int line = compilationUnit.getLineNumber(position);
int column = compilationUnit.getColumnNumber(position);
if (c == '\n' || c == '\r') {
line++;
column = 0;
} else {
column++;
}
return new InternalSyntaxToken(line, column, "", collectComments(tokenIndex), true);
isEOF = true;
value = "";
} else {
isEOF = false;
value = t.toString(tokenManager.getSource());
}
return new InternalSyntaxToken(
compilationUnit.getLineNumber(t.originalStart),
compilationUnit.getColumnNumber(t.originalStart),
t.toString(tokenManager.getSource()),
collectComments(tokenIndex),
false
);
LineColumnConverter.Pos pos = lineColumnConverter.toPos(t.originalStart);
return new InternalSyntaxToken(pos.line(), pos.columnOffset(), value, collectComments(tokenIndex), isEOF);
}

private InternalSyntaxToken createSpecialToken(int tokenIndex) {
Token t = tokenManager.get(tokenIndex);
List<SyntaxTrivia> comments = t.tokenType == TerminalTokens.TokenNameGREATER
? collectComments(tokenIndex)
: Collections.emptyList();
return new InternalSyntaxToken(
compilationUnit.getLineNumber(t.originalEnd),
compilationUnit.getColumnNumber(t.originalEnd),
">",
comments,
false
);
LineColumnConverter.Pos pos = lineColumnConverter.toPos(t.originalEnd);
return new InternalSyntaxToken(pos.line(), pos.columnOffset(), ">", comments, false);
}

private List<SyntaxTrivia> collectComments(int tokenIndex) {
Expand All @@ -491,10 +476,11 @@ private List<SyntaxTrivia> collectComments(int tokenIndex) {
List<SyntaxTrivia> comments = new ArrayList<>();
for (int i = commentIndex; i < tokenIndex; i++) {
Token t = tokenManager.get(i);
LineColumnConverter.Pos pos = lineColumnConverter.toPos(t.originalStart);
comments.add(new InternalSyntaxTrivia(
t.toString(tokenManager.getSource()),
compilationUnit.getLineNumber(t.originalStart),
compilationUnit.getColumnNumber(t.originalStart)
pos.line(),
pos.columnOffset()
));
}
return comments;
Expand Down Expand Up @@ -2570,15 +2556,15 @@ private TypeTree convertArrayType(ArrayType e) {
}

private JavaTree.ParameterizedTypeTreeImpl convertParameterizedType(ParameterizedType e) {
int pos = e.getStartPosition() + e.getLength() - 1;
LineColumnConverter.Pos pos = lineColumnConverter.toPos(e.getStartPosition() + e.getLength() - 1);
JavaTree.ParameterizedTypeTreeImpl t = new JavaTree.ParameterizedTypeTreeImpl(
convertType(e.getType()),
convertTypeArguments(
firstTokenAfter(e.getType(), TerminalTokens.TokenNameLESS),
e.typeArguments(),
new InternalSyntaxToken(
compilationUnit.getLineNumber(pos),
compilationUnit.getColumnNumber(pos),
pos.line(),
pos.columnOffset(),
">",
/* TODO */ Collections.emptyList(),
false
Expand Down
18 changes: 9 additions & 9 deletions java-frontend/src/main/java/org/sonar/java/model/JWarning.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,30 +100,30 @@ public static class Mapper extends SubscriptionVisitor {

private final PriorityQueue<JWarning> warnings = new PriorityQueue<>(Comparator.comparing(JWarning::start).thenComparing(JWarning::end));

private Mapper(CompilationUnit ast) {
private Mapper(CompilationUnit ast, LineColumnConverter lineColumnConverter) {
Stream.of(ast.getProblems())
.map(problem -> convert(problem, ast))
.map(problem -> convert(problem, ast, lineColumnConverter))
.filter(Objects::nonNull)
.forEach(warnings::add);
}

@CheckForNull
private static JWarning convert(IProblem problem, CompilationUnit root) {
private static JWarning convert(IProblem problem, CompilationUnit root, LineColumnConverter lineColumnConverter) {
for (Type type : Type.values()) {
if (type.matches(problem)) {
LineColumnConverter.Pos start = lineColumnConverter.toPos(problem.getSourceStart());
LineColumnConverter.Pos end = lineColumnConverter.toPos(problem.getSourceStart());
return new JWarning(problem.getMessage(),
type,
problem.getSourceLineNumber(),
root.getColumnNumber(problem.getSourceStart()),
root.getLineNumber(problem.getSourceEnd()),
root.getColumnNumber(problem.getSourceEnd()) + 1);
start.line(), start.columnOffset(),
end.line(), end.columnOffset());
}
}
return null;
}

public static Mapper warningsFor(CompilationUnit ast) {
return new Mapper(ast);
public static Mapper warningsFor(CompilationUnit ast, LineColumnConverter lineColumnConverter) {
return new Mapper(ast, lineColumnConverter);
}

public void mappedInto(JavaTree.CompilationUnitTreeImpl cut) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* SonarQube Java
* Copyright (C) 2012-2024 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.java.model;

import java.util.Arrays;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* There is a different convention in the JDT for line and column numbers.
* The difference is only when there are some line continuation characters in text blocks.
* Eclipse does not count the line continuation as a new line.
* So we cannot use the following methods to get the line and column number:
* {@link org.eclipse.jdt.core.dom.CompilationUnit#getColumnNumber(int position)}
* {@link org.eclipse.jdt.core.dom.CompilationUnit#getLineNumber(int position)}
*/
public class LineColumnConverter {

private static final Pattern LINE_SEPARATOR_PATTERN = Pattern.compile("\r\n?|\n");

private int[] lineStartIndexes = new int[64];
private int lineStartIndexesLength = 0;

public LineColumnConverter(String source) {
Matcher matcher = LINE_SEPARATOR_PATTERN.matcher(source);
addLineStartIndex(0);
while (matcher.find()) {
addLineStartIndex(matcher.end());
}
addLineStartIndex(Integer.MAX_VALUE);
}

private void addLineStartIndex(int index) {
if (lineStartIndexesLength == lineStartIndexes.length) {
lineStartIndexes = Arrays.copyOf(lineStartIndexes, lineStartIndexes.length * 2);
}
lineStartIndexes[lineStartIndexesLength] = index;
lineStartIndexesLength++;
}

public Pos toPos(int absolutSourcePosition) {
int searchResult = Arrays.binarySearch(lineStartIndexes, 0, lineStartIndexesLength, absolutSourcePosition);
if (searchResult < 0) {
return new Pos(-searchResult - 1, absolutSourcePosition - lineStartIndexes[-searchResult - 2]);
} else {
return new Pos(searchResult + 1, 0);
}
}

public record Pos(int line, int columnOffset) {
}

}
6 changes: 6 additions & 0 deletions java-frontend/src/test/files/metrics/TextBlock.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class TextBlock {
String a = """
Hello,
world!
""";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class TextBlock {
String a = """
Hello,\
world!
""";
String b = """
Goodbye,\
cruel \
world!
""";
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ void parsing_errors_should_be_reported_to_sonarlint() throws Exception {
scan(SONARLINT_RUNTIME, "class A {");

assertThat(sensorContext.allAnalysisErrors()).hasSize(1);
assertThat(sensorContext.allAnalysisErrors().iterator().next().message()).startsWith("Parse error at line 1 column 8");
assertThat(sensorContext.allAnalysisErrors().iterator().next().message()).startsWith("Parse error at line 1 column 9");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ void test_should_log_fail_parsing_with_incorrect_version() {
scanWithJavaVersion(8, Collections.singletonList(TestUtils.inputFile("src/test/files/metrics/Java15SwitchExpression.java")));
assertThat(logTester.logs(Level.ERROR)).containsExactly(
"Unable to parse source file : 'src/test/files/metrics/Java15SwitchExpression.java'",
"Parse error at line 3 column 12: Switch Expressions are supported from Java 14 onwards only"
"Parse error at line 3 column 13: Switch Expressions are supported from Java 14 onwards only"
);
}

Expand Down Expand Up @@ -282,7 +282,7 @@ void module_info_should_not_be_analyzed_or_change_the_version() {
.contains("1/1 source file has been analyzed");
assertThat(logTester.logs(Level.ERROR)).containsExactly(
"Unable to parse source file : 'src/test/files/metrics/Java15SwitchExpression.java'",
"Parse error at line 3 column 12: Switch Expressions are supported from Java 14 onwards only"
"Parse error at line 3 column 13: Switch Expressions are supported from Java 14 onwards only"
);
assertThat(logTester.logs(Level.WARN))
// two files, only one log
Expand All @@ -305,7 +305,7 @@ void remove_info_ro_warning_log_related_to_module_info() {
assertThat(logTester.logs(Level.WARN)).isEmpty();
assertThat(logTester.logs(Level.ERROR)).containsExactly(
"Unable to parse source file : 'src/test/files/metrics/Java15SwitchExpression.java'",
"Parse error at line 3 column 12: Switch Expressions are supported from Java 14 onwards only"
"Parse error at line 3 column 13: Switch Expressions are supported from Java 14 onwards only"
);
}

Expand Down
55 changes: 55 additions & 0 deletions java-frontend/src/test/java/org/sonar/java/model/JParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,61 @@ void failing_batch_mode_should_stop_file_by_file_when_cancelled() throws Excepti
// Missing: [action] analyse class MyClass in Methods.java
}

@Test
void test_no_line_continuation_in_text_blocks() {
CompilationUnitTree tree = parse(new File("src/test/files/metrics/TextBlock.java"));
var tokens = tokens((JavaTree) tree);
assertThat(tokens)
.extracting(token -> token.line() + "," + token.column() + ": " + token.text())
.containsExactly(
"1,0: class",
"1,6: TextBlock",
"1,16: {",
"2,2: String",
"2,9: a",
"2,11: =",
"2,13: \"\"\"\n Hello,\n world!\n \"\"\"",
"5,7: ;",
"6,0: }",
"6,1: ");
}

@Test
void test_line_continuation_in_text_blocks() {
CompilationUnitTree tree = parse(new File("src/test/files/metrics/TextBlockWithLineContinuation.java"));
var tokens = tokens((JavaTree) tree);
assertThat(tokens)
.extracting(token -> token.line() + "," + token.column() + ": " + token.text())
.containsExactly(
"1,0: class",
"1,6: TextBlock",
"1,16: {",
"2,2: String",
"2,9: a",
"2,11: =",
"2,13: \"\"\"\n Hello,\\\n world!\n \"\"\"",
"5,7: ;",
"6,2: String",
"6,9: b",
"6,11: =",
"6,13: \"\"\"\n Goodbye,\\\n cruel \\\n world!\n \"\"\"",
"10,7: ;",
"11,0: }",
"11,1: ");
}

public static List<InternalSyntaxToken> tokens(JavaTree tree) {
var tokens = new ArrayList<InternalSyntaxToken>();
if (tree instanceof InternalSyntaxToken) {
tokens.add((InternalSyntaxToken) tree);
} else {
for (var child : tree.getChildren()) {
tokens.addAll(tokens((JavaTree) child));
}
}
return tokens;
}

@Test
void failing_batch_mode_with_empty_file_list_should_not_continue_file_by_file() throws Exception {
List<InputFile> inputFiles = List.of();
Expand Down
Loading

0 comments on commit 03e34a9

Please sign in to comment.