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

…ings

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

Fixes #36256

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

@@ -66,6 +66,7 @@ export enum ErrorCode {
     SCHEMA_INVALID_ATTRIBUTE = 8002,
     SCHEMA_INVALID_ELEMENT = 8001,
     SPLIT_TWO_WAY_BINDING = 8007,
+    SUFFIX_NOT_SUPPORTED = 8105,
     SUGGEST_STRICT_TEMPLATES = 10001,
     SUGGEST_SUBOPTIMAL_TYPE_INFERENCE = 10002,
     // (undocumented)

goldens/public-api/compiler-cli/extended_template_diagnostic_name.md

@@ -13,6 +13,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

@@ -256,6 +256,18 @@ export enum ErrorCode {
    */
   TEXT_ATTRIBUTE_NOT_BINDING = 8104,
 
+  /**
+   * 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 = 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.

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',
+  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/nullish_coalescing_not_nullable",
         "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding",
+        "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported",
         "@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,45 @@
+/**
+ * @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';
+
+/**
+ * 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.name.startsWith('attr.') ||
+        (!node.name.endsWith('.px') && !node.name.endsWith('.%') && !node.name.endsWith('.em'))) {
+      return [];
+    }
+
+    const diagnostic = ctx.makeTemplateDiagnostic(
+        node.keySpan,
+        'The `.px`, `.%`, `.em`, etc. 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 nullishCoalescingNotNullableFactory} from './checks/nullish_coalescing_not_nullable';
 import {factory as textAttributeNotBindingFactory} from './checks/text_attribute_not_binding';
+import {factory as suffixNotSupportedFactory} from './checks/suffix_not_supported';
 
 export {ExtendedTemplateCheckerImpl} from './src/extended_template_checker';
 
@@ -22,4 +23,5 @@ export const ALL_DIAGNOSTIC_FACTORIES:
       nullishCoalescingNotNullableFactory,
       missingControlFlowDirectiveFactory,
       textAttributeNotBindingFactory,
+      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);
+    });
+  });
+});