feat(parser): propagate parse errors from angular compiler (#969)

angular-eslint · Apr 3, 2022 · ab9b496 · ab9b496
1 parent 76c6cac
commit ab9b496
Show file tree

Hide file tree

Showing 7 changed files with 149 additions and 12 deletions.
diff --git a/packages/eslint-plugin-template/docs/rules/accessibility-alt-text.md b/packages/eslint-plugin-template/docs/rules/accessibility-alt-text.md
@@ -409,7 +409,7 @@ The rule does not have any configuration options.
 #### ✅ Valid Code
 
 ```html
-<area aria-label="foo"></area>
+<area aria-label="foo" />
 ```
 
 <br>
@@ -435,7 +435,7 @@ The rule does not have any configuration options.
 #### ✅ Valid Code
 
 ```html
-<area aria-labelledby="id1"></area>
+<area aria-labelledby="id1" />
 ```
 
 <br>
@@ -461,7 +461,7 @@ The rule does not have any configuration options.
 #### ✅ Valid Code
 
 ```html
-<area alt="This is descriptive!"></area>
+<area alt="This is descriptive!" />
 ```
 
 <br>

diff --git a/packages/eslint-plugin-template/docs/rules/click-events-have-key-events.md b/packages/eslint-plugin-template/docs/rules/click-events-have-key-events.md
@@ -248,6 +248,60 @@ The rule does not have any configuration options.
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ```
 
+<br>
+
+---
+
+<br>
+
+#### Default Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/click-events-have-key-events": [
+      "error"
+    ]
+  }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<div (click)="onClick()" [attr.aria-hidden]="ariaHidden"></div>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+```
+
+<br>
+
+---
+
+<br>
+
+#### Default Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/click-events-have-key-events": [
+      "error"
+    ]
+  }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<div (click)="onClick()" [attr.hidden]="hidden"></div>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+```
+
 </details>
 
 <br>
@@ -356,10 +410,9 @@ The rule does not have any configuration options.
 #### ✅ Valid Code
 
 ```html
-<div (click)="onClick()" aria-hidden"></div>
+<div (click)="onClick()" aria-hidden></div>
 <div (click)="onClick()" aria-hidden="true"></div>
 <div (click)="onClick()" [attr.aria-hidden]="true"></div>
-<div (click)="onClick()" [attr.aria-hidden]="ariaHidden"></div>
 ```
 
 <br>

diff --git a/packages/eslint-plugin-template/src/utils/is-hidden-from-screen-reader.ts b/packages/eslint-plugin-template/src/utils/is-hidden-from-screen-reader.ts
@@ -15,6 +15,10 @@ export function isHiddenFromScreenReader(node: TmplAstElement): boolean {
     hasHiddenStaticStyles(node) ||
     hasHiddenStaticNgStyles(node) ||
     hasHiddenDynamicStylesWithLiteralValues(node) ||
+    /**
+     * We can't know if the element is hidden from screen reader if the value of `aria-hidden` or `hidden`
+     * is set dynamically via an Angular property binding, so we just check for raw HTML truthiness here.
+     */
     isHtmlTruthy(node, 'aria-hidden') ||
     isHtmlTruthy(node, 'hidden')
   ) {

diff --git a/packages/eslint-plugin-template/tests/rules/accessibility-alt-text/cases.ts b/packages/eslint-plugin-template/tests/rules/accessibility-alt-text/cases.ts
@@ -12,9 +12,9 @@ export const valid = [
   '<object aria-labelledby="id1">',
   '<object>Meaningful description</object>',
   '<object title="An object">',
-  '<area aria-label="foo"></area>',
-  '<area aria-labelledby="id1"></area>',
-  '<area alt="This is descriptive!"></area>',
+  '<area aria-label="foo" />',
+  '<area aria-labelledby="id1" />',
+  '<area alt="This is descriptive!" />',
   '<input type="text">',
   '<input type="image" alt="This is descriptive!">',
   '<input type="image" aria-label="foo">',

diff --git a/packages/eslint-plugin-template/tests/rules/click-events-have-key-events/cases.ts b/packages/eslint-plugin-template/tests/rules/click-events-have-key-events/cases.ts
@@ -17,12 +17,11 @@ export const valid = [
     code: '<cui-button (click)="onClick()"></cui-button>',
   },
   {
-    // It should work when element has aria-hidden.
+    // It should work when element has a static value for aria-hidden.
     code: `
-        <div (click)="onClick()" aria-hidden"></div>
+        <div (click)="onClick()" aria-hidden></div>
         <div (click)="onClick()" aria-hidden="true"></div>
         <div (click)="onClick()" [attr.aria-hidden]="true"></div>
-        <div (click)="onClick()" [attr.aria-hidden]="ariaHidden"></div>
       `,
   },
   {
@@ -121,4 +120,22 @@ export const invalid = [
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       `,
   }),
+  convertAnnotatedSourceToFailureCase({
+    messageId,
+    description:
+      'should fail when aria-hidden is set dynamically via a property binding',
+    annotatedSource: `
+      <div (click)="onClick()" [attr.aria-hidden]="ariaHidden"></div>
+      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    `,
+  }),
+  convertAnnotatedSourceToFailureCase({
+    messageId,
+    description:
+      'should fail when hidden is set dynamically via a property binding',
+    annotatedSource: `
+      <div (click)="onClick()" [attr.hidden]="hidden"></div>
+      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    `,
+  }),
 ];
diff --git a/packages/template-parser/src/index.ts b/packages/template-parser/src/index.ts
@@ -1,6 +1,7 @@
 import type {
-  ParseSourceSpan,
   Comment,
+  ParseError,
+  ParseSourceSpan,
 } from '@angular-eslint/bundled-angular-compiler';
 import { parseTemplate } from '@angular-eslint/bundled-angular-compiler';
 import type { TSESTree } from '@typescript-eslint/types';
@@ -185,6 +186,34 @@ function convertNgAstCommentsToTokens(comments: Comment[]) {
   return commentTokens.sort((a, b) => a.range[0] - b.range[0]);
 }
 
+export class TemplateParseError extends Error {
+  constructor(
+    message: string,
+    public readonly fileName: string,
+    public readonly index: number,
+    public readonly lineNumber: number,
+    public readonly column: number,
+  ) {
+    super(message);
+    Object.defineProperty(this, 'name', {
+      value: new.target.name,
+      enumerable: false,
+      configurable: true,
+    });
+  }
+}
+
+export function createTemplateParseError(
+  parseError: ParseError,
+): TemplateParseError {
+  const message = parseError.msg;
+  const fileName = parseError.span.start.file.url;
+  const index = parseError.span.start.offset;
+  const lineNumber = parseError.span.start.line + 1;
+  const column = parseError.span.start.col + 1;
+  return new TemplateParseError(message, fileName, index, lineNumber, column);
+}
+
 function parseForESLint(
   code: string,
   options: { filePath: string },
@@ -240,6 +269,10 @@ function parseForESLint(
     };
   }
 
+  if (angularCompilerResult.errors?.length) {
+    throw createTemplateParseError(angularCompilerResult.errors[0]);
+  }
+
   return {
     ast,
     scopeManager,

diff --git a/packages/template-parser/tests/index.test.ts b/packages/template-parser/tests/index.test.ts
@@ -1,3 +1,4 @@
+import type { TemplateParseError } from '../src/index';
 import { parseForESLint } from '../src/index';
 
 describe('parseForESLint()', () => {
@@ -508,4 +509,33 @@ describe('parseForESLint()', () => {
       }
     `);
   });
+
+  describe('parse errors', () => {
+    it('should appropriately throw if the Angular compiler produced parse errors', () => {
+      expect.assertions(2);
+
+      let error: TemplateParseError;
+      try {
+        parseForESLint(
+          '<p i18n>Lorem ipsum <em i18n="@@dolor">dolor</em> sit amet.</p>',
+          {
+            filePath: './invalid-nested-i18ns.html',
+          },
+        );
+      } catch (err: any) {
+        error = err;
+        expect(error).toMatchInlineSnapshot(
+          `[TemplateParseError: Cannot mark an element as translatable inside of a translatable section. Please remove the nested i18n marker.]`,
+        );
+        expect(JSON.stringify(error, null, 2)).toMatchInlineSnapshot(`
+          "{
+            \\"fileName\\": \\"./invalid-nested-i18ns.html\\",
+            \\"index\\": 20,
+            \\"lineNumber\\": 1,
+            \\"column\\": 21
+          }"
+        `);
+      }
+    });
+  });
 });