feat(eslint-plugin-template): [no-duplicate-attributes] add allowStyl…

…ePrecedenceDuplicates option (#1407) Co-authored-by: Marie Briand <mbriand@lucca.fr> Co-authored-by: James Henry <james@henry.sc>
angular-eslint · Jul 12, 2023 · 6f69af8 · 6f69af8
1 parent 90c0e91
commit 6f69af8
Show file tree

Hide file tree

Showing 4 changed files with 272 additions and 9 deletions.
diff --git a/package.json b/package.json
@@ -17,12 +17,12 @@
     "url": "https://github.com/angular-eslint/angular-eslint/issues"
   },
   "scripts": {
-    "build": "nx run-many --target=build --all --parallel",
-    "test": "nx run-many --target=test --all --parallel",
-    "integration-tests": "nx clean integration-tests && nx spawn-and-populate-local-registry integration-tests && nx run-many --target=integration-test --all",
+    "build": "nx run-many -t build",
+    "test": "nx run-many -t test",
+    "integration-tests": "nx clean integration-tests && nx spawn-and-populate-local-registry integration-tests && nx run-many -t integration-test",
     "update-integration-tests": "yarn integration-tests --updateSnapshots",
     "check-clean-workspace-after-install": "git diff --quiet --exit-code",
-    "clean": "nx reset && lerna clean && nx run-many --target=clean --all --parallel",
+    "clean": "nx reset && lerna clean && nx run-many -t clean",
     "cz": "git-cz",
     "postinstall": "tools/scripts/postinstall.sh",
     "check-clean-integration-test-fixtures": "tools/scripts/check-clean-integration-test-fixtures.sh",
@@ -31,11 +31,11 @@
     "format": "prettier --write \"./**/*.{ts,js,json,md}\"",
     "format-check": "prettier --check \"./**/*.{ts,js,json,md}\"",
     "lint": "eslint . --ext .js,.ts",
-    "typecheck": "nx run-many --target=typecheck --all --parallel",
+    "typecheck": "nx run-many -t typecheck",
     "exec-tool": "ts-node --transpile-only --project tsconfig.tools.json tools/scripts/exec-tool.ts",
     "update-configs": "ts-node --transpile-only --project tsconfig.tools.json tools/scripts/generate-configs.ts",
-    "check-rule-docs": "nx run-many --target=check-rule-docs --all --parallel",
-    "update-rule-docs": "nx run-many --target=update-rule-docs --all --parallel",
+    "check-rule-docs": "nx run-many -t check-rule-docs",
+    "update-rule-docs": "nx run-many -t update-rule-docs",
     "prepare": "husky install"
   },
   "config": {

diff --git a/packages/eslint-plugin-template/docs/rules/no-duplicate-attributes.md b/packages/eslint-plugin-template/docs/rules/no-duplicate-attributes.md
@@ -35,6 +35,10 @@ interface Options {
    * Default: `true`
    */
   allowTwoWayDataBinding?: boolean;
+  /**
+   * Whether or not Angular style precedence is allowed as an exception to the rule. See https://angular.io/guide/style-precedence#style-precedence
+   */
+  allowStylePrecedenceDuplicates?: boolean;
   /**
    * Input or output properties for which duplicate presence is allowed as an exception to the rule.
    *
@@ -384,6 +388,66 @@ interface Options {
        ~~~~~~~~~~~~                    ~~~~~~~~~~
 ```
 
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/no-duplicate-attributes": [
+      "error",
+      {
+        "allowStylePrecedenceDuplicates": true
+      }
+    ]
+  }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<input class="foo" class="bar" [class]="dynamic">
+       ~~~~~~~~~~~ ~~~~~~~~~~~
+```
+
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/no-duplicate-attributes": [
+      "error",
+      {
+        "allowStylePrecedenceDuplicates": true
+      }
+    ]
+  }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<input style="color: blue" [style]="styleExpression" style="width:50px">
+       ~~~~~~~~~~~~~~~~~~~                           ~~~~~~~~~~~~~~~~~~
+```
+
 </details>
 
 <br>
@@ -677,6 +741,64 @@ interface Options {
 <div [style.width]="col.width + 'px'" [width]="col.width"></div>
 ```
 
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/no-duplicate-attributes": [
+      "error",
+      {
+        "allowStylePrecedenceDuplicates": true
+      }
+    ]
+  }
+}
+```
+
+<br>
+
+#### ✅ Valid Code
+
+```html
+<div class="foo" name="bar" [class]="dynamic"></div>
+```
+
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/no-duplicate-attributes": [
+      "error",
+      {
+        "allowStylePrecedenceDuplicates": true
+      }
+    ]
+  }
+}
+```
+
+<br>
+
+#### ✅ Valid Code
+
+```html
+<div style="color: blue" [style]="styleExpression"></div>
+```
+
 </details>
 
 <br>
diff --git a/packages/eslint-plugin-template/src/rules/no-duplicate-attributes.ts b/packages/eslint-plugin-template/src/rules/no-duplicate-attributes.ts
@@ -11,13 +11,15 @@ import { getOriginalAttributeName } from '../utils/get-original-attribute-name';
 type Options = [
   {
     readonly allowTwoWayDataBinding?: boolean;
+    readonly allowStylePrecedenceDuplicates?: boolean;
     readonly ignore?: readonly string[];
   },
 ];
 export type MessageIds = 'noDuplicateAttributes' | 'suggestRemoveAttribute';
 export const RULE_NAME = 'no-duplicate-attributes';
 const DEFAULT_OPTIONS: Options[number] = {
   allowTwoWayDataBinding: true,
+  allowStylePrecedenceDuplicates: false,
   ignore: [],
 };
 
@@ -40,6 +42,11 @@ export default createESLintRule<Options, MessageIds>({
             default: DEFAULT_OPTIONS.allowTwoWayDataBinding,
             description: `Whether or not two-way data binding is allowed as an exception to the rule.`,
           },
+          allowStylePrecedenceDuplicates: {
+            type: 'boolean',
+            default: DEFAULT_OPTIONS.allowStylePrecedenceDuplicates,
+            description: `Whether or not Angular style precedence is allowed as an exception to the rule. See https://angular.io/guide/style-precedence#style-precedence`,
+          },
           ignore: {
             type: 'array',
             items: { type: 'string' },
@@ -57,15 +64,57 @@ export default createESLintRule<Options, MessageIds>({
     },
   },
   defaultOptions: [DEFAULT_OPTIONS],
-  create(context, [{ allowTwoWayDataBinding, ignore }]) {
+  create(
+    context,
+    [{ allowTwoWayDataBinding, allowStylePrecedenceDuplicates, ignore }],
+  ) {
     const parserServices = getTemplateParserServices(context);
 
     return {
       Element$1({ inputs, outputs, attributes }: TmplAstElement) {
-        const duplicateInputsAndAttributes = findDuplicates([
+        // According to the Angular documentation (https://angular.io/guide/style-precedence#style-precedence)
+        // Angular merges both attributes which means their combined use can be seen as valid
+        const angularStylePrecedenceDuplicatesAllowed = ['class', 'style'];
+
+        let duplicateInputsAndAttributes = findDuplicates([
           ...inputs,
           ...attributes,
         ]);
+
+        if (allowStylePrecedenceDuplicates) {
+          const inputsIgnored = inputs.filter((input) =>
+            angularStylePrecedenceDuplicatesAllowed.includes(
+              getOriginalAttributeName(input),
+            ),
+          );
+
+          if (inputsIgnored?.length > 0) {
+            const attributesIgnored = attributes.filter((attr) =>
+              angularStylePrecedenceDuplicatesAllowed.includes(
+                getOriginalAttributeName(attr),
+              ),
+            );
+            const inputsNotIgnored = inputs.filter(
+              (input) => !inputsIgnored.includes(input),
+            );
+            const attributesNotIgnored = attributes.filter(
+              (attr) => !attributesIgnored.includes(attr),
+            );
+            const ignoreDuplicated = [
+              ...findDuplicates(inputsIgnored),
+              ...findDuplicates(attributesIgnored),
+            ];
+            const notIgnoredDuplicates = [
+              ...findDuplicates(inputsNotIgnored),
+              ...findDuplicates(attributesNotIgnored),
+            ];
+            duplicateInputsAndAttributes = [
+              ...ignoreDuplicated,
+              ...notIgnoredDuplicates,
+            ];
+          }
+        }
+
         const filteredOutputs = allowTwoWayDataBinding
           ? outputs.filter((output) => {
               return !inputs.some(

diff --git a/packages/eslint-plugin-template/tests/rules/no-duplicate-attributes/cases.ts b/packages/eslint-plugin-template/tests/rules/no-duplicate-attributes/cases.ts
@@ -16,6 +16,18 @@ export const valid = [
   '<button [class.disabled]="!enabled" [disabled]="!enabled"></button>',
   '<button [@.disabled]="!enabled" [.disabled]="!enabled"></button>',
   '<div [style.width]="col.width + \'px\'" [width]="col.width"></div>',
+  {
+    code: `
+      <div class="foo" name="bar" [class]="dynamic"></div>
+    `,
+    options: [{ allowStylePrecedenceDuplicates: true }],
+  },
+  {
+    code: `
+      <div style="color: blue" [style]="styleExpression"></div>
+    `,
+    options: [{ allowStylePrecedenceDuplicates: true }],
+  },
 ];
 
 export const invalid = [
@@ -536,4 +548,84 @@ export const invalid = [
       },
     ],
   }),
+  convertAnnotatedSourceToFailureCase({
+    description: 'should report duplicate static binding for class attribute',
+    annotatedSource: `
+        <input class="foo" class="bar" [class]="dynamic">
+               ~~~~~~~~~~~ ^^^^^^^^^^^     
+      `,
+    options: [{ allowStylePrecedenceDuplicates: true }],
+    messages: [
+      {
+        char: '~',
+        messageId,
+        data: { attributeName: 'class' },
+        suggestions: [
+          {
+            messageId: suggestRemoveAttribute,
+            output: `
+        <input class="bar" [class]="dynamic">
+                                
+      `,
+            data: { attributeName: 'class' },
+          },
+        ],
+      },
+      {
+        char: '^',
+        messageId,
+        data: { attributeName: 'class' },
+        suggestions: [
+          {
+            messageId: suggestRemoveAttribute,
+            output: `
+        <input class="foo" [class]="dynamic">
+                                
+      `,
+            data: { attributeName: 'class' },
+          },
+        ],
+      },
+    ],
+  }),
+  convertAnnotatedSourceToFailureCase({
+    description: 'should report duplicate static binding for style attribute',
+    annotatedSource: `
+        <input style="color: blue" [style]="styleExpression" style="width:50px">
+               ~~~~~~~~~~~~~~~~~~~                           ^^^^^^^^^^^^^^^^^^
+      `,
+    options: [{ allowStylePrecedenceDuplicates: true }],
+    messages: [
+      {
+        char: '~',
+        messageId,
+        data: { attributeName: 'style' },
+        suggestions: [
+          {
+            messageId: suggestRemoveAttribute,
+            output: `
+        <input [style]="styleExpression" style="width:50px">
+                                                             
+      `,
+            data: { attributeName: 'style' },
+          },
+        ],
+      },
+      {
+        char: '^',
+        messageId,
+        data: { attributeName: 'style' },
+        suggestions: [
+          {
+            messageId: suggestRemoveAttribute,
+            output: `
+        <input style="color: blue" [style]="styleExpression">
+                                                             
+      `,
+            data: { attributeName: 'style' },
+          },
+        ],
+      },
+    ],
+  }),
 ];