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
@@ -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);
7 changes: 1 addition & 6 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
@@ -62,7 +62,6 @@ import {
DeleteExpression,
Diagnostic,
DiagnosticArguments,
DiagnosticCategory,
DiagnosticMessage,
Diagnostics,
DiagnosticWithDetachedLocation,
@@ -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);
}
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
@@ -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 {
1 change: 1 addition & 0 deletions src/testRunner/tests.ts
Original file line number Diff line number Diff line change
@@ -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";
2 changes: 1 addition & 1 deletion src/testRunner/unittests/incrementalParser.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
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
@@ -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
@@ -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
@@ -4,4 +4,4 @@
foo(/notregexp);

//// [parserRegularExpressionDivideAmbiguity4.js]
foo(/notregexp););
foo(/notregexp);
Original file line number Diff line number Diff line change
@@ -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
@@ -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) ====
@@ -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
@@ -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
@@ -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
@@ -5,4 +5,4 @@
goTo.marker('1');
edit.insert("\n");

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