diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md index d6052b8fe2011..ab01305fc8369 100644 --- a/goldens/public-api/compiler-cli/error_code.md +++ b/goldens/public-api/compiler-cli/error_code.md @@ -67,6 +67,7 @@ export enum ErrorCode { SCHEMA_INVALID_ATTRIBUTE = 8002, SCHEMA_INVALID_ELEMENT = 8001, SPLIT_TWO_WAY_BINDING = 8007, + SUFFIX_NOT_SUPPORTED = 8106, SUGGEST_STRICT_TEMPLATES = 10001, SUGGEST_SUBOPTIMAL_TYPE_INFERENCE = 10002, // (undocumented) 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 ee34515e73c17..7977b0070d1ea 100644 --- a/goldens/public-api/compiler-cli/extended_template_diagnostic_name.md +++ b/goldens/public-api/compiler-cli/extended_template_diagnostic_name.md @@ -15,6 +15,8 @@ export enum ExtendedTemplateDiagnosticName { // (undocumented) NULLISH_COALESCING_NOT_NULLABLE = "nullishCoalescingNotNullable", // (undocumented) + SUFFIX_NOT_SUPPORTED = "suffixNotSupported", + // (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 c2aefd56cfe20..57a03ac4555ba 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -266,6 +266,17 @@ export enum ErrorCode { * ``` */ MISSING_NGFOROF_LET = 8105, + /** + * Indicates that the binding suffix is not supported + * + * Style bindings support suffixes like `stlye.width.px`, `.em`, and `.%`. + * These suffixes are _not_ supported for attribute bindings. + * + * For example `[attr.width.px]="5"` becomes `width.px="5"` when bound. + * This is almost certainly unintentional and this error is meant to + * surface this mistake to the developer. + */ + SUFFIX_NOT_SUPPORTED = 8106, /** * The template type-checking engine would need to generate an inline type check block for a 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 a3d3f5630cad9..451e4d9c75568 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,5 +20,6 @@ export enum ExtendedTemplateDiagnosticName { NULLISH_COALESCING_NOT_NULLABLE = 'nullishCoalescingNotNullable', MISSING_CONTROL_FLOW_DIRECTIVE = 'missingControlFlowDirective', TEXT_ATTRIBUTE_NOT_BINDING = 'textAttributeNotBinding', - MISSING_NGFOROF_LET = 'missingNgForOfLet' + MISSING_NGFOROF_LET = 'missingNgForOfLet', + SUFFIX_NOT_SUPPORTED = 'suffixNotSupported', } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel index c9299e9573295..906b601e2265b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel @@ -16,6 +16,7 @@ ts_library( "//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/suffix_not_supported", "//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/suffix_not_supported/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported/BUILD.bazel new file mode 100644 index 0000000000000..157e446f08802 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported/BUILD.bazel @@ -0,0 +1,15 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "suffix_not_supported", + srcs = ["index.ts"], + visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"], + deps = [ + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/core:api", + "//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/suffix_not_supported/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported/index.ts new file mode 100644 index 0000000000000..a3a323dd6fbf8 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported/index.ts @@ -0,0 +1,49 @@ +/** + * @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, TmplAstBoundAttribute, TmplAstNode} from '@angular/compiler'; +import ts from 'typescript'; + +import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics'; +import {NgTemplateDiagnostic} from '../../../api'; +import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api'; + +const STYLE_SUFFIXES = ['px', '%', 'em']; + +/** + * A check which detects when the `.px`, `.%`, and `.em` suffixes are used with an attribute + * binding. These suffixes are only available for style bindings. + */ +class SuffixNotSupportedCheck extends TemplateCheckWithVisitor { + override code = ErrorCode.SUFFIX_NOT_SUPPORTED as const; + + override visitNode( + ctx: TemplateContext, component: ts.ClassDeclaration, + node: TmplAstNode|AST): NgTemplateDiagnostic[] { + if (!(node instanceof TmplAstBoundAttribute)) return []; + + if (!node.keySpan.toString().startsWith('attr.') || + !STYLE_SUFFIXES.some(suffix => node.name.endsWith(`.${suffix}`))) { + return []; + } + + const diagnostic = ctx.makeTemplateDiagnostic( + node.keySpan, + `The ${ + STYLE_SUFFIXES.map(suffix => `'.${suffix}'`) + .join(', ')} suffixes are only supported on style bindings.`); + return [diagnostic]; + } +} + +export const factory: TemplateCheckFactory< + ErrorCode.SUFFIX_NOT_SUPPORTED, ExtendedTemplateDiagnosticName.SUFFIX_NOT_SUPPORTED> = { + code: ErrorCode.SUFFIX_NOT_SUPPORTED, + name: ExtendedTemplateDiagnosticName.SUFFIX_NOT_SUPPORTED, + create: () => new SuffixNotSupportedCheck(), +}; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts index a260610172d22..92ba21781fa5c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts @@ -13,6 +13,7 @@ import {factory as invalidBananaInBoxFactory} from './checks/invalid_banana_in_b 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 suffixNotSupportedFactory} from './checks/suffix_not_supported'; import {factory as textAttributeNotBindingFactory} from './checks/text_attribute_not_binding'; export {ExtendedTemplateCheckerImpl} from './src/extended_template_checker'; @@ -24,4 +25,5 @@ export const ALL_DIAGNOSTIC_FACTORIES: missingControlFlowDirectiveFactory, textAttributeNotBindingFactory, missingNgForOfLetFactory, + suffixNotSupportedFactory, ]; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/suffix_not_supported/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/suffix_not_supported/BUILD.bazel new file mode 100644 index 0000000000000..917dc7aeb3ab0 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/suffix_not_supported/BUILD.bazel @@ -0,0 +1,27 @@ +load("//tools:defaults.bzl", "jasmine_node_test", "ts_library") + +ts_library( + name = "test_lib", + testonly = True, + srcs = ["suffix_not_supported_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/suffix_not_supported", + "//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/suffix_not_supported/suffix_not_supported_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/suffix_not_supported/suffix_not_supported_spec.ts new file mode 100644 index 0000000000000..27dc183add612 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/suffix_not_supported/suffix_not_supported_spec.ts @@ -0,0 +1,84 @@ +/** + * @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 {DiagnosticCategoryLabel} from '@angular/compiler-cli/src/ngtsc/core/api'; +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 suffixNotSupportedFactory} from '../../../checks/suffix_not_supported'; +import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker'; + +runInEachFileSystem(() => { + describe('SuffixNotSupportedCheck', () => { + it('binds the error code to its extended template diagnostic name', () => { + expect(suffixNotSupportedFactory.code).toBe(ErrorCode.SUFFIX_NOT_SUPPORTED); + expect(suffixNotSupportedFactory.name) + .toBe(ExtendedTemplateDiagnosticName.SUFFIX_NOT_SUPPORTED); + }); + + it('should produce suffix not supported warning', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([{ + fileName, + templates: { + 'TestCmp': `
`, + }, + source: 'export class TestCmp {}' + }]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, program.getTypeChecker(), [suffixNotSupportedFactory], {}); + 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.SUFFIX_NOT_SUPPORTED)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe(`attr.width.px`); + }); + + it('should not produce suffix not supported warning on a style binding', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([{ + fileName, + templates: { + 'TestCmp': `
`, + }, + source: 'export class TestCmp {}' + }]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, program.getTypeChecker(), [suffixNotSupportedFactory], {}); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(0); + }); + + it('should not produce suffix not supported warning on an input', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + source: 'export class TestCmp {}', + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, program.getTypeChecker(), [suffixNotSupportedFactory], {}); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(0); + }); + }); +});