feat(compiler): warn when style suffixes are used with attribute bind…

…ings (#46651)

This commit adds an extended diagnostic which warns when style suffixes such as '.px'
are used with attribute bindings (attr.width.px).

Fixes #36256

PR Close #46651

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)

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"
 }
 

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

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',
 }

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",
     ],

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",
+    ],
+)

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<ErrorCode.SUFFIX_NOT_SUPPORTED> {
+  override code = ErrorCode.SUFFIX_NOT_SUPPORTED as const;
+
+  override visitNode(
+      ctx: TemplateContext<ErrorCode.SUFFIX_NOT_SUPPORTED>, component: ts.ClassDeclaration,
+      node: TmplAstNode|AST): NgTemplateDiagnostic<ErrorCode.SUFFIX_NOT_SUPPORTED>[] {
+    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(),
+};

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,
     ];

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",
+    ],
+)

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': `<div [attr.width.px]="1"></div>`,
+        },
+        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': `<div [style.width.px]="1"></div>`,
+        },
+        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': `<div [myInput.px]="1"></div>`,
+          },
+          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);
+    });
+  });
+});