Skip to content

Commit 83e6db4

Browse files
dgp1130atscott
dgp1130
authored and
atscott
committed
refactor(compiler-cli): add validation to extended template diagnostics configuration (#44391)
Refs #42966. This validates the `tsconfig.json` options for extended template diagnostics. It verifies: * `strictTemplates` must be enabled if `extendedDiagnostics` have any explicit configuration. * `extendedDiagnostics.defaultCategory` must be a valid `DiagnosticCategoryLabel`. * `extendedDiagnostics.checks` keys must all be real diagnostics. * `extendedDiagnostics.checks` values must all be valid `DiagnosticCategoryLabel`s. These include new error codes, each of which prints out the exact property that was the issue and what the available options are to fix them. It does disallow the config: ```json { "angularCompilerOptions": { "strictTemplates": false, "extendedDiagnostics": { "defaultCategory": "suppress" } } } ``` Such a configuration is technically valid and could be executed, but will be rejected by this verification logic. There isn't much reason to ever do this, since users could just drop `extendedDiagnostics` altogether and get the intended effect. This is unlikely to be a significant issue for users, so it is considered invalid for now to keep the implementation simple. PR Close #44391
1 parent 9e5ac51 commit 83e6db4

File tree

6 files changed

+228
-19
lines changed

6 files changed

+228
-19
lines changed

‎goldens/public-api/compiler-cli/error_code.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ export enum ErrorCode {
1111
COMPONENT_MISSING_TEMPLATE = 2001,
1212
COMPONENT_RESOURCE_NOT_FOUND = 2008,
1313
// (undocumented)
14+
CONFIG_EXTENDED_DIAGNOSTICS_IMPLIES_STRICT_TEMPLATES = 4003,
15+
// (undocumented)
16+
CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CATEGORY_LABEL = 4004,
17+
// (undocumented)
18+
CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CHECK = 4005,
19+
// (undocumented)
1420
CONFIG_FLAT_MODULE_NO_INDEX = 4001,
1521
// (undocumented)
1622
CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,

‎packages/compiler-cli/src/ngtsc/core/src/compiler.ts‎

Lines changed: 90 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {ALL_DIAGNOSTIC_FACTORIES, ExtendedTemplateCheckerImpl} from '../../typec
3232
import {ExtendedTemplateChecker} from '../../typecheck/extended/api';
3333
import {getSourceFileOrNull, isDtsPath, toUnredirectedSourceFile} from '../../util/src/typescript';
3434
import {Xi18nContext} from '../../xi18n';
35-
import {NgCompilerAdapter, NgCompilerOptions} from '../api';
35+
import {DiagnosticCategoryLabel, NgCompilerAdapter, NgCompilerOptions} from '../api';
3636

3737
/**
3838
* State information about a compilation which is only generated once some data is requested from
@@ -322,11 +322,8 @@ export class NgCompiler {
322322
'The \'_extendedTemplateDiagnostics\' option requires \'strictTemplates\' to also be enabled.');
323323
}
324324

325-
this.constructionDiagnostics.push(...this.adapter.constructionDiagnostics);
326-
const incompatibleTypeCheckOptionsDiagnostic = verifyCompatibleTypeCheckOptions(this.options);
327-
if (incompatibleTypeCheckOptionsDiagnostic !== null) {
328-
this.constructionDiagnostics.push(incompatibleTypeCheckOptionsDiagnostic);
329-
}
325+
this.constructionDiagnostics.push(
326+
...this.adapter.constructionDiagnostics, ...verifyCompatibleTypeCheckOptions(this.options));
330327

331328
this.currentProgram = inputProgram;
332329
this.closureCompilerEnabled = !!this.options.annotateForClosureCompiler;
@@ -1135,16 +1132,15 @@ function getR3SymbolsFile(program: ts.Program): ts.SourceFile|null {
11351132
* "fullTemplateTypeCheck", it is required that the latter is not explicitly disabled if the
11361133
* former is enabled.
11371134
*/
1138-
function verifyCompatibleTypeCheckOptions(options: NgCompilerOptions): ts.Diagnostic|null {
1135+
function*
1136+
verifyCompatibleTypeCheckOptions(options: NgCompilerOptions):
1137+
Generator<ts.Diagnostic, void, void> {
11391138
if (options.fullTemplateTypeCheck === false && options.strictTemplates === true) {
1140-
return {
1139+
yield makeConfigDiagnostic({
11411140
category: ts.DiagnosticCategory.Error,
1142-
code: ngErrorCode(ErrorCode.CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK),
1143-
file: undefined,
1144-
start: undefined,
1145-
length: undefined,
1146-
messageText:
1147-
`Angular compiler option "strictTemplates" is enabled, however "fullTemplateTypeCheck" is disabled.
1141+
code: ErrorCode.CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK,
1142+
messageText: `
1143+
Angular compiler option "strictTemplates" is enabled, however "fullTemplateTypeCheck" is disabled.
11481144
11491145
Having the "strictTemplates" flag enabled implies that "fullTemplateTypeCheck" is also enabled, so
11501146
the latter can not be explicitly disabled.
@@ -1154,11 +1150,88 @@ One of the following actions is required:
11541150
2. Remove "strictTemplates" or set it to 'false'.
11551151
11561152
More information about the template type checking compiler options can be found in the documentation:
1157-
https://angular.io/guide/template-typecheck`,
1158-
};
1153+
https://angular.io/guide/template-typecheck
1154+
`.trim(),
1155+
});
1156+
}
1157+
1158+
if (options.extendedDiagnostics && options.strictTemplates === false) {
1159+
yield makeConfigDiagnostic({
1160+
category: ts.DiagnosticCategory.Error,
1161+
code: ErrorCode.CONFIG_EXTENDED_DIAGNOSTICS_IMPLIES_STRICT_TEMPLATES,
1162+
messageText: `
1163+
Angular compiler option "extendedDiagnostics" is configured, however "strictTemplates" is disabled.
1164+
1165+
Using "extendedDiagnostics" requires that "strictTemplates" is also enabled.
1166+
1167+
One of the following actions is required:
1168+
1. Remove "strictTemplates: false" to enable it.
1169+
2. Remove "extendedDiagnostics" configuration to disable them.
1170+
`.trim(),
1171+
});
11591172
}
11601173

1161-
return null;
1174+
const allowedCategoryLabels = Array.from(Object.values(DiagnosticCategoryLabel)) as string[];
1175+
const defaultCategory = options.extendedDiagnostics?.defaultCategory;
1176+
if (defaultCategory && !allowedCategoryLabels.includes(defaultCategory)) {
1177+
yield makeConfigDiagnostic({
1178+
category: ts.DiagnosticCategory.Error,
1179+
code: ErrorCode.CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CATEGORY_LABEL,
1180+
messageText: `
1181+
Angular compiler option "extendedDiagnostics.defaultCategory" has an unknown diagnostic category: "${
1182+
defaultCategory}".
1183+
1184+
Allowed diagnostic categories are:
1185+
${allowedCategoryLabels.join('\n')}
1186+
`.trim(),
1187+
});
1188+
}
1189+
1190+
const allExtendedDiagnosticNames =
1191+
ALL_DIAGNOSTIC_FACTORIES.map((factory) => factory.name) as string[];
1192+
for (const [checkName, category] of Object.entries(options.extendedDiagnostics?.checks ?? {})) {
1193+
if (!allExtendedDiagnosticNames.includes(checkName)) {
1194+
yield makeConfigDiagnostic({
1195+
category: ts.DiagnosticCategory.Error,
1196+
code: ErrorCode.CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CHECK,
1197+
messageText: `
1198+
Angular compiler option "extendedDiagnostics.checks" has an unknown check: "${checkName}".
1199+
1200+
Allowed check names are:
1201+
${allExtendedDiagnosticNames.join('\n')}
1202+
`.trim(),
1203+
});
1204+
}
1205+
1206+
if (!allowedCategoryLabels.includes(category)) {
1207+
yield makeConfigDiagnostic({
1208+
category: ts.DiagnosticCategory.Error,
1209+
code: ErrorCode.CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CATEGORY_LABEL,
1210+
messageText: `
1211+
Angular compiler option "extendedDiagnostics.checks['${
1212+
checkName}']" has an unknown diagnostic category: "${category}".
1213+
1214+
Allowed diagnostic categories are:
1215+
${allowedCategoryLabels.join('\n')}
1216+
`.trim(),
1217+
});
1218+
}
1219+
}
1220+
}
1221+
1222+
function makeConfigDiagnostic({category, code, messageText}: {
1223+
category: ts.DiagnosticCategory,
1224+
code: ErrorCode,
1225+
messageText: string,
1226+
}): ts.Diagnostic {
1227+
return {
1228+
category,
1229+
code: ngErrorCode(code),
1230+
file: undefined,
1231+
start: undefined,
1232+
length: undefined,
1233+
messageText,
1234+
};
11621235
}
11631236

11641237
class ReferenceGraphAdapter implements ReferencesRegistry {

‎packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ export enum ErrorCode {
7171

7272
CONFIG_FLAT_MODULE_NO_INDEX = 4001,
7373
CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
74+
CONFIG_EXTENDED_DIAGNOSTICS_IMPLIES_STRICT_TEMPLATES = 4003,
75+
CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CATEGORY_LABEL = 4004,
76+
CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CHECK = 4005,
7477

7578
/**
7679
* Raised when a host expression has a parse error, such as a host listener or host binding

‎packages/compiler-cli/src/ngtsc/typecheck/extended/checks/invalid_banana_in_box/BUILD.bazel‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ load("//tools:defaults.bzl", "ts_library")
33
ts_library(
44
name = "invalid_banana_in_box",
55
srcs = ["index.ts"],
6-
visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"],
6+
visibility = [
7+
"//packages/compiler-cli/src/ngtsc:__subpackages__",
8+
"//packages/compiler-cli/test/ngtsc:__pkg__",
9+
],
710
deps = [
811
"//packages/compiler",
912
"//packages/compiler-cli/src/ngtsc/diagnostics",

‎packages/compiler-cli/test/ngtsc/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ ts_library(
77
deps = [
88
"//packages/compiler",
99
"//packages/compiler-cli",
10+
"//packages/compiler-cli/src/ngtsc/core:api",
1011
"//packages/compiler-cli/src/ngtsc/diagnostics",
1112
"//packages/compiler-cli/src/ngtsc/file_system",
1213
"//packages/compiler-cli/src/ngtsc/file_system/testing",
1314
"//packages/compiler-cli/src/ngtsc/indexer",
1415
"//packages/compiler-cli/src/ngtsc/reflection",
1516
"//packages/compiler-cli/src/ngtsc/testing",
17+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/invalid_banana_in_box",
1618
"//packages/compiler-cli/src/ngtsc/util",
1719
"//packages/compiler-cli/test:test_utils",
1820
"@npm//source-map",

‎packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts‎

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88

99
import ts from 'typescript';
1010

11+
import {DiagnosticCategoryLabel} from '../../src/ngtsc/core/api';
1112
import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics';
12-
import {absoluteFrom as _, getFileSystem, getSourceFileOrError} from '../../src/ngtsc/file_system';
13+
import {absoluteFrom as _, getSourceFileOrError} from '../../src/ngtsc/file_system';
1314
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
1415
import {expectCompleteReuse, getSourceCodeForDiagnostic, loadStandardTestFiles} from '../../src/ngtsc/testing';
16+
import {factory as invalidBananaInBoxFactory} from '../../src/ngtsc/typecheck/extended/checks/invalid_banana_in_box';
1517

1618
import {NgtscTestEnvironment} from './env';
1719

@@ -2454,6 +2456,126 @@ export declare class AnimationEvent {
24542456
const diags = env.driveDiagnostics();
24552457
expect(diags.length).toBe(0);
24562458
});
2459+
2460+
it('should error if "strictTemplates" is false when "extendedDiagnostics" is configured', () => {
2461+
env.tsconfig({strictTemplates: false, extendedDiagnostics: {}});
2462+
2463+
const diags = env.driveDiagnostics();
2464+
expect(diags.length).toBe(1);
2465+
expect(diags[0].messageText)
2466+
.toContain(
2467+
'Angular compiler option "extendedDiagnostics" is configured, however "strictTemplates" is disabled.');
2468+
});
2469+
it('should not error if "strictTemplates" is true when "extendedDiagnostics" is configured',
2470+
() => {
2471+
env.tsconfig({strictTemplates: true, extendedDiagnostics: {}});
2472+
2473+
const diags = env.driveDiagnostics();
2474+
expect(diags).toEqual([]);
2475+
});
2476+
it('should not error if "strictTemplates" is false when "extendedDiagnostics" is not configured',
2477+
() => {
2478+
env.tsconfig({strictTemplates: false});
2479+
2480+
const diags = env.driveDiagnostics();
2481+
expect(diags).toEqual([]);
2482+
});
2483+
2484+
it('should error if "extendedDiagnostics.defaultCategory" is set to an unknown value', () => {
2485+
env.tsconfig({
2486+
extendedDiagnostics: {
2487+
defaultCategory: 'does-not-exist',
2488+
},
2489+
});
2490+
2491+
const diags = env.driveDiagnostics();
2492+
expect(diags.length).toBe(1);
2493+
expect(diags[0].messageText)
2494+
.toContain(
2495+
'Angular compiler option "extendedDiagnostics.defaultCategory" has an unknown diagnostic category: "does-not-exist".');
2496+
expect(diags[0].messageText).toContain(`
2497+
Allowed diagnostic categories are:
2498+
warning
2499+
error
2500+
suppress
2501+
`.trim());
2502+
});
2503+
it('should not error if "extendedDiagnostics.defaultCategory" is set to a known value',
2504+
() => {
2505+
env.tsconfig({
2506+
extendedDiagnostics: {
2507+
defaultCategory: DiagnosticCategoryLabel.Error,
2508+
},
2509+
});
2510+
2511+
const diags = env.driveDiagnostics();
2512+
expect(diags).toEqual([]);
2513+
});
2514+
2515+
it('should error if "extendedDiagnostics.checks" contains an unknown check', () => {
2516+
env.tsconfig({
2517+
extendedDiagnostics: {
2518+
checks: {
2519+
doesNotExist: DiagnosticCategoryLabel.Error,
2520+
},
2521+
},
2522+
});
2523+
2524+
const diags = env.driveDiagnostics();
2525+
expect(diags.length).toBe(1);
2526+
expect(diags[0].messageText)
2527+
.toContain(
2528+
'Angular compiler option "extendedDiagnostics.checks" has an unknown check: "doesNotExist".');
2529+
});
2530+
it('should not error if "extendedDiagnostics.checks" contains all known checks', () => {
2531+
env.tsconfig({
2532+
extendedDiagnostics: {
2533+
checks: {
2534+
[invalidBananaInBoxFactory.name]: DiagnosticCategoryLabel.Error,
2535+
},
2536+
},
2537+
});
2538+
2539+
const diags = env.driveDiagnostics();
2540+
expect(diags).toEqual([]);
2541+
});
2542+
2543+
it('should error if "extendedDiagnostics.checks" contains an unknown diagnostic category',
2544+
() => {
2545+
env.tsconfig({
2546+
extendedDiagnostics: {
2547+
checks: {
2548+
[invalidBananaInBoxFactory.name]: 'does-not-exist',
2549+
},
2550+
},
2551+
});
2552+
2553+
const diags = env.driveDiagnostics();
2554+
expect(diags.length).toBe(1);
2555+
expect(diags[0].messageText)
2556+
.toContain(`Angular compiler option "extendedDiagnostics.checks['${
2557+
invalidBananaInBoxFactory
2558+
.name}']" has an unknown diagnostic category: "does-not-exist".`);
2559+
expect(diags[0].messageText).toContain(`
2560+
Allowed diagnostic categories are:
2561+
warning
2562+
error
2563+
suppress
2564+
`.trim());
2565+
});
2566+
it('should not error if "extendedDiagnostics.checks" contains all known diagnostic categories',
2567+
() => {
2568+
env.tsconfig({
2569+
extendedDiagnostics: {
2570+
checks: {
2571+
[invalidBananaInBoxFactory.name]: DiagnosticCategoryLabel.Error,
2572+
},
2573+
},
2574+
});
2575+
2576+
const diags = env.driveDiagnostics();
2577+
expect(diags).toEqual([]);
2578+
});
24572579
});
24582580

24592581
describe('stability', () => {

0 commit comments

Comments
 (0)