From 3004e2ad3a267eeae79d91ee487cfd33efbf9fb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Exbrayat?= Date: Fri, 15 Jul 2022 09:37:52 +0200 Subject: [PATCH] fix(compiler-cli): improve the missingControlFlowDirective message The extended diagnostics about missing control flow directive was only mentioning that the `CommonModule` should be imported. Now that the control flow directives are available as standalone, the message mentions that directive itself can be imported. The message now also mentions which import should be used for the directive (as it can be tricky to figure out that `NgForOf` is the directive corresponding to `*ngFor`). --- .../missing_control_flow_directive/index.ts | 19 ++++++++++++++----- .../missing_control_flow_directive_spec.ts | 14 ++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive/index.ts index 8bd0088523c08..47230bbb3caa3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive/index.ts @@ -15,14 +15,18 @@ import {NgTemplateDiagnostic} from '../../../api'; import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api'; /** - * The list of known control flow directives present in the `CommonModule`. + * The list of known control flow directives present in the `CommonModule`, + * and their corresponding imports. * * Note: there is no `ngSwitch` here since it's typically used as a regular * binding (e.g. `[ngSwitch]`), however the `ngSwitchCase` and `ngSwitchDefault` * are used as structural directives and a warning would be generated. Once the * `CommonModule` is included, the `ngSwitch` would also be covered. */ -const KNOWN_CONTROL_FLOW_DIRECTIVES = new Set(['ngIf', 'ngFor', 'ngSwitchCase', 'ngSwitchDefault']); +export const KNOWN_CONTROL_FLOW_DIRECTIVES = new Map([ + ['ngIf', 'NgIf'], ['ngFor', 'NgForOf'], ['ngSwitchCase', 'NgSwitchCase'], + ['ngSwitchDefault', 'NgSwitchDefault'] +]); /** * Ensures that there are no known control flow directives (such as *ngIf and *ngFor) @@ -64,9 +68,14 @@ class MissingControlFlowDirectiveCheck extends } const sourceSpan = controlFlowAttr.keySpan || controlFlowAttr.sourceSpan; - const errorMessage = `The \`*${controlFlowAttr.name}\` directive was used in the template, ` + - `but the \`CommonModule\` was not imported. Please make sure that the \`CommonModule\` ` + - `is included into the \`@Component.imports\` array of this component.`; + const correspondingImport = KNOWN_CONTROL_FLOW_DIRECTIVES.get(controlFlowAttr.name); + const errorMessage = + `The \`*${controlFlowAttr.name}\` directive was used in the template, ` + + `but neither the \`${ + correspondingImport}\` directive nor the \`CommonModule\` was imported. ` + + `Please make sure that either the \`${ + correspondingImport}\` directive or the \`CommonModule\` ` + + `is included in the \`@Component.imports\` array of this component.`; const diagnostic = ctx.makeTemplateDiagnostic(sourceSpan, errorMessage); return [diagnostic]; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_control_flow_directive/missing_control_flow_directive_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_control_flow_directive/missing_control_flow_directive_spec.ts index c8fe7e4c406d6..8f2f31e89ec96 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_control_flow_directive/missing_control_flow_directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_control_flow_directive/missing_control_flow_directive_spec.ts @@ -13,14 +13,12 @@ import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system'; import {runInEachFileSystem} from '../../../../../file_system/testing'; import {getSourceCodeForDiagnostic} from '../../../../../testing'; import {getClass, setup} from '../../../../testing'; -import {factory as missingControlFlowDirectiveCheck} from '../../../checks/missing_control_flow_directive'; +import {factory as missingControlFlowDirectiveCheck, KNOWN_CONTROL_FLOW_DIRECTIVES} from '../../../checks/missing_control_flow_directive'; import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker'; -const KNOWN_CONTROL_FLOW_DIRECTIVES = ['ngIf', 'ngFor', 'ngSwitchCase', 'ngSwitchDefault']; - runInEachFileSystem(() => { describe('MissingControlFlowDirectiveCheck', () => { - KNOWN_CONTROL_FLOW_DIRECTIVES.forEach(directive => { + KNOWN_CONTROL_FLOW_DIRECTIVES.forEach((correspondingImport, directive) => { ['div', 'ng-template', 'ng-container', 'ng-content'].forEach(element => { it(`should produce a warning when the '${directive}' directive is not imported ` + `(when used on the '${element}' element)`, @@ -47,6 +45,14 @@ runInEachFileSystem(() => { expect(diags.length).toBe(1); expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); expect(diags[0].code).toBe(ngErrorCode(ErrorCode.MISSING_CONTROL_FLOW_DIRECTIVE)); + expect(diags[0].messageText) + .toBe( + `The \`*${directive}\` directive was used in the template, ` + + `but neither the \`${ + correspondingImport}\` directive nor the \`CommonModule\` was imported. ` + + `Please make sure that either the \`${ + correspondingImport}\` directive or the \`CommonModule\` ` + + `is included in the \`@Component.imports\` array of this component.`); expect(getSourceCodeForDiagnostic(diags[0])).toBe(directive); });