From 7d072e2ec053f388adce00be54c75d8c76373699 Mon Sep 17 00:00:00 2001
From: Adrian Baran <93612066+adbaran1@users.noreply.github.com>
Date: Thu, 18 Nov 2021 07:12:44 -0600
Subject: [PATCH] feat(i18n): option to require description for i18n metadata
 (#804)

---
 .../eslint-plugin-template/docs/rules/i18n.md | 60 +++++++++++++++++++
 .../eslint-plugin-template/src/rules/i18n.ts  | 43 +++++++++----
 .../tests/rules/i18n/cases.ts                 | 32 ++++++++++
 3 files changed, 123 insertions(+), 12 deletions(-)

diff --git a/packages/eslint-plugin-template/docs/rules/i18n.md b/packages/eslint-plugin-template/docs/rules/i18n.md
index 2a1552c51..8c04e01d5 100644
--- a/packages/eslint-plugin-template/docs/rules/i18n.md
+++ b/packages/eslint-plugin-template/docs/rules/i18n.md
@@ -49,6 +49,7 @@ interface Options {
    */
   ignoreAttributes?: string[];
   ignoreTags?: string[];
+  requireDescription?: boolean;
 }
 
 ```
@@ -450,6 +451,36 @@ interface Options {
 </div>
 ```
 
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/i18n": [
+      "error",
+      {
+        "requireDescription": true
+      }
+    ]
+  }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<h1 i18n>Hello</h1>
+~~~~~~~~~~~~~~~~~~~
+```
+
 </details>
 
 <br>
@@ -1130,6 +1161,35 @@ interface Options {
 </ul>
 ```
 
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+  "rules": {
+    "@angular-eslint/template/i18n": [
+      "error",
+      {
+        "requireDescription": true
+      }
+    ]
+  }
+}
+```
+
+<br>
+
+#### ✅ Valid Code
+
+```html
+<h1 i18n="An introduction header for this sample">Hello i18n!</h1>
+```
+
 </details>
 
 <br>
diff --git a/packages/eslint-plugin-template/src/rules/i18n.ts b/packages/eslint-plugin-template/src/rules/i18n.ts
index 91295cc3c..6ad47fcf3 100644
--- a/packages/eslint-plugin-template/src/rules/i18n.ts
+++ b/packages/eslint-plugin-template/src/rules/i18n.ts
@@ -58,6 +58,7 @@ type Options = [
     readonly checkText?: boolean;
     readonly ignoreAttributes?: readonly string[];
     readonly ignoreTags?: readonly string[];
+    readonly requireDescription?: boolean;
   },
 ];
 export type MessageIds =
@@ -66,7 +67,8 @@ export type MessageIds =
   | 'i18nCustomIdOnAttribute'
   | 'i18nCustomIdOnElement'
   | 'i18nDuplicateCustomId'
-  | 'suggestAddI18nAttribute';
+  | 'suggestAddI18nAttribute'
+  | 'i18nMissingDescription';
 type StronglyTypedElement = Omit<TmplAstElement, 'i18n'> & {
   i18n: Message;
   parent?: AST;
@@ -98,6 +100,8 @@ const STYLE_GUIDE_LINK_ICU = `${STYLE_GUIDE_LINK}#mark-plurals-and-alternates-fo
 const STYLE_GUIDE_LINK_TEXTS = `${STYLE_GUIDE_LINK}#mark-text-for-translations`;
 const STYLE_GUIDE_LINK_CUSTOM_IDS = `${STYLE_GUIDE_LINK}#manage-marked-text-with-custom-ids`;
 const STYLE_GUIDE_LINK_UNIQUE_CUSTOM_IDS = `${STYLE_GUIDE_LINK}#define-unique-custom-ids`;
+const STYLE_GUIDE_LINK_COMMON_PREPARE = `${STYLE_GUIDE_LINK}-common-prepare`;
+const STYLE_GUIDE_LINK_METADATA_FOR_TRANSLATION = `${STYLE_GUIDE_LINK_COMMON_PREPARE}#i18n-metadata-for-translation`;
 
 export default createESLintRule<Options, MessageIds>({
   name: RULE_NAME,
@@ -147,6 +151,10 @@ export default createESLintRule<Options, MessageIds>({
               type: 'string',
             },
           },
+          requireDescription: {
+            type: 'boolean',
+            default: DEFAULT_OPTIONS.requireDescription,
+          },
         },
         additionalProperties: false,
       },
@@ -158,6 +166,7 @@ export default createESLintRule<Options, MessageIds>({
       i18nCustomIdOnElement: `Missing custom ID on element. See more at ${STYLE_GUIDE_LINK_CUSTOM_IDS}`,
       i18nDuplicateCustomId: `Duplicate custom ID "@@{{customId}}". See more at ${STYLE_GUIDE_LINK_UNIQUE_CUSTOM_IDS}`,
       suggestAddI18nAttribute: 'Add the `i18n` attribute',
+      i18nMissingDescription: `Missing i18n description on element. See more at ${STYLE_GUIDE_LINK_METADATA_FOR_TRANSLATION}`,
     },
   },
   defaultOptions: [DEFAULT_OPTIONS],
@@ -171,6 +180,7 @@ export default createESLintRule<Options, MessageIds>({
         checkText,
         ignoreAttributes,
         ignoreTags,
+        requireDescription,
       },
     ],
   ) {
@@ -187,7 +197,7 @@ export default createESLintRule<Options, MessageIds>({
     const collectedCustomIds = new Map<string, readonly ParseSourceSpan[]>();
 
     function handleElement({
-      i18n: { customId },
+      i18n: { description, customId },
       name,
       parent,
       sourceSpan,
@@ -196,17 +206,26 @@ export default createESLintRule<Options, MessageIds>({
         return;
       }
 
-      if (!isEmpty(customId)) {
-        const sourceSpans = collectedCustomIds.get(customId) ?? [];
-        collectedCustomIds.set(customId, [...sourceSpans, sourceSpan]);
-        return;
+      const loc = parserServices.convertNodeSourceSpanToLoc(sourceSpan);
+
+      if (checkId) {
+        if (isEmpty(customId)) {
+          context.report({
+            messageId: 'i18nCustomIdOnElement',
+            loc,
+          });
+        } else {
+          const sourceSpans = collectedCustomIds.get(customId) ?? [];
+          collectedCustomIds.set(customId, [...sourceSpans, sourceSpan]);
+        }
       }
 
-      const loc = parserServices.convertNodeSourceSpanToLoc(sourceSpan);
-      context.report({
-        messageId: 'i18nCustomIdOnElement',
-        loc,
-      });
+      if (requireDescription && isEmpty(description)) {
+        context.report({
+          messageId: 'i18nMissingDescription',
+          loc,
+        });
+      }
     }
 
     function handleTextAttribute({
@@ -308,7 +327,7 @@ export default createESLintRule<Options, MessageIds>({
     }
 
     return {
-      ...(checkId && {
+      ...((checkId || requireDescription) && {
         'Element[i18n]'(node: StronglyTypedElement) {
           handleElement(node);
         },
diff --git a/packages/eslint-plugin-template/tests/rules/i18n/cases.ts b/packages/eslint-plugin-template/tests/rules/i18n/cases.ts
index 4e17a6bf7..453881e7f 100644
--- a/packages/eslint-plugin-template/tests/rules/i18n/cases.ts
+++ b/packages/eslint-plugin-template/tests/rules/i18n/cases.ts
@@ -7,6 +7,7 @@ const i18nCustomIdOnAttribute: MessageIds = 'i18nCustomIdOnAttribute';
 const i18nCustomIdOnElement: MessageIds = 'i18nCustomIdOnElement';
 const i18nDuplicateCustomId: MessageIds = 'i18nDuplicateCustomId';
 const suggestAddI18nAttribute: MessageIds = 'suggestAddI18nAttribute';
+const i18nMissingDescription: MessageIds = 'i18nMissingDescription';
 
 export const valid = [
   `
@@ -157,6 +158,18 @@ export const valid = [
       <li>ItemC</li>
     </ul>
   `,
+  {
+    code: `
+      <h1 i18n="An introduction header for this sample">Hello i18n!</h1>
+    `,
+    options: [{ checkId: false, requireDescription: true }],
+  },
+  {
+    code: `
+      <h1 i18n="An introduction header for this sample@@custom-id">Hello i18n!</h1>
+    `,
+    options: [{ requireDescription: true }],
+  },
 ];
 
 export const invalid = [
@@ -451,4 +464,23 @@ export const invalid = [
       </div>
     `,
   }),
+  convertAnnotatedSourceToFailureCase({
+    description: 'should fail if i18n description is missing',
+    annotatedSource: `
+      <h1 i18n>Hello</h1>
+      ~~~~~~~~~~~~~~~~~~~
+    `,
+    messageId: i18nMissingDescription,
+    options: [{ checkId: false, requireDescription: true }],
+  }),
+  convertAnnotatedSourceToFailureCase({
+    description:
+      'should fail if i18n description is missing, despite an ID being provided',
+    annotatedSource: `
+      <h1 i18n="@@custom-id">Hello</h1>
+      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    `,
+    messageId: i18nMissingDescription,
+    options: [{ requireDescription: true }],
+  }),
 ];