From bb8470ddd17bdad1d9da2a4276e7fa1d117308d4 Mon Sep 17 00:00:00 2001
From: m-akinc <7282195+m-akinc@users.noreply.github.com>
Date: Wed, 22 May 2024 07:02:38 -0500
Subject: [PATCH] feat(eslint-plugin-template): [i18n] add allowMarkupInContent
option (#1795)
---
.../eslint-plugin-template/docs/rules/i18n.md | 292 ++++++++++++++++++
.../eslint-plugin-template/src/rules/i18n.ts | 42 ++-
.../tests/rules/i18n/cases.ts | 93 ++++++
3 files changed, 416 insertions(+), 11 deletions(-)
diff --git a/packages/eslint-plugin-template/docs/rules/i18n.md b/packages/eslint-plugin-template/docs/rules/i18n.md
index 4c6722e70..7ec23e91e 100644
--- a/packages/eslint-plugin-template/docs/rules/i18n.md
+++ b/packages/eslint-plugin-template/docs/rules/i18n.md
@@ -30,6 +30,10 @@ The rule accepts an options object with the following properties:
```ts
interface Options {
+ /**
+ * Default: `true`
+ */
+ allowMarkupInContent?: boolean;
boundTextAllowedPattern?: string;
/**
* Default: `true`
@@ -130,6 +134,33 @@ interface Options {
+#### Default Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error"
+ ]
+ }
+}
+```
+
+
+
+#### ❌ Invalid Code
+
+```html
+{ value, plural, =0 {No elements} =1 {111} }
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+```
+
+
+
+---
+
+
+
#### Custom Config
```json
@@ -1098,6 +1129,111 @@ interface Options {
```
+
+
+---
+
+
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error",
+ {
+ "allowMarkupInContent": false,
+ "checkId": false
+ }
+ ]
+ }
+}
+```
+
+
+
+#### ❌ Invalid Code
+
+```html
+
+ Text to translate
+
+ ~~~~~~~~~~~
+
+```
+
+
+
+---
+
+
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error",
+ {
+ "allowMarkupInContent": false,
+ "checkId": false
+ }
+ ]
+ }
+}
+```
+
+
+
+#### ❌ Invalid Code
+
+```html
+
+ Text to translate
+
+ ~~~~~~~~~~~
+
+```
+
+
+
+---
+
+
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error",
+ {
+ "allowMarkupInContent": false,
+ "checkId": false
+ }
+ ]
+ }
+}
+```
+
+
+
+#### ❌ Invalid Code
+
+```html
+
+
+ ~~~~~~~~~~
+ Text to translate
+ ~~~~~~~~~~~~~~~~~
+
+ ~~~~~~
+
@@ -1551,6 +1687,34 @@ interface Options {
#### ✅ Valid Code
+```html
+
+ { value, plural, =0 {No elements} =1 {111} }
+
+```
+
+
+
+---
+
+
+
+#### Default Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error"
+ ]
+ }
+}
+```
+
+
+
+#### ✅ Valid Code
+
```html
{ value, plural, =0 {No elements
} =1 {111} }
@@ -2124,6 +2288,134 @@ interface Options {
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error",
+ {
+ "allowMarkupInContent": false,
+ "checkId": false
+ }
+ ]
+ }
+}
+```
+
+
+
+#### ✅ Valid Code
+
+```html
+
+ Text to translate{{ Bound }}
+
+```
+
+
+
+---
+
+
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error",
+ {
+ "allowMarkupInContent": false,
+ "checkId": false
+ }
+ ]
+ }
+}
+```
+
+
+
+#### ✅ Valid Code
+
+```html
+
+ Text to translate{{ Bound }}
+
+```
+
+
+
+---
+
+
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error",
+ {
+ "allowMarkupInContent": false,
+ "checkId": false
+ }
+ ]
+ }
+}
+```
+
+
+
+#### ✅ Valid Code
+
+```html
+
+ { value, plural, =0 {No elements} =1 {111} }
+
+```
+
+
+
+---
+
+
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/i18n": [
+ "error",
+ {
+ "allowMarkupInContent": false,
+ "checkId": false
+ }
+ ]
+ }
+}
+```
+
+
+
+#### ✅ Valid Code
+
+```html
+
+ { value, plural, =0 {
No elements
} =1 {111} }
+
+
+---
+
+
+
#### Default Config
```json
diff --git a/packages/eslint-plugin-template/src/rules/i18n.ts b/packages/eslint-plugin-template/src/rules/i18n.ts
index a632f2a83..f1745425e 100644
--- a/packages/eslint-plugin-template/src/rules/i18n.ts
+++ b/packages/eslint-plugin-template/src/rules/i18n.ts
@@ -63,6 +63,7 @@ const DEFAULT_ALLOWED_ATTRIBUTES: ReadonlySet = new Set([
type Options = [
{
+ readonly allowMarkupInContent?: boolean;
readonly boundTextAllowedPattern?: string;
readonly checkAttributes?: boolean;
readonly checkDuplicateId?: boolean;
@@ -82,7 +83,8 @@ export type MessageIds =
| 'i18nDuplicateCustomId'
| 'suggestAddI18nAttribute'
| 'i18nMissingDescription'
- | 'i18nMissingMeaning';
+ | 'i18nMissingMeaning'
+ | 'i18nMarkupInContent';
type StronglyTypedI18n = Omit & {
i18n: I18nMeta;
parent?: AST;
@@ -105,6 +107,7 @@ type StronglyTypedBoundTextOrIcuOrText = (
};
export const RULE_NAME = 'i18n';
const DEFAULT_OPTIONS: Options[number] = {
+ allowMarkupInContent: true,
checkAttributes: true,
checkId: true,
checkDuplicateId: true,
@@ -138,6 +141,10 @@ export default createESLintRule({
{
type: 'object',
properties: {
+ allowMarkupInContent: {
+ type: 'boolean',
+ default: DEFAULT_OPTIONS.allowMarkupInContent,
+ },
boundTextAllowedPattern: {
type: 'string',
},
@@ -191,6 +198,8 @@ export default createESLintRule({
suggestAddI18nAttribute: 'Add the `i18n` attribute',
i18nMissingDescription: `Missing i18n description on element. See more at ${STYLE_GUIDE_LINK_METADATA_FOR_TRANSLATION}`,
i18nMissingMeaning: `Missing i18n meaning on element. See more at ${STYLE_GUIDE_LINK_METADATA_FOR_TRANSLATION}`,
+ i18nMarkupInContent:
+ 'Avoid HTML markup in an element with an i18n attribute.',
},
},
defaultOptions: [DEFAULT_OPTIONS],
@@ -198,6 +207,7 @@ export default createESLintRule({
context,
[
{
+ allowMarkupInContent,
boundTextAllowedPattern,
checkAttributes,
checkId,
@@ -211,7 +221,7 @@ export default createESLintRule({
],
) {
const parserServices = getTemplateParserServices(context);
- const sourceCode = context.getSourceCode();
+ const sourceCode = context.sourceCode;
const allowedBoundTextPattern = boundTextAllowedPattern
? new RegExp(boundTextAllowedPattern)
: DEFAULT_ALLOWED_BOUND_TEXT_PATTERN;
@@ -246,16 +256,23 @@ export default createESLintRule({
const { i18n, sourceSpan } = node;
const parent = getNextElementOrTemplateParent(node);
- if (
- isTagAllowed(allowedTags, node) ||
- isElementWithI18n(parent) ||
- isNgTemplateWithI18n(parent)
- ) {
+ if (isTagAllowed(allowedTags, node)) {
return;
}
const loc = parserServices.convertNodeSourceSpanToLoc(sourceSpan);
+ if (isElementWithI18n(parent) || isNgTemplateWithI18n(parent)) {
+ if (allowMarkupInContent) {
+ return;
+ }
+ context.report({
+ messageId: 'i18nMarkupInContent',
+ loc,
+ });
+ return;
+ }
+
const customId = getI18nCustomId(i18n);
if (checkId) {
@@ -400,10 +417,13 @@ export default createESLintRule({
}
return {
- ...((checkId || requireDescription || requireMeaning) && {
- ':matches(Element$1, Template[tagName="ng-template"])[i18n]'(
- node: StronglyTypedElement | StronglyTypedTemplate,
- ) {
+ ...((!allowMarkupInContent ||
+ checkId ||
+ requireDescription ||
+ requireMeaning) && {
+ [`:matches(Element$1, Template[tagName="ng-template"])${
+ allowMarkupInContent ? '[i18n]' : ''
+ }`](node: StronglyTypedElement | StronglyTypedTemplate) {
handleElementOrTemplate(node);
},
}),
diff --git a/packages/eslint-plugin-template/tests/rules/i18n/cases.ts b/packages/eslint-plugin-template/tests/rules/i18n/cases.ts
index 81c7f9302..e82e2de4a 100644
--- a/packages/eslint-plugin-template/tests/rules/i18n/cases.ts
+++ b/packages/eslint-plugin-template/tests/rules/i18n/cases.ts
@@ -9,6 +9,7 @@ const i18nDuplicateCustomId: MessageIds = 'i18nDuplicateCustomId';
const suggestAddI18nAttribute: MessageIds = 'suggestAddI18nAttribute';
const i18nMissingDescription: MessageIds = 'i18nMissingDescription';
const i18nMissingMeaning: MessageIds = 'i18nMissingMeaning';
+const i18nMarkupInContent: MessageIds = 'i18nMarkupInContent';
export const valid = [
`
@@ -98,6 +99,11 @@ export const valid = [
{{ error.title }}
`,
+ `
+
+ { value, plural, =0 {No elements} =1 {111} }
+
+ `,
`
{ value, plural, =0 {No elements
} =1 {111} }
@@ -240,6 +246,39 @@ export const valid = [
`,
options: [{ ignoreTags: ['ng-template'] }],
},
+ {
+ code: `
+
+ Text to translate{{ Bound }}
+
+ `,
+ options: [{ allowMarkupInContent: false, checkId: false }],
+ },
+ {
+ code: `
+
+ Text to translate{{ Bound }}
+
+ `,
+ options: [{ allowMarkupInContent: false, checkId: false }],
+ },
+ {
+ code: `
+
+ { value, plural, =0 {No elements} =1 {111} }
+
+ `,
+ options: [{ allowMarkupInContent: false, checkId: false }],
+ },
+ {
+ code: `
+
+ { value, plural, =0 {
No elements
} =1 {111} }
+
+ Text to translate
+
+ ~~~~~~~~~~~
+
+ `,
+ messageId: i18nMarkupInContent,
+ options: [{ allowMarkupInContent: false, checkId: false }],
+ }),
+ convertAnnotatedSourceToFailureCase({
+ description:
+ 'should fail if `i18n` attribute is on `Template` containing markup',
+ annotatedSource: `
+
+ Text to translate
+
+ ~~~~~~~~~~~
+
+ `,
+ messageId: i18nMarkupInContent,
+ options: [{ allowMarkupInContent: false, checkId: false }],
+ }),
+ convertAnnotatedSourceToFailureCase({
+ description:
+ 'should fail if `i18n` attribute is on element containing markup, even if it also has `i18n` attribute',
+ annotatedSource: `
+
+
+ ~~~~~~~~~~
+ Text to translate
+ ~~~~~~~~~~~~~~~~~
+
+ ~~~~~~
+