Skip to content

Commit 021d4ef

Browse files
ghislainpiotghislainpiot
committed
SONARPY-2063 Multiline strings should have a correct end position (#1923)
1 parent 89ec6c0 commit 021d4ef

File tree

8 files changed

+53
-27
lines changed

8 files changed

+53
-27
lines changed

‎python-frontend/src/main/java/org/sonar/plugins/python/api/tree/Token.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,6 @@ public interface Token extends Tree {
4242

4343
int valueLength();
4444

45+
boolean isCompressed();
46+
4547
}

‎python-frontend/src/main/java/org/sonar/python/IPythonLocation.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,8 @@
2121

2222
import java.util.Map;
2323

24-
public record IPythonLocation(int line, int column, Map<Integer, Integer> colOffset) {
24+
public record IPythonLocation(int line, int column, Map<Integer, Integer> colOffset, boolean isCompresssed) {
25+
public IPythonLocation(int line, int column, Map<Integer, Integer> colOffset) {
26+
this(line, column, colOffset, false);
27+
}
2528
}

‎python-frontend/src/main/java/org/sonar/python/TokenLocation.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,13 @@ public TokenLocation(Token token) {
3636
String[] lines = value.split("\r\n|\n|\r", -1);
3737

3838
if (lines.length > 1) {
39-
endLine = token.line() + lines.length - 1;
40-
endLineOffset = lines[lines.length - 1].length();
41-
39+
if (token.isCompressed()) {
40+
endLine = token.line();
41+
endLineOffset = this.startLineOffset + token.valueLength();
42+
} else {
43+
endLine = token.line() + lines.length - 1;
44+
endLineOffset = lines[lines.length - 1].length();
45+
}
4246
} else {
4347
this.endLine = this.startLine;
4448
this.endLineOffset = this.startLineOffset + token.valueLength();

‎python-frontend/src/main/java/org/sonar/python/tree/TokenEnricher.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public static TokenImpl enrichToken(Token token, Map<Integer, IPythonLocation> o
5050
.map(t -> computeTriviaLocation(t, location.line(), startCol, token.getLine(), offsetMap))
5151
.toList();
5252

53-
return new TokenImpl(token, location.line(), startCol, escapedCharInToken, trivia);
53+
return new TokenImpl(token, location.line(), startCol, escapedCharInToken, trivia, location.isCompresssed());
5454
}
5555
return new TokenImpl(token);
5656
}
@@ -59,14 +59,16 @@ private static Trivia computeTriviaLocation(com.sonar.sslr.api.Trivia trivia, in
5959
int escapedCharInToken = computeEscapeCharsInToken(trivia.getToken().getValue());
6060
var line = parentLine;
6161
var col = parentCol - escapedCharInToken - trivia.getToken().getValue().length();
62+
var isCompressed = false;
6263
if (parentPythonLine != trivia.getToken().getLine()) {
6364
IPythonLocation location = offsetMap.get(trivia.getToken().getLine());
6465
line = location.line();
6566
Map<Integer, Integer> escapeCharsMap = location.colOffset();
6667
col = computeColWithEscapes(trivia.getToken().getColumn(), escapeCharsMap, location.column());
68+
isCompressed = location.isCompresssed();
6769
}
6870
return new TriviaImpl(new TokenImpl(trivia.getToken(), line, col,
69-
escapedCharInToken, List.of()));
71+
escapedCharInToken, List.of(), isCompressed));
7072
}
7173

7274
private static int computeEscapeCharsInToken(String tokenValue) {

‎python-frontend/src/main/java/org/sonar/python/tree/TokenImpl.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,20 @@ public class TokenImpl extends PyTree implements Token {
3636
private Integer line;
3737
private Integer column;
3838
private int includedEscapeChars;
39+
private boolean isCompressed = false;
3940

4041
public TokenImpl(com.sonar.sslr.api.Token token) {
4142
this.token = token;
4243
this.trivia = token.getTrivia().stream().map(tr -> new TriviaImpl(new TokenImpl(tr.getToken()))).collect(Collectors.toList());
4344
}
4445

45-
public TokenImpl(com.sonar.sslr.api.Token token, int line, int column, int includedEscapeChars, List<Trivia> trivia) {
46+
public TokenImpl(com.sonar.sslr.api.Token token, int line, int column, int includedEscapeChars, List<Trivia> trivia, boolean isCompressed) {
4647
this.token = token;
4748
this.line = line;
4849
this.column = column;
4950
this.includedEscapeChars = includedEscapeChars;
5051
this.trivia = trivia;
52+
this.isCompressed = isCompressed;
5153
}
5254

5355
@Override
@@ -118,4 +120,10 @@ public Token lastToken() {
118120
public int valueLength() {
119121
return value().length() + includedEscapeChars();
120122
}
123+
124+
@Override
125+
public boolean isCompressed() {
126+
return this.isCompressed;
127+
}
128+
121129
}

‎python-frontend/src/test/java/org/sonar/python/TokenLocationTest.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,23 +68,29 @@ void test_comment() {
6868

6969
@Test
7070
void test_escaped_chars_ipython_lexer() {
71-
var token = new TokenImpl(iPythonLex("\"1\\n3\"").get(0), 3, 10, 3, List.of());
71+
var token = new TokenImpl(iPythonLex("\"1\\n3\"").get(0), 3, 10, 3, List.of(), false);
7272
TokenLocation tokenLocation = new TokenLocation(token);
7373
assertOffsets(tokenLocation, 3, 10, 3, 19);
7474

75-
token = new TokenImpl(iPythonLex("foo").get(0), 10, 20, 0, List.of());
75+
token = new TokenImpl(iPythonLex("foo").get(0), 10, 20, 0, List.of(), false);
7676
tokenLocation = new TokenLocation(token);
7777
assertOffsets(tokenLocation, 10, 20, 10, 23);
7878
}
7979

8080
@Test
8181
void test_multiline_ipython_lexer() {
8282
var tokens = iPythonLex("'''first line\nsecond\\t'''");
83-
var token = new TokenImpl(tokens.get(0), 3, 10, 1, List.of());
83+
var token = new TokenImpl(tokens.get(0), 3, 10, 1, List.of(), false);
8484
TokenLocation tokenLocation = new TokenLocation(token);
8585
assertOffsets(tokenLocation, 3, 10, 4, 11);
8686
}
87-
87+
@Test
88+
void test_multiline_ipython_lexer_compressed() {
89+
var tokens = iPythonLex("'''first line\nsecond\\t'''");
90+
var token = new TokenImpl(tokens.get(0), 3, 10, 1, List.of(), true);
91+
TokenLocation tokenLocation = new TokenLocation(token);
92+
assertOffsets(tokenLocation, 3, 10, 3, 36);
93+
}
8894
private static void assertOffsets(TokenLocation tokenLocation, int startLine, int startLineOffset, int endLine, int endLineOffset) {
8995
assertThat(tokenLocation.startLine()).as("start line").isEqualTo(startLine);
9096
assertThat(tokenLocation.startLineOffset()).as("start line offset").isEqualTo(startLineOffset);

‎sonar-python-plugin/src/main/java/org/sonar/plugins/python/IpynbNotebookParser.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,11 @@ private static NotebookParsingData parseSourceArray(int startLine, JsonParser jP
179179
var lastSourceLine = "\n";
180180
while (jParser.nextToken() != JsonToken.END_ARRAY) {
181181
String sourceLine = jParser.getValueAsString();
182-
tokenLocation = jParser.currentTokenLocation();
183-
var countEscapedChar = countEscapeCharacters(sourceLine, tokenLocation.getColumnNr());
184-
cellData.addLineToSource(sourceLine, tokenLocation.getLineNr(), tokenLocation.getColumnNr(), countEscapedChar);
182+
var newTokenLocation = jParser.currentTokenLocation();
183+
var countEscapedChar = countEscapeCharacters(sourceLine, newTokenLocation.getColumnNr());
184+
cellData.addLineToSource(sourceLine, newTokenLocation.getLineNr(), newTokenLocation.getColumnNr(), countEscapedChar);
185185
lastSourceLine = sourceLine;
186+
tokenLocation = newTokenLocation;
186187
}
187188
if (!lastSourceLine.endsWith("\n")) {
188189
cellData.appendToSource("\n");
@@ -204,7 +205,7 @@ private static NotebookParsingData parseSourceMultilineString(int startLine, Jso
204205
var countEscapedChar = countEscapeCharacters(line, previousLen + previousExtraChars + tokenLocation.getColumnNr());
205206
var currentCount = countEscapedChar.get(-1);
206207
cellData.addLineToSource(line, new IPythonLocation(tokenLocation.getLineNr(),
207-
tokenLocation.getColumnNr() + previousLen + previousExtraChars, countEscapedChar));
208+
tokenLocation.getColumnNr() + previousLen + previousExtraChars, countEscapedChar, true));
208209
cellData.appendToSource("\n");
209210
previousLen = previousLen + line.length() + 2;
210211
previousExtraChars = previousExtraChars + currentCount;
@@ -230,7 +231,7 @@ private static Map<Integer, Integer> countEscapeCharacters(String sourceLine, in
230231
numberOfExtraChars++;
231232
colMap.put(i, i + colOffSet + count + numberOfExtraChars);
232233
break;
233-
// we never encounter \n or \r as the lines are split at these characters
234+
// we never encounter \n or \r as the lines are split at these characters
234235
case '\b', '\f', '\t':
235236
// we increase the count of one char as we count the \ but not the t or b
236237
count += 1;

‎sonar-python-plugin/src/test/java/org/sonar/plugins/python/IpynbNotebookParserTest.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ void testParseNotebook() throws IOException {
5252
//" print \"not none\"\n"
5353
assertThat(result.locationMap()).extracting(map -> map.get(3)).isEqualTo(new IPythonLocation(19, 5, Map.of(10, 16, 19, 26, -1, 2)));
5454
//"source": "#Some code\nprint(\"hello world\\n\")",
55-
assertThat(result.locationMap()).extracting(map -> map.get(16)).isEqualTo(new IPythonLocation(64, 14, Map.of(-1, 0)));
56-
assertThat(result.locationMap()).extracting(map -> map.get(17)).isEqualTo(new IPythonLocation(64, 27, Map.of(6, 34, 18, 47, 20, 50, -1, 3)));
55+
assertThat(result.locationMap()).extracting(map -> map.get(16)).isEqualTo(new IPythonLocation(64, 14, Map.of(-1, 0), true));
56+
assertThat(result.locationMap()).extracting(map -> map.get(17)).isEqualTo(new IPythonLocation(64, 27, Map.of(6, 34, 18, 47, 20, 50, -1, 3), true));
5757
//"source": "print(\"My\\ntext\")\nprint(\"Something else\\n\")"
58-
assertThat(result.locationMap()).extracting(map -> map.get(22)).isEqualTo(new IPythonLocation(83, 14, Map.of(6, 21, 9, 25, 15, 32, -1, 3)));
59-
assertThat(result.locationMap()).extracting(map -> map.get(23)).isEqualTo(new IPythonLocation(83, 37, Map.of(6, 44, 21, 60, 23, 63, -1, 3)));
58+
assertThat(result.locationMap()).extracting(map -> map.get(22)).isEqualTo(new IPythonLocation(83, 14, Map.of(6, 21, 9, 25, 15, 32, -1, 3), true));
59+
assertThat(result.locationMap()).extracting(map -> map.get(23)).isEqualTo(new IPythonLocation(83, 37, Map.of(6, 44, 21, 60, 23, 63, -1, 3), true));
6060

6161
//"source": "a = \"A bunch of characters \\n \\f \\r \\ \"\nb = None"
6262
assertThat(result.locationMap()).extracting(map -> map.get(25))
63-
.isEqualTo(new IPythonLocation(90, 14, Map.of(4,19, 27, 43, 30, 47, 33, 51, 36, 55, 39, 59, -1, 6)));
64-
assertThat(result.locationMap()).extracting(map -> map.get(26)).isEqualTo(new IPythonLocation(90, 63, Map.of(-1, 0)));
63+
.isEqualTo(new IPythonLocation(90, 14, Map.of(4,19, 27, 43, 30, 47, 33, 51, 36, 55, 39, 59, -1, 6), true));
64+
assertThat(result.locationMap()).extracting(map -> map.get(26)).isEqualTo(new IPythonLocation(90, 63, Map.of(-1, 0), true));
6565
// last line with the cell delimiter which contains the EOF token
6666
assertThat(result.locationMap()).extracting(map -> map.get(27)).isEqualTo(new IPythonLocation(90, 14, Map.of(-1, 0)));
6767
}
@@ -141,12 +141,12 @@ void testParseNotebookSingleLine() throws IOException {
141141
assertThat(result.locationMap().get(4).column()).isEqualTo(452);
142142

143143
// First and second line
144-
assertThat(result.locationMap()).containsEntry(1, new IPythonLocation(1, 382, Map.of(-1, 0)));
145-
assertThat(result.locationMap()).containsEntry(2, new IPythonLocation(1, 429, Map.of(-1, 0)));
144+
assertThat(result.locationMap()).containsEntry(1, new IPythonLocation(1, 382, Map.of(-1, 0), true));
145+
assertThat(result.locationMap()).containsEntry(2, new IPythonLocation(1, 429, Map.of(-1, 0), true));
146146

147-
assertThat(result.locationMap()).containsEntry(6, new IPythonLocation(1, 559, Map.of(-1, 3, 0, 560, 1, 562, 2, 564)));
148-
assertThat(result.locationMap()).containsEntry(7, new IPythonLocation(1, 610, Map.of(-1, 0)));
149-
assertThat(result.locationMap()).containsEntry(8, new IPythonLocation(1, 637, Map.of(-1, 3, 1, 640, 2, 642, 0, 638)));
147+
assertThat(result.locationMap()).containsEntry(6, new IPythonLocation(1, 559, Map.of(-1, 3, 0, 560, 1, 562, 2, 564), true));
148+
assertThat(result.locationMap()).containsEntry(7, new IPythonLocation(1, 610, Map.of(-1, 0), true));
149+
assertThat(result.locationMap()).containsEntry(8, new IPythonLocation(1, 637, Map.of(-1, 3, 1, 640, 2, 642, 0, 638), true));
150150
}
151151

152152
@Test

0 commit comments

Comments
 (0)