feat(eslint-plugin-template): add no duplicate attributes rule

Closes angular-eslint#293
mattlewis92 · Jan 28, 2021 · acdc80a · acdc80a
1 parent 926c402
commit acdc80a
Show file tree

Hide file tree

Showing 4 changed files with 294 additions and 0 deletions.
diff --git a/packages/eslint-plugin-template/src/configs/all.json b/packages/eslint-plugin-template/src/configs/all.json
@@ -16,6 +16,7 @@
     "@angular-eslint/template/no-autofocus": "error",
     "@angular-eslint/template/no-call-expression": "error",
     "@angular-eslint/template/no-distracting-elements": "error",
+    "@angular-eslint/template/no-duplicate-attributes": "error",
     "@angular-eslint/template/no-negated-async": "error",
     "@angular-eslint/template/no-positive-tabindex": "error",
     "@angular-eslint/template/use-track-by-function": "error"

diff --git a/packages/eslint-plugin-template/src/index.ts b/packages/eslint-plugin-template/src/index.ts
@@ -44,6 +44,9 @@ import noCallExpression, {
 import noDistractingElements, {
   RULE_NAME as noDistractingElementsRuleName,
 } from './rules/no-distracting-elements';
+import noDuplicateAttributes, {
+  RULE_NAME as noDuplicateAttributesRuleName,
+} from './rules/no-duplicate-attributes';
 import noNegatedAsync, {
   RULE_NAME as noNegatedAsyncRuleName,
 } from './rules/no-negated-async';
@@ -78,6 +81,7 @@ export default {
     [noAutofocusRuleName]: noAutofocus,
     [noCallExpressionRuleName]: noCallExpression,
     [noDistractingElementsRuleName]: noDistractingElements,
+    [noDuplicateAttributesRuleName]: noDuplicateAttributes,
     [noNegatedAsyncRuleName]: noNegatedAsync,
     [noPositiveTabindexRuleName]: noPositiveTabindex,
     [useTrackByFunctionRuleName]: useTrackByFunction,

diff --git a/packages/eslint-plugin-template/src/rules/no-duplicate-attributes.ts b/packages/eslint-plugin-template/src/rules/no-duplicate-attributes.ts
@@ -0,0 +1,138 @@
+import {
+  BindingType,
+  TmplAstBoundAttribute,
+  TmplAstBoundEvent,
+  TmplAstElement,
+  TmplAstTextAttribute,
+  ParsedEventType,
+} from '@angular/compiler';
+import {
+  createESLintRule,
+  getTemplateParserServices,
+} from '../utils/create-eslint-rule';
+
+type Options = [];
+export type MessageIds = 'noDuplicateAttributes';
+export const RULE_NAME = 'no-duplicate-attributes';
+
+export default createESLintRule<Options, MessageIds>({
+  name: RULE_NAME,
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'Ensures that there are no duplicate input properties or output event listeners',
+      category: 'Possible Errors',
+      recommended: false,
+    },
+    schema: [],
+    messages: {
+      noDuplicateAttributes: 'Duplicate attribute "{{attributeName}}"',
+    },
+  },
+  defaultOptions: [],
+  create(context) {
+    const parserServices = getTemplateParserServices(context);
+
+    return {
+      Element({ inputs, outputs, attributes }: TmplAstElement) {
+        const duplicateInputsAndAttributes = findDuplicates([
+          ...inputs,
+          ...attributes,
+        ]);
+
+        const nonTwoWayBindingOutputs = outputs.filter((output) => {
+          return !inputs.some(
+            (input) =>
+              input.sourceSpan.start === output.sourceSpan.start &&
+              input.sourceSpan.end === output.sourceSpan.end,
+          );
+        });
+
+        const duplicateOutputs = findDuplicates(nonTwoWayBindingOutputs);
+
+        const allDuplicates = [
+          ...duplicateInputsAndAttributes,
+          ...duplicateOutputs,
+        ];
+
+        allDuplicates.forEach((duplicate) => {
+          const loc = parserServices.convertNodeSourceSpanToLoc(
+            duplicate.sourceSpan,
+          );
+
+          context.report({
+            messageId: 'noDuplicateAttributes',
+            loc,
+            data: {
+              attributeName: getAttributeName(duplicate),
+            },
+          });
+        });
+      },
+    };
+  },
+});
+
+interface BoundAttribute extends Omit<TmplAstBoundAttribute, 'type'> {
+  type: 'BoundAttribute';
+  __originalType: BindingType;
+}
+
+interface BoundEvent extends Omit<TmplAstBoundEvent, 'type'> {
+  type: 'BoundEvent';
+  __originalType: ParsedEventType;
+}
+
+function getAttributeName(
+  attribute:
+    | BoundAttribute
+    | TmplAstTextAttribute
+    | BoundEvent
+    | { name: string },
+): string {
+  if ('type' in attribute) {
+    if (attribute.type === 'BoundAttribute') {
+      switch (attribute.__originalType) {
+        case BindingType.Class:
+          return `class.${attribute.name}`;
+        case BindingType.Style:
+          return `style.${attribute.name}${
+            attribute.unit ? '.' + attribute.unit : ''
+          }`;
+        case BindingType.Animation:
+          return `@${attribute.name}`;
+      }
+    } else if (attribute.type === 'BoundEvent') {
+      if (attribute.__originalType === ParsedEventType.Animation) {
+        return `@${attribute.name}${
+          attribute.phase ? '.' + attribute.phase : ''
+        }`;
+      }
+
+      if (attribute.target) {
+        return `${attribute.target}:${attribute.name}`;
+      }
+    }
+  }
+
+  return attribute.name;
+}
+
+function findDuplicates(
+  elements: Array<TmplAstBoundEvent>,
+): Array<TmplAstBoundEvent>;
+function findDuplicates(
+  elements: Array<TmplAstBoundAttribute | TmplAstTextAttribute>,
+): Array<TmplAstBoundAttribute | TmplAstTextAttribute>;
+function findDuplicates(
+  elements: Array<{ name: string }>,
+): Array<{ name: string }> {
+  return elements.filter((element) => {
+    return elements.some(
+      (otherElement) =>
+        otherElement !== element &&
+        getAttributeName(otherElement) === getAttributeName(element),
+    );
+  });
+}
diff --git a/packages/eslint-plugin-template/tests/rules/no-duplicate-attributes.test.ts b/packages/eslint-plugin-template/tests/rules/no-duplicate-attributes.test.ts
@@ -0,0 +1,151 @@
+import {
+  convertAnnotatedSourceToFailureCase,
+  RuleTester,
+} from '@angular-eslint/utils';
+import rule, {
+  MessageIds,
+  RULE_NAME,
+} from '../../src/rules/no-duplicate-attributes';
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parser: '@angular-eslint/template-parser',
+});
+
+const messageId: MessageIds = 'noDuplicateAttributes';
+
+ruleTester.run(RULE_NAME, rule, {
+  valid: [
+    '<input name="foo">',
+    '<input [name]="foo">',
+    '<input (change)="bar()">',
+    '<input [(ngModel)]="foo">',
+    '<input [(ngModel)]="model" (ngModelChange)="modelChanged()">',
+    '<div (@fade.start)="animationStarted($event)" (@fade.done)="animationDone($event)"></div>',
+    '<div (window:keydown)="windowKeydown($event)" (document:keydown)="documentKeydown($event)" (document:keyup)="documentKeyup($event)" (keyup)="keyup($event)" (keydown)="keydown($event)"></div>',
+    '<div [style.width.px]="col.width" [width]="col.width"></div>',
+    '<button [class.disabled]="!enabled" [disabled]="!enabled"></button>',
+    '<button [@.disabled]="!enabled" [.disabled]="!enabled"></button>',
+    '<div [style.width]="col.width + \'px\'" [width]="col.width"></div>',
+  ],
+  invalid: [
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail with 2 inputs with the same name',
+      annotatedSource: `
+        <input [name]="foo" [name]="bar">
+               ~~~~~~~~~~~~ ^^^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description:
+        'should fail with an input and a text attribute with the same name',
+      annotatedSource: `
+        <input [name]="foo" name="bar">
+               ~~~~~~~~~~~~ ^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail with 2 text attributes with the same name',
+      annotatedSource: `
+        <input name="foo" name="bar">
+               ~~~~~~~~~~ ^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail with 2 outputs with the same name',
+      annotatedSource: `
+        <input (change)="foo($event)" (change)="bar($event)">
+               ~~~~~~~~~~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail with 2 banana in a box with the same name',
+      annotatedSource: `
+        <input [(ngModel)]="model" [(ngModel)]="otherModel">
+               ~~~~~~~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description:
+        'should fail with duplicate attributes but allow non duplicates',
+      annotatedSource: `
+        <input [name]="foo" [other]="bam" [name]="bar">
+               ~~~~~~~~~~~~               ^^^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail with 3 duplications',
+      annotatedSource: `
+        <input [name]="foo" [name]="bar" [name]="bam">
+               ~~~~~~~~~~~~ ^^^^^^^^^^^^ ############
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+        { char: '#', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail multiple combinations of duplicates',
+      annotatedSource: `
+        <input [(ngModel)]="model" [name]="foo" [(ngModel)]="otherModel" name="bar">
+               ~~~~~~~~~~~~~~~~~~~ ^^^^^^^^^^^^ ######################## %%%%%%%%%%
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+        { char: '#', messageId },
+        { char: '%', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail with multiple animation outputs',
+      annotatedSource: `
+        <input (@fade.start)="animationStarted($event)" (@fade.start)="animationStarted($event)">
+               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+    convertAnnotatedSourceToFailureCase({
+      description: 'should fail with multiple outputs on the window',
+      annotatedSource: `
+        <input (window:resize)="windowResized($event)" (resize)="resize()" (window:resize)="windowResized2($event)">
+               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+      `,
+      messages: [
+        { char: '~', messageId },
+        { char: '^', messageId },
+      ],
+    }),
+  ],
+});