Skip to content

Commit 4c378c0

Browse files
sheetalkamatsheetalkamat
authored
Report config file parsing diagnostics correctly with tsc --b (microsoft#36520)
* Refactor the test * Add tests for syntax errors in tsconfig not being reported * Report config file parsing diagnostics correctly Fixes microsoft#36515 * Fix errors in existing tests for unintended tsconfig parse errors * Fix lint
1 parent c1e45ac commit 4c378c0

File tree

34 files changed

+543
-84
lines changed

34 files changed

+543
-84
lines changed

‎src/compiler/tsbuildPublic.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ namespace ts {
828828
if (state.options.verbose) reportStatus(state, Diagnostics.Building_project_0, project);
829829

830830
if (config.fileNames.length === 0) {
831-
reportAndStoreErrors(state, projectPath, config.errors);
831+
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
832832
// Nothing to build - must be a solution file, basically
833833
buildResult = BuildResultFlags.None;
834834
step = Step.QueueReferencingProjects;
@@ -846,7 +846,7 @@ namespace ts {
846846
config.options,
847847
compilerHost,
848848
getOldProgram(state, projectPath, config),
849-
config.errors,
849+
getConfigFileParsingDiagnostics(config),
850850
config.projectReferences
851851
);
852852
step++;
@@ -1118,7 +1118,7 @@ namespace ts {
11181118
function needsBuild({ options }: SolutionBuilderState, status: UpToDateStatus, config: ParsedCommandLine) {
11191119
if (status.type !== UpToDateStatusType.OutOfDateWithPrepend || options.force) return true;
11201120
return config.fileNames.length === 0 ||
1121-
!!config.errors.length ||
1121+
!!getConfigFileParsingDiagnostics(config).length ||
11221122
!isIncrementalCompilation(config.options);
11231123
}
11241124

@@ -1172,7 +1172,7 @@ namespace ts {
11721172
verboseReportProjectStatus(state, project, status);
11731173
if (!options.force) {
11741174
if (status.type === UpToDateStatusType.UpToDate) {
1175-
reportAndStoreErrors(state, projectPath, config.errors);
1175+
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
11761176
projectPendingBuild.delete(projectPath);
11771177
// Up to date, skip
11781178
if (options.dry) {
@@ -1183,7 +1183,7 @@ namespace ts {
11831183
}
11841184

11851185
if (status.type === UpToDateStatusType.UpToDateWithUpstreamTypes) {
1186-
reportAndStoreErrors(state, projectPath, config.errors);
1186+
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
11871187
return createUpdateOutputFileStampsProject(
11881188
state,
11891189
project,
@@ -1195,7 +1195,7 @@ namespace ts {
11951195
}
11961196

11971197
if (status.type === UpToDateStatusType.UpstreamBlocked) {
1198-
reportAndStoreErrors(state, projectPath, config.errors);
1198+
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
11991199
projectPendingBuild.delete(projectPath);
12001200
if (options.verbose) {
12011201
reportStatus(
@@ -1211,7 +1211,7 @@ namespace ts {
12111211
}
12121212

12131213
if (status.type === UpToDateStatusType.ContainerOnly) {
1214-
reportAndStoreErrors(state, projectPath, config.errors);
1214+
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
12151215
projectPendingBuild.delete(projectPath);
12161216
// Do nothing
12171217
continue;

‎src/testRunner/tsconfig.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
"unittests/services/textChanges.ts",
108108
"unittests/services/transpile.ts",
109109
"unittests/tsbuild/amdModulesWithOut.ts",
110+
"unittests/tsbuild/configFileErrors.ts",
110111
"unittests/tsbuild/containerOnlyReferenced.ts",
111112
"unittests/tsbuild/demo.ts",
112113
"unittests/tsbuild/emitDeclarationOnly.ts",
@@ -116,7 +117,6 @@
116117
"unittests/tsbuild/inferredTypeFromTransitiveModule.ts",
117118
"unittests/tsbuild/javascriptProjectEmit.ts",
118119
"unittests/tsbuild/lateBoundSymbol.ts",
119-
"unittests/tsbuild/missingExtendedFile.ts",
120120
"unittests/tsbuild/moduleSpecifiers.ts",
121121
"unittests/tsbuild/noEmitOnError.ts",
122122
"unittests/tsbuild/outFile.ts",

‎src/testRunner/unittests/tsbuild/configFileErrors.ts

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
namespace ts {
2+
describe("unittests:: tsbuild:: configFileErrors:: when tsconfig extends the missing file", () => {
3+
verifyTsc({
4+
scenario: "configFileErrors",
5+
subScenario: "when tsconfig extends the missing file",
6+
fs: () => loadProjectFromDisk("tests/projects/missingExtendedConfig"),
7+
commandLineArgs: ["--b", "/src/tsconfig.json"],
8+
});
9+
});
10+
11+
describe("unittests:: tsbuild:: configFileErrors:: reports syntax errors in config file", () => {
12+
verifyTscIncrementalEdits({
13+
scenario: "configFileErrors",
14+
subScenario: "reports syntax errors in config file",
15+
fs: () => loadProjectFromFiles({
16+
"/src/a.ts": "export function foo() { }",
17+
"/src/b.ts": "export function bar() { }",
18+
"/src/tsconfig.json": Utils.dedent`
19+
{
20+
"compilerOptions": {
21+
"composite": true,
22+
},
23+
"files": [
24+
"a.ts"
25+
"b.ts"
26+
]
27+
}`
28+
}),
29+
commandLineArgs: ["--b", "/src/tsconfig.json"],
30+
incrementalScenarios: [
31+
{
32+
buildKind: BuildKind.IncrementalDtsUnchanged,
33+
modifyFs: fs => replaceText(fs, "/src/tsconfig.json", ",", `,
34+
"declaration": true,`),
35+
subScenario: "reports syntax errors after change to config file"
36+
},
37+
{
38+
buildKind: BuildKind.IncrementalDtsUnchanged,
39+
modifyFs: fs => appendText(fs, "/src/a.ts", "export function fooBar() { }"),
40+
subScenario: "reports syntax errors after change to ts file"
41+
},
42+
noChangeRun,
43+
{
44+
buildKind: BuildKind.IncrementalDtsChange,
45+
modifyFs: fs => fs.writeFileSync(
46+
"/src/tsconfig.json",
47+
JSON.stringify({
48+
compilerOptions: { composite: true, declaration: true },
49+
files: ["a.ts", "b.ts"]
50+
})
51+
),
52+
subScenario: "builds after fixing config file errors"
53+
},
54+
]
55+
});
56+
});
57+
}

‎src/testRunner/unittests/tsbuild/containerOnlyReferenced.ts

+1-10
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,9 @@
11
namespace ts {
22
describe("unittests:: tsbuild:: when containerOnly project is referenced", () => {
3-
let projFs: vfs.FileSystem;
4-
before(() => {
5-
projFs = loadProjectFromDisk("tests/projects/containerOnlyReferenced");
6-
});
7-
8-
after(() => {
9-
projFs = undefined!; // Release the contents
10-
});
11-
123
verifyTscIncrementalEdits({
134
scenario: "containerOnlyReferenced",
145
subScenario: "verify that subsequent builds after initial build doesnt build anything",
15-
fs: () => projFs,
6+
fs: () => loadProjectFromDisk("tests/projects/containerOnlyReferenced"),
167
commandLineArgs: ["--b", "/src", "--verbose"],
178
incrementalScenarios: [noChangeRun]
189
});

‎src/testRunner/unittests/tsbuild/javascriptProjectEmit.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ namespace ts {
212212
{
213213
"extends": "../tsconfig.base.json",
214214
"compilerOptions": {
215-
"outDir": null
215+
"outDir": null,
216216
"composite": true
217217
},
218218
"include": ["index.ts", "obj.json"]

‎src/testRunner/unittests/tsbuild/lateBoundSymbol.ts

+1-9
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,8 @@
11
namespace ts {
22
describe("unittests:: tsbuild:: lateBoundSymbol:: interface is merged and contains late bound member", () => {
3-
let projFs: vfs.FileSystem;
4-
before(() => {
5-
projFs = loadProjectFromDisk("tests/projects/lateBoundSymbol");
6-
});
7-
after(() => {
8-
projFs = undefined!; // Release the contents
9-
});
10-
113
verifyTscIncrementalEdits({
124
subScenario: "interface is merged and contains late bound member",
13-
fs: () => projFs,
5+
fs: () => loadProjectFromDisk("tests/projects/lateBoundSymbol"),
146
scenario: "lateBoundSymbol",
157
commandLineArgs: ["--b", "/src/tsconfig.json", "--verbose"],
168
incrementalScenarios: [{

‎src/testRunner/unittests/tsbuild/missingExtendedFile.ts

-17
This file was deleted.

‎src/testRunner/unittests/tsbuild/noEmitOnError.ts

+1-8
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,9 @@
11
namespace ts {
22
describe("unittests:: tsbuild - with noEmitOnError", () => {
3-
let projFs: vfs.FileSystem;
4-
before(() => {
5-
projFs = loadProjectFromDisk("tests/projects/noEmitOnError");
6-
});
7-
after(() => {
8-
projFs = undefined!;
9-
});
103
verifyTsc({
114
scenario: "noEmitOnError",
125
subScenario: "has empty files diagnostic when files is empty and no references are provided",
13-
fs: () => projFs,
6+
fs: () => loadProjectFromDisk("tests/projects/noEmitOnError"),
147
commandLineArgs: ["--b", "/src/tsconfig.json"],
158
});
169
});

‎src/testRunner/unittests/tsbuild/outFile.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ namespace ts {
453453

454454
function stripInternalOfThird(fs: vfs.FileSystem) {
455455
replaceText(fs, sources[project.third][source.config], `"declaration": true,`, `"declaration": true,
456-
"stripInternal": true`);
456+
"stripInternal": true,`);
457457
}
458458

459459
function stripInternalScenario(fs: vfs.FileSystem, removeCommentsDisabled?: boolean, jsDocStyle?: boolean) {

‎src/testRunner/unittests/tsbuild/resolveJsonModule.ts

+1-10
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,10 @@ export default hello.hello`);
7171
});
7272

7373
describe("unittests:: tsbuild:: with resolveJsonModule option on project importJsonFromProjectReference", () => {
74-
let projFs: vfs.FileSystem;
75-
before(() => {
76-
projFs = loadProjectFromDisk("tests/projects/importJsonFromProjectReference");
77-
});
78-
79-
after(() => {
80-
projFs = undefined!; // Release the contents
81-
});
82-
8374
verifyTscIncrementalEdits({
8475
scenario: "resolveJsonModule",
8576
subScenario: "importing json module from project reference",
86-
fs: () => projFs,
77+
fs: () => loadProjectFromDisk("tests/projects/importJsonFromProjectReference"),
8778
commandLineArgs: ["--b", "src/tsconfig.json", "--verbose"],
8879
incrementalScenarios: [noChangeRun]
8980
});

‎src/testRunner/unittests/tsbuild/watchMode.ts

+61-4
Original file line numberDiff line numberDiff line change
@@ -1266,17 +1266,15 @@ const a = {
12661266
),
12671267
changes: [
12681268
sys => {
1269-
const content = sys.readFile(`${projectsLocation}/reexport/src/pure/session.ts`)!;
1270-
sys.writeFile(`${projectsLocation}/reexport/src/pure/session.ts`, content.replace("// ", ""));
1269+
replaceFileText(sys, `${projectsLocation}/reexport/src/pure/session.ts`, "// ", "");
12711270
sys.checkTimeoutQueueLengthAndRun(1); // build src/pure
12721271
sys.checkTimeoutQueueLengthAndRun(1); // build src/main
12731272
sys.checkTimeoutQueueLengthAndRun(1); // build src
12741273
sys.checkTimeoutQueueLength(0);
12751274
return "Introduce error";
12761275
},
12771276
sys => {
1278-
const content = sys.readFile(`${projectsLocation}/reexport/src/pure/session.ts`)!;
1279-
sys.writeFile(`${projectsLocation}/reexport/src/pure/session.ts`, content.replace("bar: ", "// bar: "));
1277+
replaceFileText(sys, `${projectsLocation}/reexport/src/pure/session.ts`, "bar: ", "// bar: ");
12801278
sys.checkTimeoutQueueLengthAndRun(1); // build src/pure
12811279
sys.checkTimeoutQueueLengthAndRun(1); // build src/main
12821280
sys.checkTimeoutQueueLengthAndRun(1); // build src
@@ -1286,4 +1284,63 @@ const a = {
12861284
]
12871285
});
12881286
});
1287+
1288+
describe("unittests:: tsbuild:: watchMode:: configFileErrors:: reports syntax errors in config file", () => {
1289+
verifyTscWatch({
1290+
scenario: "configFileErrors",
1291+
subScenario: "reports syntax errors in config file",
1292+
sys: () => createWatchedSystem(
1293+
[
1294+
{ path: `${projectRoot}/a.ts`, content: "export function foo() { }" },
1295+
{ path: `${projectRoot}/b.ts`, content: "export function bar() { }" },
1296+
{
1297+
path: `${projectRoot}/tsconfig.json`,
1298+
content: Utils.dedent`
1299+
{
1300+
"compilerOptions": {
1301+
"composite": true,
1302+
},
1303+
"files": [
1304+
"a.ts"
1305+
"b.ts"
1306+
]
1307+
}`
1308+
},
1309+
libFile
1310+
],
1311+
{ currentDirectory: projectRoot }
1312+
),
1313+
commandLineArgs: ["--b", "-w"],
1314+
changes: [
1315+
sys => {
1316+
replaceFileText(sys, `${projectRoot}/tsconfig.json`, ",", `,
1317+
"declaration": true,`);
1318+
sys.checkTimeoutQueueLengthAndRun(1); // build the project
1319+
sys.checkTimeoutQueueLength(0);
1320+
return "reports syntax errors after change to config file";
1321+
},
1322+
sys => {
1323+
replaceFileText(sys, `${projectRoot}/a.ts`, "foo", "fooBar");
1324+
sys.checkTimeoutQueueLengthAndRun(1); // build the project
1325+
sys.checkTimeoutQueueLength(0);
1326+
return "reports syntax errors after change to ts file";
1327+
},
1328+
sys => {
1329+
replaceFileText(sys, `${projectRoot}/tsconfig.json`, "", "");
1330+
sys.checkTimeoutQueueLengthAndRun(1); // build the project
1331+
sys.checkTimeoutQueueLength(0);
1332+
return "reports error when there is no change to tsconfig file";
1333+
},
1334+
sys => {
1335+
sys.writeFile(`${projectRoot}/tsconfig.json`, JSON.stringify({
1336+
compilerOptions: { composite: true, declaration: true },
1337+
files: ["a.ts", "b.ts"]
1338+
}));
1339+
sys.checkTimeoutQueueLengthAndRun(1); // build the project
1340+
sys.checkTimeoutQueueLength(0);
1341+
return "builds after fixing config file errors";
1342+
}
1343+
]
1344+
});
1345+
});
12891346
}

‎src/testRunner/unittests/tscWatch/helpers.ts

+5
Original file line numberDiff line numberDiff line change
@@ -417,4 +417,9 @@ namespace ts.tscWatch {
417417
});
418418
});
419419
}
420+
421+
export function replaceFileText(sys: WatchedSystem, file: string, searchValue: string | RegExp, replaceValue: string) {
422+
const content = Debug.assertDefined(sys.readFile(file));
423+
sys.writeFile(file, content.replace(searchValue, replaceValue));
424+
}
420425
}

‎src/testRunner/unittests/tscWatch/programUpdates.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -1086,8 +1086,7 @@ export function two() {
10861086
});
10871087

10881088
function changeParameterTypeOfBFile(sys: WatchedSystem, parameterName: string, toType: string) {
1089-
const oldContent = sys.readFile(`${projectRoot}/b.ts`)!;
1090-
sys.writeFile(`${projectRoot}/b.ts`, oldContent.replace(new RegExp(`${parameterName}\: [a-z]*`), `${parameterName}: ${toType}`));
1089+
replaceFileText(sys, `${projectRoot}/b.ts`, new RegExp(`${parameterName}\: [a-z]*`), `${parameterName}: ${toType}`);
10911090
sys.runQueuedTimeoutCallbacks();
10921091
return `Changed ${parameterName} type to ${toType}`;
10931092
}

‎tests/baselines/reference/tsbuild/amdModulesWithOut/initial-build/multiple-emitHelpers-in-all-projects.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,7 @@ declare function appfile4Spread(...b: number[]): void;
10461046
"declarationMap": true,
10471047
"outFile": "module.js"
10481048
},
1049-
"exclude": ["module.d.ts"]
1049+
"exclude": ["module.d.ts"],
10501050
"references": [
10511051
{ "path": "../lib", "prepend": true }
10521052
]

‎tests/baselines/reference/tsbuild/amdModulesWithOut/initial-build/multiple-prologues-in-all-projects.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ declare const myVar = 30;
679679
"declarationMap": true,
680680
"outFile": "module.js"
681681
},
682-
"exclude": ["module.d.ts"]
682+
"exclude": ["module.d.ts"],
683683
"references": [
684684
{ "path": "../lib", "prepend": true }
685685
]

‎tests/baselines/reference/tsbuild/amdModulesWithOut/initial-build/stripInternal.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2092,7 +2092,7 @@ declare const myVar = 30;
20922092
"declarationMap": true,
20932093
"outFile": "module.js"
20942094
},
2095-
"exclude": ["module.d.ts"]
2095+
"exclude": ["module.d.ts"],
20962096
"references": [
20972097
{ "path": "../lib", "prepend": true }
20982098
]

0 commit comments

Comments
 (0)