Skip to content

Commit 2a01f92

Browse files
authored
Improve JSON parser error recovery (#42657)
* Improve JSON parser error recovery * Add error baselines * Move tsconfig root checking out of common JSON checking function * Use new function in parseConfigFileTextToJson * Fix test * Replace non-null assertion with explicit debug assertion
1 parent 9de8dbb commit 2a01f92

File tree

11 files changed

+285
-32
lines changed

11 files changed

+285
-32
lines changed

src/compiler/commandLineParser.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1603,7 +1603,7 @@ namespace ts {
16031603
export function parseConfigFileTextToJson(fileName: string, jsonText: string): { config?: any; error?: Diagnostic } {
16041604
const jsonSourceFile = parseJsonText(fileName, jsonText);
16051605
return {
1606-
config: convertToObject(jsonSourceFile, jsonSourceFile.parseDiagnostics),
1606+
config: convertConfigFileToObject(jsonSourceFile, jsonSourceFile.parseDiagnostics, /*reportOptionsErrors*/ false, /*optionsIterator*/ undefined),
16071607
error: jsonSourceFile.parseDiagnostics.length ? jsonSourceFile.parseDiagnostics[0] : undefined
16081608
};
16091609
}
@@ -1767,11 +1767,35 @@ namespace ts {
17671767
onSetUnknownOptionKeyValueInRoot(key: string, keyNode: PropertyName, value: CompilerOptionsValue, valueNode: Expression): void;
17681768
}
17691769

1770+
function convertConfigFileToObject(sourceFile: JsonSourceFile, errors: Push<Diagnostic>, reportOptionsErrors: boolean, optionsIterator: JsonConversionNotifier | undefined): any {
1771+
const rootExpression: Expression | undefined = sourceFile.statements[0]?.expression;
1772+
const knownRootOptions = reportOptionsErrors ? getTsconfigRootOptionsMap() : undefined;
1773+
if (rootExpression && rootExpression.kind !== SyntaxKind.ObjectLiteralExpression) {
1774+
errors.push(createDiagnosticForNodeInSourceFile(
1775+
sourceFile,
1776+
rootExpression,
1777+
Diagnostics.The_root_value_of_a_0_file_must_be_an_object,
1778+
getBaseFileName(sourceFile.fileName) === "jsconfig.json" ? "jsconfig.json" : "tsconfig.json"
1779+
));
1780+
// Last-ditch error recovery. Somewhat useful because the JSON parser will recover from some parse errors by
1781+
// synthesizing a top-level array literal expression. There's a reasonable chance the first element of that
1782+
// array is a well-formed configuration object, made into an array element by stray characters.
1783+
if (isArrayLiteralExpression(rootExpression)) {
1784+
const firstObject = find(rootExpression.elements, isObjectLiteralExpression);
1785+
if (firstObject) {
1786+
return convertToObjectWorker(sourceFile, firstObject, errors, /*returnValue*/ true, knownRootOptions, optionsIterator);
1787+
}
1788+
}
1789+
return {};
1790+
}
1791+
return convertToObjectWorker(sourceFile, rootExpression, errors, /*returnValue*/ true, knownRootOptions, optionsIterator);
1792+
}
1793+
17701794
/**
17711795
* Convert the json syntax tree into the json value
17721796
*/
17731797
export function convertToObject(sourceFile: JsonSourceFile, errors: Push<Diagnostic>): any {
1774-
return convertToObjectWorker(sourceFile, errors, /*returnValue*/ true, /*knownRootOptions*/ undefined, /*jsonConversionNotifier*/ undefined);
1798+
return convertToObjectWorker(sourceFile, sourceFile.statements[0]?.expression, errors, /*returnValue*/ true, /*knownRootOptions*/ undefined, /*jsonConversionNotifier*/ undefined);
17751799
}
17761800

17771801
/**
@@ -1782,15 +1806,16 @@ namespace ts {
17821806
/*@internal*/
17831807
export function convertToObjectWorker(
17841808
sourceFile: JsonSourceFile,
1809+
rootExpression: Expression | undefined,
17851810
errors: Push<Diagnostic>,
17861811
returnValue: boolean,
17871812
knownRootOptions: CommandLineOption | undefined,
17881813
jsonConversionNotifier: JsonConversionNotifier | undefined): any {
1789-
if (!sourceFile.statements.length) {
1814+
if (!rootExpression) {
17901815
return returnValue ? {} : undefined;
17911816
}
17921817

1793-
return convertPropertyValueToJson(sourceFile.statements[0].expression, knownRootOptions);
1818+
return convertPropertyValueToJson(rootExpression, knownRootOptions);
17941819

17951820
function isRootOptionMap(knownOptions: ESMap<string, CommandLineOption> | undefined) {
17961821
return knownRootOptions && (knownRootOptions as TsConfigOnlyOption).elementOptions === knownOptions;
@@ -2733,7 +2758,7 @@ namespace ts {
27332758
}
27342759
}
27352760
};
2736-
const json = convertToObjectWorker(sourceFile, errors, /*returnValue*/ true, getTsconfigRootOptionsMap(), optionsIterator);
2761+
const json = convertConfigFileToObject(sourceFile, errors, /*reportOptionsErrors*/ true, optionsIterator);
27372762

27382763
if (!typeAcquisition) {
27392764
if (typingOptionstypeAcquisition) {

src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3850,6 +3850,10 @@
38503850
"category": "Error",
38513851
"code": 5091
38523852
},
3853+
"The root value of a '{0}' file must be an object.": {
3854+
"category": "Error",
3855+
"code": 5092
3856+
},
38533857

38543858
"Generates a sourcemap for each corresponding '.d.ts' file.": {
38553859
"category": "Message",

src/compiler/parser.ts

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,7 @@ namespace ts {
819819
scriptKind = ensureScriptKind(fileName, scriptKind);
820820
if (scriptKind === ScriptKind.JSON) {
821821
const result = parseJsonText(fileName, sourceText, languageVersion, syntaxCursor, setParentNodes);
822-
convertToObjectWorker(result, result.parseDiagnostics, /*returnValue*/ false, /*knownRootOptions*/ undefined, /*jsonConversionNotifier*/ undefined);
822+
convertToObjectWorker(result, result.statements[0]?.expression, result.parseDiagnostics, /*returnValue*/ false, /*knownRootOptions*/ undefined, /*jsonConversionNotifier*/ undefined);
823823
result.referencedFiles = emptyArray;
824824
result.typeReferenceDirectives = emptyArray;
825825
result.libReferenceDirectives = emptyArray;
@@ -862,36 +862,56 @@ namespace ts {
862862
endOfFileToken = parseTokenNode<EndOfFileToken>();
863863
}
864864
else {
865-
let expression;
866-
switch (token()) {
867-
case SyntaxKind.OpenBracketToken:
868-
expression = parseArrayLiteralExpression();
869-
break;
870-
case SyntaxKind.TrueKeyword:
871-
case SyntaxKind.FalseKeyword:
872-
case SyntaxKind.NullKeyword:
873-
expression = parseTokenNode<BooleanLiteral | NullLiteral>();
874-
break;
875-
case SyntaxKind.MinusToken:
876-
if (lookAhead(() => nextToken() === SyntaxKind.NumericLiteral && nextToken() !== SyntaxKind.ColonToken)) {
877-
expression = parsePrefixUnaryExpression() as JsonMinusNumericLiteral;
878-
}
879-
else {
865+
// Loop and synthesize an ArrayLiteralExpression if there are more than
866+
// one top-level expressions to ensure all input text is consumed.
867+
let expressions: Expression[] | Expression | undefined;
868+
while (token() !== SyntaxKind.EndOfFileToken) {
869+
let expression;
870+
switch (token()) {
871+
case SyntaxKind.OpenBracketToken:
872+
expression = parseArrayLiteralExpression();
873+
break;
874+
case SyntaxKind.TrueKeyword:
875+
case SyntaxKind.FalseKeyword:
876+
case SyntaxKind.NullKeyword:
877+
expression = parseTokenNode<BooleanLiteral | NullLiteral>();
878+
break;
879+
case SyntaxKind.MinusToken:
880+
if (lookAhead(() => nextToken() === SyntaxKind.NumericLiteral && nextToken() !== SyntaxKind.ColonToken)) {
881+
expression = parsePrefixUnaryExpression() as JsonMinusNumericLiteral;
882+
}
883+
else {
884+
expression = parseObjectLiteralExpression();
885+
}
886+
break;
887+
case SyntaxKind.NumericLiteral:
888+
case SyntaxKind.StringLiteral:
889+
if (lookAhead(() => nextToken() !== SyntaxKind.ColonToken)) {
890+
expression = parseLiteralNode() as StringLiteral | NumericLiteral;
891+
break;
892+
}
893+
// falls through
894+
default:
880895
expression = parseObjectLiteralExpression();
881-
}
882-
break;
883-
case SyntaxKind.NumericLiteral:
884-
case SyntaxKind.StringLiteral:
885-
if (lookAhead(() => nextToken() !== SyntaxKind.ColonToken)) {
886-
expression = parseLiteralNode() as StringLiteral | NumericLiteral;
887896
break;
897+
}
898+
899+
// Error recovery: collect multiple top-level expressions
900+
if (expressions && isArray(expressions)) {
901+
expressions.push(expression);
902+
}
903+
else if (expressions) {
904+
expressions = [expressions, expression];
905+
}
906+
else {
907+
expressions = expression;
908+
if (token() !== SyntaxKind.EndOfFileToken) {
909+
parseErrorAtCurrentToken(Diagnostics.Unexpected_token);
888910
}
889-
// falls through
890-
default:
891-
expression = parseObjectLiteralExpression();
892-
break;
911+
}
893912
}
894913

914+
const expression = isArray(expressions) ? finishNode(factory.createArrayLiteralExpression(expressions), pos) : Debug.checkDefined(expressions);
895915
const statement = factory.createExpressionStatement(expression) as JsonObjectExpressionStatement;
896916
finishNode(statement, pos);
897917
statements = createNodeArray([statement], pos);

src/testRunner/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
"unittests/factory.ts",
6767
"unittests/incrementalParser.ts",
6868
"unittests/jsDocParsing.ts",
69+
"unittests/jsonParserRecovery.ts",
6970
"unittests/moduleResolution.ts",
7071
"unittests/parsePseudoBigInt.ts",
7172
"unittests/paths.ts",

src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ namespace ts {
6969
assert.equal(actualError.category, expectedError.category, `Expected error-category: ${JSON.stringify(expectedError.category)}. Actual error-category: ${JSON.stringify(actualError.category)}.`);
7070
if (!ignoreLocation) {
7171
assert(actualError.file);
72-
assert(actualError.start);
72+
assert.isDefined(actualError.start);
7373
assert(actualError.length);
7474
}
7575
}
@@ -603,5 +603,84 @@ namespace ts {
603603
hasParseErrors: true
604604
});
605605
});
606+
607+
it("Convert a tsconfig file with stray trailing characters", () => {
608+
assertCompilerOptionsWithJsonText(`{
609+
"compilerOptions": {
610+
"target": "esnext"
611+
}
612+
} blah`, "tsconfig.json", {
613+
compilerOptions: {
614+
target: ScriptTarget.ESNext
615+
},
616+
hasParseErrors: true,
617+
errors: [{
618+
...Diagnostics.The_root_value_of_a_0_file_must_be_an_object,
619+
messageText: "The root value of a 'tsconfig.json' file must be an object.",
620+
file: undefined,
621+
start: 0,
622+
length: 0
623+
}]
624+
});
625+
});
626+
627+
it("Convert a tsconfig file with stray leading characters", () => {
628+
assertCompilerOptionsWithJsonText(`blah {
629+
"compilerOptions": {
630+
"target": "esnext"
631+
}
632+
}`, "tsconfig.json", {
633+
compilerOptions: {
634+
target: ScriptTarget.ESNext
635+
},
636+
hasParseErrors: true,
637+
errors: [{
638+
...Diagnostics.The_root_value_of_a_0_file_must_be_an_object,
639+
messageText: "The root value of a 'tsconfig.json' file must be an object.",
640+
file: undefined,
641+
start: 0,
642+
length: 0
643+
}]
644+
});
645+
});
646+
647+
it("Convert a tsconfig file as an array", () => {
648+
assertCompilerOptionsWithJsonText(`[{
649+
"compilerOptions": {
650+
"target": "esnext"
651+
}
652+
}]`, "tsconfig.json", {
653+
compilerOptions: {
654+
target: ScriptTarget.ESNext
655+
},
656+
errors: [{
657+
...Diagnostics.The_root_value_of_a_0_file_must_be_an_object,
658+
messageText: "The root value of a 'tsconfig.json' file must be an object.",
659+
file: undefined,
660+
start: 0,
661+
length: 0
662+
}]
663+
});
664+
});
665+
666+
it("Don't crash when root expression is not objecty at all", () => {
667+
assertCompilerOptionsWithJsonText(`42`, "tsconfig.json", {
668+
compilerOptions: {},
669+
errors: [{
670+
...Diagnostics.The_root_value_of_a_0_file_must_be_an_object,
671+
messageText: "The root value of a 'tsconfig.json' file must be an object.",
672+
file: undefined,
673+
start: 0,
674+
length: 0
675+
}]
676+
});
677+
});
678+
679+
it("Allow trailing comments", () => {
680+
assertCompilerOptionsWithJsonText(`{} // no options`, "tsconfig.json", {
681+
compilerOptions: {},
682+
errors: []
683+
});
684+
});
606685
});
607686
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
namespace ts {
2+
describe("unittests:: jsonParserRecovery", () => {
3+
function parsesToValidSourceFileWithErrors(name: string, text: string) {
4+
it(name, () => {
5+
const file = parseJsonText(name, text);
6+
assert(file.parseDiagnostics.length, "Should have parse errors");
7+
Harness.Baseline.runBaseline(
8+
`jsonParserRecovery/${name.replace(/[^a-z0-9_-]/ig, "_")}.errors.txt`,
9+
Harness.Compiler.getErrorBaseline([{
10+
content: text,
11+
unitName: name
12+
}], file.parseDiagnostics));
13+
14+
// Will throw if parse tree does not cover full input text
15+
file.getChildren();
16+
});
17+
}
18+
19+
parsesToValidSourceFileWithErrors("trailing identifier", "{} blah");
20+
parsesToValidSourceFileWithErrors("TypeScript code", "interface Foo {} blah");
21+
parsesToValidSourceFileWithErrors("Two comma-separated objects", "{}, {}");
22+
parsesToValidSourceFileWithErrors("Two objects", "{} {}");
23+
parsesToValidSourceFileWithErrors("JSX", `
24+
interface Test {}
25+
26+
const Header = () => (
27+
<div>
28+
<h1>Header</h1>
29+
<style jsx>
30+
{\`
31+
h1 {
32+
color: red;
33+
}
34+
\`}
35+
</style>
36+
</div>
37+
)`);
38+
});
39+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
JSX(2,9): error TS1005: '{' expected.
2+
JSX(2,19): error TS1005: ',' expected.
3+
JSX(2,24): error TS1005: ',' expected.
4+
JSX(4,9): error TS1012: Unexpected token.
5+
JSX(4,15): error TS1005: ':' expected.
6+
JSX(15,10): error TS1005: '}' expected.
7+
8+
9+
==== JSX (6 errors) ====
10+
11+
interface Test {}
12+
~~~~~~~~~
13+
!!! error TS1005: '{' expected.
14+
~~~~
15+
!!! error TS1005: ',' expected.
16+
~
17+
!!! error TS1005: ',' expected.
18+
19+
const Header = () => (
20+
~~~~~
21+
!!! error TS1012: Unexpected token.
22+
~~~~~~
23+
!!! error TS1005: ':' expected.
24+
<div>
25+
<h1>Header</h1>
26+
<style jsx>
27+
{`
28+
h1 {
29+
color: red;
30+
}
31+
`}
32+
</style>
33+
</div>
34+
)
35+
36+
!!! error TS1005: '}' expected.
37+
!!! related TS1007 JSX:4:9: The parser expected to find a '}' to match the '{' token here.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Two comma-separated objects(1,3): error TS1012: Unexpected token.
2+
Two comma-separated objects(1,5): error TS1136: Property assignment expected.
3+
4+
5+
==== Two comma-separated objects (2 errors) ====
6+
{}, {}
7+
~
8+
!!! error TS1012: Unexpected token.
9+
~
10+
!!! error TS1136: Property assignment expected.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Two objects(1,4): error TS1012: Unexpected token.
2+
3+
4+
==== Two objects (1 errors) ====
5+
{} {}
6+
~
7+
!!! error TS1012: Unexpected token.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
TypeScript code(1,1): error TS1005: '{' expected.
2+
TypeScript code(1,11): error TS1005: ',' expected.
3+
TypeScript code(1,15): error TS1005: ',' expected.
4+
TypeScript code(1,18): error TS1012: Unexpected token.
5+
TypeScript code(1,22): error TS1005: '}' expected.
6+
7+
8+
==== TypeScript code (5 errors) ====
9+
interface Foo {} blah
10+
~~~~~~~~~
11+
!!! error TS1005: '{' expected.
12+
~~~
13+
!!! error TS1005: ',' expected.
14+
~
15+
!!! error TS1005: ',' expected.
16+
~~~~
17+
!!! error TS1012: Unexpected token.
18+
19+
!!! error TS1005: '}' expected.
20+
!!! related TS1007 TypeScript code:1:18: The parser expected to find a '}' to match the '{' token here.

0 commit comments

Comments
 (0)