Skip to content

Commit 2163648

Browse files
committed
fix location off-by-ones with regexp parsing
1 parent 80919e3 commit 2163648

File tree

6 files changed

+1283
-469
lines changed

6 files changed

+1283
-469
lines changed

javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ public Label visit(Literal nd, Context c) {
589589

590590
trapwriter.addTuple("literals", valueString, source, key);
591591
Position start = nd.getLoc().getStart();
592-
com.semmle.util.locations.Position startPos = new com.semmle.util.locations.Position(start.getLine(), start.getColumn(), start.getOffset());
592+
com.semmle.util.locations.Position startPos = new com.semmle.util.locations.Position(start.getLine(), start.getColumn() + 1 /* Convert from 0-based to 1-based. */, start.getOffset());
593593

594594
if (nd.isRegExp()) {
595595
OffsetTranslation offsets = new OffsetTranslation();
@@ -867,7 +867,7 @@ private void extractRegxpFromBinop(BinaryExpression nd, Context c) {
867867
}
868868
OffsetTranslation offsets = concatResult.snd();
869869
Position start = nd.getLoc().getStart();
870-
com.semmle.util.locations.Position startPos = new com.semmle.util.locations.Position(start.getLine(), start.getColumn(), start.getOffset());
870+
com.semmle.util.locations.Position startPos = new com.semmle.util.locations.Position(start.getLine(), start.getColumn() + 1 /* Convert from 0-based to 1-based. */, start.getOffset());
871871
SourceMap sourceMap = SourceMap.legacyWithStartPos(SourceMap.fromString(nd.getLoc().getSource()).offsetBy(0, offsets), startPos);
872872
regexpExtractor.extract(foldedString, sourceMap, nd, true);
873873
return;

javascript/extractor/src/com/semmle/js/extractor/RegExpExtractor.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,12 @@ private Label extractTerm(RegExpTerm term, Label parent, int idx) {
121121
}
122122

123123
public void emitLocation(SourceElement term, Label lbl) {
124-
int sl = sourceMap.getStart(term.getLoc().getStart().getColumn()).getLine();
125-
int sc = sourceMap.getStart(term.getLoc().getStart().getColumn()).getColumn() + 1; // convert to 1-based
126-
int el = sourceMap.getEnd(term.getLoc().getEnd().getColumn()).getLine();
127-
int ec = sourceMap.getEnd(term.getLoc().getEnd().getColumn()).getColumn() - 1; // convert to inclusive
124+
int start = term.getLoc().getStart().getColumn();
125+
int sl = sourceMap.getStart(start).getLine();
126+
int sc = sourceMap.getStart(start).getColumn();
127+
int end = term.getLoc().getEnd().getColumn();
128+
int el = sourceMap.getStart(end).getLine();
129+
int ec = sourceMap.getStart(end).getColumn() - 1; // convert to inclusive
128130
locationManager.emitSnippetLocation(lbl, sl, sc, el, ec);
129131
}
130132

javascript/extractor/tests/regexp/input/multipart.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,21 @@ var reg4 = new RegExp(
1111
"bar" +
1212
"baz" +
1313
"qux"
14-
);
14+
);
15+
16+
var bad95 = new RegExp(
17+
"(a" +
18+
"|" +
19+
"aa)*" +
20+
"b$"
21+
);
22+
23+
var bad96 = new RegExp("(" +
24+
"(c|cc)*|" +
25+
"(d|dd)*|" +
26+
"(e|ee)*" +
27+
")f$");
28+
29+
var bad97 = new RegExp(
30+
"(g|gg" +
31+
")*h$");

0 commit comments

Comments
 (0)