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

Improve Recovery of Unterminated Regular Expressions #58289

Merged
merged 11 commits into from
May 24, 2024
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31885,7 +31885,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.
~~
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include angle brackets (in addition to parens and braces) to the logic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With respect to treating them as unterminated? I don't think that's necessary. (?<) will still produce an error during the grammar check pass while still maintaining a proper bound for the RegExp body.

Also, Annex B does not treat /\k</ as an error since the RegExp does not define a named capture group, which indicates that <> handling is not a lexical syntax error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too terribly concerned about this case, to be honest. Parsing falls off the rails here because of malformed JSX, not a malformed RegExp.

!!! 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 /;");