[Fix] no-invalid-html-attribute: substitute placeholders in suggest…

…ion messages
Efimenko · May 20, 2024 · 917240b · 917240b
1 parent 4e3f2ae
commit 917240b
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 100 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 [#3724]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3724
 [#3694]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3694
 
+### Fixed
+* [`no-invalid-html-attribute`]: substitute placeholders in suggestion messages ([#3759][] @mdjermanovic)
+
 ## [7.34.4] - 2024.07.13
 
 ### Fixed

diff --git a/lib/rules/no-invalid-html-attribute.js b/lib/rules/no-invalid-html-attribute.js
@@ -8,7 +8,6 @@
 const matchAll = require('string.prototype.matchall');
 const docsUrl = require('../util/docsUrl');
 const report = require('../util/report');
-const getMessageData = require('../util/message');
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -224,6 +223,7 @@ const COMPONENT_ATTRIBUTE_MAP = new Map([
   ['rel', new Set(['link', 'a', 'area', 'form'])],
 ]);
 
+/* eslint-disable eslint-plugin/no-unused-message-ids -- false positives, these messageIds are used */
 const messages = {
   emptyIsMeaningless: 'An empty “{{attributeName}}” attribute is meaningless.',
   neverValid: '“{{reportingValue}}” is never a valid “{{attributeName}}” attribute value.',
@@ -264,15 +264,11 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
     report(context, messages.onlyStrings, 'onlyStrings', {
       node,
       data,
-      suggest: [
-        Object.assign(
-          getMessageData('suggestRemoveNonString', messages.suggestRemoveNonString),
-          {
-            data,
-            fix(fixer) { return fixer.remove(parentNode); },
-          }
-        ),
-      ],
+      suggest: [{
+        messageId: 'suggestRemoveNonString',
+        data,
+        fix(fixer) { return fixer.remove(parentNode); },
+      }],
     });
     return;
   }
@@ -283,15 +279,11 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
     report(context, messages.noEmpty, 'noEmpty', {
       node,
       data,
-      suggest: [
-        Object.assign(
-          getMessageData('suggestRemoveEmpty', messages.suggestRemoveEmpty),
-          {
-            data,
-            fix(fixer) { return fixer.remove(node.parent); },
-          }
-        ),
-      ],
+      suggest: [{
+        messageId: 'suggestRemoveEmpty',
+        data,
+        fix(fixer) { return fixer.remove(node.parent); },
+      }],
     });
     return;
   }
@@ -307,18 +299,16 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
         reportingValue,
       };
 
+      const suggest = [{
+        messageId: 'suggestRemoveInvalid',
+        data,
+        fix(fixer) { return fixer.removeRange(singlePart.range); },
+      }];
+
       report(context, messages.neverValid, 'neverValid', {
         node,
         data,
-        suggest: [
-          Object.assign(
-            getMessageData('suggestRemoveInvalid', messages.suggestRemoveInvalid),
-            {
-              data,
-              fix(fixer) { return fixer.removeRange(singlePart.range); },
-            }
-          ),
-        ],
+        suggest,
       });
     } else if (!allowedTags.has(parentNodeName)) {
       const data = {
@@ -327,18 +317,16 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
         elementName: parentNodeName,
       };
 
+      const suggest = [{
+        messageId: 'suggestRemoveInvalid',
+        data,
+        fix(fixer) { return fixer.removeRange(singlePart.range); },
+      }];
+
       report(context, messages.notValidFor, 'notValidFor', {
         node,
         data,
-        suggest: [
-          Object.assign(
-            getMessageData('suggestRemoveInvalid', messages.suggestRemoveInvalid),
-            {
-              data,
-              fix(fixer) { return fixer.removeRange(singlePart.range); },
-            }
-          ),
-        ],
+        suggest,
       });
     }
   }
@@ -375,33 +363,27 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
 
   const whitespaceParts = splitIntoRangedParts(node, /(\s+)/g);
   for (const whitespacePart of whitespaceParts) {
+    const data = { attributeName };
+
     if (whitespacePart.range[0] === (node.range[0] + 1) || whitespacePart.range[1] === (node.range[1] - 1)) {
       report(context, messages.spaceDelimited, 'spaceDelimited', {
         node,
-        data: { attributeName },
-        suggest: [
-          Object.assign(
-            getMessageData('suggestRemoveWhitespaces', messages.suggestRemoveWhitespaces),
-            {
-              data: { attributeName },
-              fix(fixer) { return fixer.removeRange(whitespacePart.range); },
-            }
-          ),
-        ],
+        data,
+        suggest: [{
+          messageId: 'suggestRemoveWhitespaces',
+          data,
+          fix(fixer) { return fixer.removeRange(whitespacePart.range); },
+        }],
       });
     } else if (whitespacePart.value !== '\u0020') {
       report(context, messages.spaceDelimited, 'spaceDelimited', {
         node,
-        data: { attributeName },
-        suggest: [
-          Object.assign(
-            getMessageData('suggestRemoveWhitespaces', messages.suggestRemoveWhitespaces),
-            {
-              data: { attributeName },
-              fix(fixer) { return fixer.replaceTextRange(whitespacePart.range, '\u0020'); },
-            }
-          ),
-        ],
+        data,
+        suggest: [{
+          messageId: 'suggestRemoveWhitespaces',
+          data,
+          fix(fixer) { return fixer.replaceTextRange(whitespacePart.range, '\u0020'); },
+        }],
       });
     }
   }
@@ -426,15 +408,11 @@ function checkAttribute(context, node) {
     report(context, messages.onlyMeaningfulFor, 'onlyMeaningfulFor', {
       node: node.name,
       data,
-      suggest: [
-        Object.assign(
-          getMessageData('suggestRemoveDefault', messages.suggestRemoveDefault),
-          {
-            data,
-            fix(fixer) { return fixer.remove(node); },
-          }
-        ),
-      ],
+      suggest: [{
+        messageId: 'suggestRemoveDefault',
+        data,
+        fix(fixer) { return fixer.remove(node); },
+      }],
     });
     return;
   }
@@ -447,12 +425,11 @@ function checkAttribute(context, node) {
     report(context, messages.emptyIsMeaningless, 'emptyIsMeaningless', {
       node: node.name,
       data,
-      suggest: [
-        Object.assign(
-          getMessageData('suggestRemoveEmpty', messages.suggestRemoveEmpty),
-          { data, fix }
-        ),
-      ],
+      suggest: [{
+        messageId: 'suggestRemoveEmpty',
+        data,
+        fix,
+      }],
     });
     return;
   }
@@ -475,25 +452,23 @@ function checkAttribute(context, node) {
     report(context, messages.onlyStrings, 'onlyStrings', {
       node: node.value,
       data,
-      suggest: [
-        Object.assign(
-          getMessageData('suggestRemoveDefault', messages.suggestRemoveDefault),
-          { data, fix }
-        ),
-      ],
+      suggest: [{
+        messageId: 'suggestRemoveDefault',
+        data,
+        fix,
+      }],
     });
   } else if (node.value.expression.type === 'Identifier' && node.value.expression.name === 'undefined') {
     const data = { attributeName: attribute };
 
     report(context, messages.onlyStrings, 'onlyStrings', {
       node: node.value,
       data,
-      suggest: [
-        Object.assign(
-          getMessageData('suggestRemoveDefault', messages.suggestRemoveDefault),
-          { data, fix }
-        ),
-      ],
+      suggest: [{
+        messageId: 'suggestRemoveDefault',
+        data,
+        fix,
+      }],
     });
   }
 }
@@ -523,15 +498,11 @@ function checkPropValidValue(context, node, value, attribute) {
     report(context, messages.neverValid, 'neverValid', {
       node: value,
       data,
-      suggest: [
-        Object.assign(
-          getMessageData('suggestRemoveInvalid', messages.suggestRemoveInvalid),
-          {
-            data,
-            fix(fixer) { return fixer.replaceText(value, value.raw.replace(value.value, '')); },
-          }
-        ),
-      ],
+      suggest: [{
+        messageId: 'suggestRemoveInvalid',
+        data,
+        fix(fixer) { return fixer.replaceText(value, value.raw.replace(value.value, '')); },
+      }],
     });
   } else if (!validTagSet.has(node.arguments[0].value)) {
     report(context, messages.notValidFor, 'notValidFor', {

diff --git a/tests/helpers/parsers.js b/tests/helpers/parsers.js
@@ -105,7 +105,7 @@ const parsers = {
           && {
             errors: testObject.errors.map(
               (errorObject) => {
-                const nextSuggestions = errorObject.suggestions && {
+                const nextSuggestions = errorObject.suggestions && typeof errorObject.suggestions !== 'number' && {
                   suggestions: errorObject.suggestions.map((suggestion) => Object.assign({}, suggestion, {
                     output: suggestion.output + extraComment,
                   })),

diff --git a/tests/lib/rules/no-invalid-html-attribute.js b/tests/lib/rules/no-invalid-html-attribute.js
@@ -9,6 +9,8 @@
 // Requirements
 // ------------------------------------------------------------------------------
 
+const semver = require('semver');
+const eslintPkg = require('eslint/package.json');
 const RuleTester = require('../../helpers/ruleTester');
 const rule = require('../../../lib/rules/no-invalid-html-attribute');
 const parsers = require('../../helpers/parsers');
@@ -488,12 +490,19 @@ ruleTester.run('no-invalid-html-attribute', rule, {
             attributeName: 'rel',
             reportingValue: 1,
           },
-          // suggestions: [
-          //   {
-          //     messageId: 'suggestRemoveDefault',
-          //     output: 'React.createElement("a", { })',
-          //   },
-          // ],
+
+          // FIXME: this suggestion produces invalid code
+          // In ESLint > 9, RuleTester doesn't allow suggestions with parsing errors.
+          suggestions: semver.major(eslintPkg.version) < 9
+            ? [
+              {
+                messageId: 'suggestRemoveInvalid',
+                data: { reportingValue: '1' },
+                output: 'React.createElement("a", { rel:  })',
+              },
+            ]
+            : 1,
+
           type: 'Literal',
         },
       ],