Skip to content

Commit

Permalink
Improve Recovery of Unterminated Regular Expressions (#58289)
Browse files Browse the repository at this point in the history
Co-authored-by: Ron Buckton <ron.buckton@microsoft.com>
  • Loading branch information
graphemecluster and rbuckton authored May 24, 2024
1 parent 842cf17 commit d0ef028
Show file tree
Hide file tree
Showing 20 changed files with 223 additions and 125 deletions.
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31974,7 +31974,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
scanner.setScriptTarget(sourceFile.languageVersion);
scanner.setLanguageVariant(sourceFile.languageVariant);
scanner.setOnError((message, length, arg0) => {
// emulate `parseErrorAtPosition` from parser.ts
// For providing spelling suggestions
const start = scanner!.getTokenEnd();
if (message.category === DiagnosticCategory.Message && lastError && start === lastError.start && length === lastError.length) {
const error = createDetachedDiagnostic(sourceFile.fileName, sourceFile.text, start, length, message, arg0);
Expand Down
7 changes: 1 addition & 6 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ import {
DeleteExpression,
Diagnostic,
DiagnosticArguments,
DiagnosticCategory,
DiagnosticMessage,
Diagnostics,
DiagnosticWithDetachedLocation,
Expand Down Expand Up @@ -2144,11 +2143,7 @@ namespace Parser {
// Don't report another error if it would just be at the same position as the last error.
const lastError = lastOrUndefined(parseDiagnostics);
let result: DiagnosticWithDetachedLocation | undefined;
if (message.category === DiagnosticCategory.Message && lastError && start === lastError.start && length === lastError.length) {
result = createDetachedDiagnostic(fileName, sourceText, start, length, message, ...args);
addRelatedInfo(lastError, result);
}
else if (!lastError || start !== lastError.start) {
if (!lastError || start !== lastError.start) {
result = createDetachedDiagnostic(fileName, sourceText, start, length, message, ...args);
parseDiagnostics.push(result);
}
Expand Down
172 changes: 101 additions & 71 deletions src/compiler/scanner.ts

Large diffs are not rendered by default.

22 changes: 11 additions & 11 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2765,17 +2765,17 @@ export interface RegularExpressionLiteral extends LiteralExpression {
// dprint-ignore
/** @internal */
export const enum RegularExpressionFlags {
None = 0,
HasIndices = 1 << 0, // d
Global = 1 << 1, // g
IgnoreCase = 1 << 2, // i
Multiline = 1 << 3, // m
DotAll = 1 << 4, // s
Unicode = 1 << 5, // u
UnicodeSets = 1 << 6, // v
Sticky = 1 << 7, // y
UnicodeMode = Unicode | UnicodeSets,
Modifiers = IgnoreCase | Multiline | DotAll,
None = 0,
HasIndices = 1 << 0, // d
Global = 1 << 1, // g
IgnoreCase = 1 << 2, // i
Multiline = 1 << 3, // m
DotAll = 1 << 4, // s
Unicode = 1 << 5, // u
UnicodeSets = 1 << 6, // v
Sticky = 1 << 7, // y
AnyUnicodeMode = Unicode | UnicodeSets,
Modifiers = IgnoreCase | Multiline | DotAll,
}

export interface NoSubstitutionTemplateLiteral extends LiteralExpression, TemplateLiteralLikeNode, Declaration {
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export * from "./unittests/paths.js";
export * from "./unittests/printer.js";
export * from "./unittests/programApi.js";
export * from "./unittests/publicApi.js";
export * from "./unittests/regExpScannerRecovery.js";
export * from "./unittests/reuseProgramStructure.js";
export * from "./unittests/semver.js";
export * from "./unittests/services/cancellableLanguageServiceOperations.js";
Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/incrementalParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe("unittests:: incrementalParser::", () => {
const oldText = ts.ScriptSnapshot.fromString(source);
const newTextAndChange = withInsert(oldText, semicolonIndex, "/");

compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 0);
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4);
});

it("Regular expression 2", () => {
Expand Down
81 changes: 81 additions & 0 deletions src/testRunner/unittests/regExpScannerRecovery.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import * as ts from "../_namespaces/ts.js";

describe("unittests:: regExpScannerRecovery", () => {
const testCases = [
"/",
"/[]",
"/{}",
"/()",
"/foo",
"/foo[]",
"/foo{}",
"/foo()",
"/[]foo",
"/{}foo",
"/()foo",
"/{[]}",
"/([])",
"/[)}({]",
"/({[]})",
"/\\[",
"/\\{",
"/\\(",
"/[\\[]",
"/(\\[)",
"/{\\[}",
"/[\\(]",
"/(\\()",
"/{\\(}",
"/[\\{]",
"/(\\{)",
"/{\\{}",
"/\\{(\\[\\([{])",
"/\\]",
"/\\}",
"/\\)",
"/[\\]]",
"/(\\])",
"/{\\]}",
"/[\\)]",
"/(\\))",
"/{\\)}",
"/[\\}]",
"/(\\})",
"/{\\}}",
"/({[\\])]})",
];
const whiteSpaceSequences = [
"",
" ",
"\t\f",
"\u3000\u2003",
];
for (const testCase of testCases) {
for (const whiteSpaces of whiteSpaceSequences) {
const testCaseWithWhiteSpaces = testCase + whiteSpaces;
const sources = [
`const regex = ${testCaseWithWhiteSpaces};`,
`(${testCaseWithWhiteSpaces});`,
`([${testCaseWithWhiteSpaces}]);`,
`({prop: ${testCaseWithWhiteSpaces}});`,
`({prop: ([(${testCaseWithWhiteSpaces})])});`,
`({[(${testCaseWithWhiteSpaces}).source]: 42});`,
];
for (const source of sources) {
it("stops parsing unterminated regexes at correct position: " + JSON.stringify(source), () => {
const { parseDiagnostics } = ts.createLanguageServiceSourceFile(
/*fileName*/ "",
ts.ScriptSnapshot.fromString(source),
ts.ScriptTarget.Latest,
/*version*/ "0",
/*setNodeParents*/ false,
);
const diagnostic = ts.find(parseDiagnostics, ({ code }) => code === ts.Diagnostics.Unterminated_regular_expression_literal.code);
assert(diagnostic, "There should be an 'Unterminated regular expression literal.' error");
assert.equal(diagnostic.start, source.indexOf("/"), "Diagnostic should start at where the regex starts");
assert.equal(diagnostic.length, testCase.length, "Diagnostic should end at where the regex ends");
});
}
}
}
});
7 changes: 2 additions & 5 deletions tests/baselines/reference/parser645086_1.errors.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
parser645086_1.ts(1,13): error TS1005: ',' expected.
parser645086_1.ts(1,14): error TS1134: Variable declaration expected.
parser645086_1.ts(1,15): error TS1161: Unterminated regular expression literal.


==== parser645086_1.ts (3 errors) ====
==== parser645086_1.ts (2 errors) ====
var v = /[]/]/
~
!!! error TS1005: ',' expected.
~
!!! error TS1134: Variable declaration expected.

!!! error TS1161: Unterminated regular expression literal.
!!! error TS1134: Variable declaration expected.
7 changes: 2 additions & 5 deletions tests/baselines/reference/parser645086_2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
parser645086_2.ts(1,14): error TS1005: ',' expected.
parser645086_2.ts(1,15): error TS1134: Variable declaration expected.
parser645086_2.ts(1,16): error TS1161: Unterminated regular expression literal.


==== parser645086_2.ts (3 errors) ====
==== parser645086_2.ts (2 errors) ====
var v = /[^]/]/
~
!!! error TS1005: ',' expected.
~
!!! error TS1134: Variable declaration expected.

!!! error TS1161: Unterminated regular expression literal.
!!! error TS1134: Variable declaration expected.
4 changes: 2 additions & 2 deletions tests/baselines/reference/parserMissingToken2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
parserMissingToken2.ts(1,2): error TS1161: Unterminated regular expression literal.
parserMissingToken2.ts(1,1): error TS1161: Unterminated regular expression literal.


==== parserMissingToken2.ts (1 errors) ====
/ b;
~~~
!!! error TS1161: Unterminated regular expression literal.
2 changes: 1 addition & 1 deletion tests/baselines/reference/parserMissingToken2.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
/ b;
//// [parserMissingToken2.js]
/ b;;
/ b;
4 changes: 2 additions & 2 deletions tests/baselines/reference/parserMissingToken2.types
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

=== parserMissingToken2.ts ===
/ b;
>/ b; : RegExp
> : ^^^^^^
>/ b : RegExp
> : ^^^^^^

Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
parserRegularExpressionDivideAmbiguity4.ts(1,1): error TS2304: Cannot find name 'foo'.
parserRegularExpressionDivideAmbiguity4.ts(1,6): error TS1161: Unterminated regular expression literal.
parserRegularExpressionDivideAmbiguity4.ts(1,17): error TS1005: ')' expected.
parserRegularExpressionDivideAmbiguity4.ts(1,5): error TS1161: Unterminated regular expression literal.


==== parserRegularExpressionDivideAmbiguity4.ts (3 errors) ====
==== parserRegularExpressionDivideAmbiguity4.ts (2 errors) ====
foo(/notregexp);
~~~
!!! error TS2304: Cannot find name 'foo'.

!!! error TS1161: Unterminated regular expression literal.

!!! error TS1005: ')' expected.
~~~~~~~~~~
!!! error TS1161: Unterminated regular expression literal.
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
foo(/notregexp);
//// [parserRegularExpressionDivideAmbiguity4.js]
foo(/notregexp););
foo(/notregexp);
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

=== parserRegularExpressionDivideAmbiguity4.ts ===
foo(/notregexp);
>foo(/notregexp); : any
> : ^^^
>foo(/notregexp) : any
> : ^^^
>foo : any
> : ^^^
>/notregexp); : RegExp
> : ^^^^^^
>/notregexp : RegExp
> : ^^^^^^

4 changes: 2 additions & 2 deletions tests/baselines/reference/tsxAttributeInvalidNames.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ file.tsx(11,1): error TS2362: The left-hand side of an arithmetic operation must
file.tsx(11,8): error TS1003: Identifier expected.
file.tsx(11,9): error TS2304: Cannot find name 'data'.
file.tsx(11,13): error TS1005: ';' expected.
file.tsx(11,20): error TS1161: Unterminated regular expression literal.
file.tsx(11,19): error TS1161: Unterminated regular expression literal.


==== file.tsx (13 errors) ====
Expand Down Expand Up @@ -49,5 +49,5 @@ file.tsx(11,20): error TS1161: Unterminated regular expression literal.
!!! error TS2304: Cannot find name 'data'.
~
!!! error TS1005: ';' expected.
~~
!!! error TS1161: Unterminated regular expression literal.
2 changes: 1 addition & 1 deletion tests/baselines/reference/tsxAttributeInvalidNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ data = { 32: } / > ;
{
32;
}
/>;;
/>;
4 changes: 2 additions & 2 deletions tests/baselines/reference/tsxAttributeInvalidNames.types
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ declare module JSX {
> : ^^^
>32 : 32
> : ^^
>/>; : RegExp
> : ^^^^^^
>/> : RegExp
> : ^^^^^^

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
unterminatedRegexAtEndOfSource1.ts(1,10): error TS1161: Unterminated regular expression literal.
unterminatedRegexAtEndOfSource1.ts(1,9): error TS1161: Unterminated regular expression literal.


==== unterminatedRegexAtEndOfSource1.ts (1 errors) ====
var a = /
~
!!! error TS1161: Unterminated regular expression literal.
2 changes: 1 addition & 1 deletion tests/cases/fourslash/whiteSpaceTrimming4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
goTo.marker('1');
edit.insert("\n");

verify.currentFileContentIs("var re = /\\w+ \n /;");
verify.currentFileContentIs("var re = /\\w+\n /;");

0 comments on commit d0ef028

Please sign in to comment.