diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md index a98edb2a7f8..d6052b8fe20 100644 --- a/goldens/public-api/compiler-cli/error_code.md +++ b/goldens/public-api/compiler-cli/error_code.md @@ -46,6 +46,7 @@ export enum ErrorCode { INLINE_TYPE_CTOR_REQUIRED = 8901, INVALID_BANANA_IN_BOX = 8101, MISSING_CONTROL_FLOW_DIRECTIVE = 8103, + MISSING_NGFOROF_LET = 8105, MISSING_PIPE = 8004, MISSING_REFERENCE_TARGET = 8003, NGMODULE_BOOTSTRAP_IS_STANDALONE = 6009, diff --git a/goldens/public-api/compiler-cli/extended_template_diagnostic_name.md b/goldens/public-api/compiler-cli/extended_template_diagnostic_name.md index 8241cb63a57..ee34515e73c 100644 --- a/goldens/public-api/compiler-cli/extended_template_diagnostic_name.md +++ b/goldens/public-api/compiler-cli/extended_template_diagnostic_name.md @@ -11,6 +11,8 @@ export enum ExtendedTemplateDiagnosticName { // (undocumented) MISSING_CONTROL_FLOW_DIRECTIVE = "missingControlFlowDirective", // (undocumented) + MISSING_NGFOROF_LET = "missingNgForOfLet", + // (undocumented) NULLISH_COALESCING_NOT_NULLABLE = "nullishCoalescingNotNullable", // (undocumented) TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding" diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 9b112352967..c2aefd56cfe 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -256,6 +256,17 @@ export enum ErrorCode { */ TEXT_ATTRIBUTE_NOT_BINDING = 8104, + /** + * NgForOf is used in a template, but the user forgot to include let + * in their statement. + * + * For example: + * ``` + * + * ``` + */ + MISSING_NGFOROF_LET = 8105, + /** * The template type-checking engine would need to generate an inline type check block for a * component, but the current type-checking environment doesn't support it. diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts index 8002e7a9ffb..a3d3f5630ca 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts @@ -20,4 +20,5 @@ export enum ExtendedTemplateDiagnosticName { NULLISH_COALESCING_NOT_NULLABLE = 'nullishCoalescingNotNullable', MISSING_CONTROL_FLOW_DIRECTIVE = 'missingControlFlowDirective', TEXT_ATTRIBUTE_NOT_BINDING = 'textAttributeNotBinding', + MISSING_NGFOROF_LET = 'missingNgForOfLet' } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel index 9109bdd8b6a..c9299e95732 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel @@ -14,6 +14,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck/extended/api", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/invalid_banana_in_box", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding", "@npm//typescript", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let/BUILD.bazel new file mode 100644 index 00000000000..7403a86983f --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let/BUILD.bazel @@ -0,0 +1,17 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "missing_ngforof_let", + srcs = ["index.ts"], + visibility = [ + "//packages/compiler-cli/src/ngtsc:__subpackages__", + "//packages/compiler-cli/test/ngtsc:__pkg__", + ], + deps = [ + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/diagnostics", + "//packages/compiler-cli/src/ngtsc/typecheck/api", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/api", + "@npm//typescript", + ], +) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let/index.ts new file mode 100644 index 00000000000..51d84d76d37 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let/index.ts @@ -0,0 +1,53 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {AST, TmplAstNode, TmplAstTemplate} from '@angular/compiler'; +import ts from 'typescript'; + +import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics'; +import {NgTemplateDiagnostic} from '../../../api'; +import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api'; + +/** + * Ensures a user doesn't forget to omit `let` when using ngfor. + * Will return diagnostic information when `let` is missing. + */ +class MissingNgForOfLetCheck extends TemplateCheckWithVisitor { + override code = ErrorCode.MISSING_NGFOROF_LET as const; + + override visitNode( + ctx: TemplateContext, component: ts.ClassDeclaration, + node: TmplAstNode|AST): NgTemplateDiagnostic[] { + const isTemplate = node instanceof TmplAstTemplate; + if (!(node instanceof TmplAstTemplate)) { + return []; + } + + if (node.templateAttrs.length === 0) { + return []; + } + const attr = node.templateAttrs.find(x => x.name === 'ngFor'); + if (attr === undefined) { + return []; + } + + if (node.variables.length > 0) { + return []; + } + const errorString = 'Your ngFor is missing a value. Did you forget to add the `let` keyword?'; + const diagnostic = ctx.makeTemplateDiagnostic(attr.sourceSpan, errorString); + return [diagnostic]; + } +} + +export const factory: TemplateCheckFactory< + ErrorCode.MISSING_NGFOROF_LET, ExtendedTemplateDiagnosticName.MISSING_NGFOROF_LET> = { + code: ErrorCode.MISSING_NGFOROF_LET, + name: ExtendedTemplateDiagnosticName.MISSING_NGFOROF_LET, + create: () => new MissingNgForOfLetCheck(), +}; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts index 5823b60bcfe..a260610172d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts @@ -11,6 +11,7 @@ import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../diagnostics'; import {TemplateCheckFactory} from './api'; import {factory as invalidBananaInBoxFactory} from './checks/invalid_banana_in_box'; import {factory as missingControlFlowDirectiveFactory} from './checks/missing_control_flow_directive'; +import {factory as missingNgForOfLetFactory} from './checks/missing_ngforof_let'; import {factory as nullishCoalescingNotNullableFactory} from './checks/nullish_coalescing_not_nullable'; import {factory as textAttributeNotBindingFactory} from './checks/text_attribute_not_binding'; @@ -22,4 +23,5 @@ export const ALL_DIAGNOSTIC_FACTORIES: nullishCoalescingNotNullableFactory, missingControlFlowDirectiveFactory, textAttributeNotBindingFactory, + missingNgForOfLetFactory, ]; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_ngforof_let/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_ngforof_let/BUILD.bazel new file mode 100644 index 00000000000..9011ebd5fdb --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_ngforof_let/BUILD.bazel @@ -0,0 +1,27 @@ +load("//tools:defaults.bzl", "jasmine_node_test", "ts_library") + +ts_library( + name = "test_lib", + testonly = True, + srcs = ["missing_ngforof_let_spec.ts"], + deps = [ + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/core:api", + "//packages/compiler-cli/src/ngtsc/diagnostics", + "//packages/compiler-cli/src/ngtsc/file_system", + "//packages/compiler-cli/src/ngtsc/file_system/testing", + "//packages/compiler-cli/src/ngtsc/testing", + "//packages/compiler-cli/src/ngtsc/typecheck/extended", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let", + "//packages/compiler-cli/src/ngtsc/typecheck/testing", + "@npm//typescript", + ], +) + +jasmine_node_test( + name = "test", + bootstrap = ["//tools/testing:node_no_angular_es2015"], + deps = [ + ":test_lib", + ], +) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_ngforof_let/missing_ngforof_let_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_ngforof_let/missing_ngforof_let_spec.ts new file mode 100644 index 00000000000..29c06e934f2 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_ngforof_let/missing_ngforof_let_spec.ts @@ -0,0 +1,83 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import ts from 'typescript'; + +import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics'; +import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system'; +import {runInEachFileSystem} from '../../../../../file_system/testing'; +import {getSourceCodeForDiagnostic} from '../../../../../testing'; +import {getClass, setup} from '../../../../testing'; +import {factory as missingNgForOfLet} from '../../../checks/missing_ngforof_let/index'; +import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker'; + +runInEachFileSystem(() => { + describe('TemplateChecks', () => { + it('binds the error code to its extended template diagnostic name', () => { + expect(missingNgForOfLet.code).toBe(ErrorCode.MISSING_NGFOROF_LET); + expect(missingNgForOfLet.name).toBe(ExtendedTemplateDiagnosticName.MISSING_NGFOROF_LET); + }); + + it('should produce missing ngforof let warning', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([{ + fileName, + templates: { + 'TestCmp': '
  • {{thing["name"]}}
', + }, + source: + 'export class TestCmp { items: [] = [{\'name\': \'diana\'}, {\'name\': \'prince\'}] }' + }]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, program.getTypeChecker(), [missingNgForOfLet], {} /* options */); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.MISSING_NGFOROF_LET)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe('ngFor="thing '); + }); + + it('should not produce missing ngforof let warning if written correctly', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([{ + fileName, + templates: { + 'TestCmp': '
  • {{item["name"]}};
', + }, + source: + 'export class TestCmp { items: [] = [{\'name\': \'diana\'}, {\'name\': \'prince\'}] }' + }]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, program.getTypeChecker(), [missingNgForOfLet], {} /* options */); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(0); + }); + + it('should not produce missing ngforof let warning if written correctly in longhand', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([{ + fileName, + templates: { + 'TestCmp': '{{item["name"]}}', + }, + source: + 'export class TestCmp { items: [] = [{\'name\': \'diana\'}, {\'name\': \'prince\'}] }' + }]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, program.getTypeChecker(), [missingNgForOfLet], {} /* options */); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(0); + }); + }); +});