[Fix] no-unknown-property: properly recognize unknown HTML/DOM attributes

sjarva · ljharb · commit f54b391d6ed8 · 2022-08-31T18:13:17.000+03:00
Refactored going through different scenarios and error cases to be more clear.
Also add some new unit tests, and move one should-have-been invalid test case from valid to invalid.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * [`jsx-key`]: avoid a crash with optional chaining ([#3371][] @ljharb)
 * [`jsx-sort-props`]: avoid a crash with spread props ([#3376][] @ljharb)
 * [`no-unknown-property`]: properly recognize valid data- and aria- attributes ([#3377][] @sjarva)
+* [`no-unknown-property`]: properly recognize unknown HTML/DOM attributes ([#3377][] @sjarva)
 
 ### Changed
 * [Docs] [`jsx-sort-props`]: replace ref string with ref variable ([#3375][] @Luccasoli)
diff --git a/docs/rules/no-unknown-property.md b/docs/rules/no-unknown-property.md
@@ -15,6 +15,10 @@ Examples of **incorrect** code for this rule:
 var React = require('react');
 
 var Hello = <div class="hello">Hello World</div>;
+var Alphabet = <div abc="something">Alphabet</div>;
+
+// Invalid aria-* attribute
+var IconButton = <div aria-foo="bar" />;
 ```
 
 Examples of **correct** code for this rule:
@@ -23,14 +27,23 @@ Examples of **correct** code for this rule:
 var React = require('react');
 
 var Hello = <div className="hello">Hello World</div>;
+var Button = <button disabled>Cannot click me</button>;
+var Img = <img src={catImage} alt="A cat sleeping on a keyboard" />;
 
 // aria-* attributes
 var IconButton = <button aria-label="Close" onClick={this.close}>{closeIcon}</button>;
 
-
 // data-* attributes
 var Data = <div data-index={12}>Some data</div>;
 
+// React components are ignored
+var MyComponent = <App class="foo-bar"/>;
+var AnotherComponent = <Foo.bar for="bar" />;
+
+// Custom web components are ignored
+var MyElem = <div class="foo" is="my-elem"></div>;
+var AtomPanel = <atom-panel class="foo"></atom-panel>;
+
 ```
 
 ## Rule Options
diff --git a/lib/rules/no-unknown-property.js b/lib/rules/no-unknown-property.js
@@ -366,37 +366,60 @@ module.exports = {
 
         const tagName = getTagName(node);
 
-        // 1. Some attributes are allowed on some tags only.
-        const allowedTags = has(ATTRIBUTE_TAGS_MAP, name) ? ATTRIBUTE_TAGS_MAP[name] : null;
-        if (tagName && allowedTags && /[^A-Z]/.test(tagName.charAt(0)) && allowedTags.indexOf(tagName) === -1) {
-          report(context, messages.invalidPropOnTag, 'invalidPropOnTag', {
+        // Let's dive deeper into tags that are HTML/DOM elements (`<button>`), and not React components (`<Button />`)
+        if (isValidHTMLTagInJSX(node)) {
+          // Some attributes are allowed on some tags only
+          const allowedTags = has(ATTRIBUTE_TAGS_MAP, name) ? ATTRIBUTE_TAGS_MAP[name] : null;
+          if (tagName && allowedTags) {
+            // Scenario 1A: Allowed attribute found where not supposed to, report it
+            if (allowedTags.indexOf(tagName) === -1) {
+              report(context, messages.invalidPropOnTag, 'invalidPropOnTag', {
+                node,
+                data: {
+                  name,
+                  tagName,
+                  allowedTags: allowedTags.join(', '),
+                },
+              });
+            }
+            // Scenario 1B: There are allowed attributes on allowed tags, no need to report it
+            return;
+          }
+
+          // Let's see if the attribute is a close version to some standard property name
+          const standardName = getStandardName(name, context);
+
+          const hasStandardNameButIsNotUsed = standardName && standardName !== name;
+          const usesStandardName = standardName && standardName === name;
+
+          if (usesStandardName) {
+            // Scenario 2A: The attribute name is the standard name, no need to report it
+            return;
+          }
+
+          if (hasStandardNameButIsNotUsed) {
+            // Scenario 2B: The name of the attribute is close to a standard one, report it with the standard name
+            report(context, messages.unknownPropWithStandardName, 'unknownPropWithStandardName', {
+              node,
+              data: {
+                name,
+                standardName,
+              },
+              fix(fixer) {
+                return fixer.replaceText(node.name, standardName);
+              },
+            });
+            return;
+          }
+
+          // Scenario 3: We have an attribute that is unknown, report it
+          report(context, messages.unknownProp, 'unknownProp', {
             node,
             data: {
               name,
-              tagName,
-              allowedTags: allowedTags.join(', '),
             },
           });
         }
-
-        // 2. Otherwise, we'll try to find if the attribute is a close version
-        // of what we should normally have with React. If yes, we'll report an
-        // error. We don't want to report if the input attribute name is the
-        // standard name though!
-        const standardName = getStandardName(name, context);
-        if (!isValidHTMLTagInJSX(node) || !standardName || standardName === name) {
-          return;
-        }
-        report(context, messages.unknownPropWithStandardName, 'unknownPropWithStandardName', {
-          node,
-          data: {
-            name,
-            standardName,
-          },
-          fix(fixer) {
-            return fixer.replaceText(node.name, standardName);
-          },
-        });
       },
     };
   },
diff --git a/tests/lib/rules/no-unknown-property.js b/tests/lib/rules/no-unknown-property.js
@@ -29,40 +29,101 @@ const parserOptions = {
 const ruleTester = new RuleTester({ parserOptions });
 ruleTester.run('no-unknown-property', rule, {
   valid: parsers.all([
+    // React components and their props/attributes should be fine
     { code: '<App class="bar" />;' },
     { code: '<App for="bar" />;' },
+    { code: '<App someProp="bar" />;' },
     { code: '<Foo.bar for="bar" />;' },
     { code: '<App accept-charset="bar" />;' },
-    { code: '<meta charset="utf-8" />;' },
-    { code: '<meta charSet="utf-8" />;' },
     { code: '<App http-equiv="bar" />;' },
     {
       code: '<App xlink:href="bar" />;',
       features: ['jsx namespace'],
     },
     { code: '<App clip-path="bar" />;' },
+    // Some HTML/DOM elements with common attributes should work
     { code: '<div className="bar"></div>;' },
     { code: '<div onMouseDown={this._onMouseDown}></div>;' },
-    // data attributes should work
+    { code: '<a href="someLink">Read more</a>' },
+    { code: '<img src="cat_keyboard.jpeg" alt="A cat sleeping on a keyboard" />' },
+    { code: '<input type="password" required />' },
+    { code: '<input ref={this.input} type="radio" />' },
+    { code: '<button disabled>You cannot click me</button>;' },
+    // Case ignored attributes, for `charset` discussion see https://github.com/jsx-eslint/eslint-plugin-react/pull/1863
+    { code: '<meta charset="utf-8" />;' },
+    { code: '<meta charSet="utf-8" />;' },
+    // Some custom web components that are allowed to use `class` instead of `className`
+    { code: '<div class="foo" is="my-elem"></div>;' },
+    { code: '<div {...this.props} class="foo" is="my-elem"></div>;' },
+    { code: '<atom-panel class="foo"></atom-panel>;' },
+    // data-* attributes should work
     { code: '<div data-foo="bar"></div>;' },
     { code: '<div data-foo-bar="baz"></div>;' },
     { code: '<div data-parent="parent"></div>;' },
     { code: '<div data-index-number="1234"></div>;' },
-    { code: '<div class="foo" is="my-elem"></div>;' },
-    { code: '<div {...this.props} class="foo" is="my-elem"></div>;' },
-    { code: '<atom-panel class="foo"></atom-panel>;' }, {
+    // Ignoring should work
+    {
       code: '<div class="bar"></div>;',
       options: [{ ignore: ['class'] }],
     },
-    // aria attributes should work
+    {
+      code: '<div someProp="bar"></div>;',
+      options: [{ ignore: ['someProp'] }],
+    },
+
+    // aria-* attributes should work
     { code: '<button aria-haspopup="true">Click me to open pop up</button>;' },
     { code: '<button aria-label="Close" onClick={someThing.close} />;' },
+    // Attributes on allowed elements should work
     { code: '<script crossOrigin />' },
     { code: '<audio crossOrigin />' },
-    { code: '<div hasOwnProperty="should not be allowed tag" />' },
     { code: '<svg><image crossOrigin /></svg>' },
   ]),
   invalid: parsers.all([
+    {
+      code: '<div hasOwnProperty="should not be allowed property"></div>;',
+      errors: [
+        {
+          messageId: 'unknownProp',
+          data: {
+            name: 'hasOwnProperty',
+          },
+        },
+      ],
+    },
+    {
+      code: '<div abc="should not be allowed property"></div>;',
+      errors: [
+        {
+          messageId: 'unknownProp',
+          data: {
+            name: 'abc',
+          },
+        },
+      ],
+    },
+    {
+      code: '<div aria-fake="should not be allowed property"></div>;',
+      errors: [
+        {
+          messageId: 'unknownProp',
+          data: {
+            name: 'aria-fake',
+          },
+        },
+      ],
+    },
+    {
+      code: '<div someProp="bar"></div>;',
+      errors: [
+        {
+          messageId: 'unknownProp',
+          data: {
+            name: 'someProp',
+          },
+        },
+      ],
+    },
     {
       code: '<div class="bar"></div>;',
       output: '<div className="bar"></div>;',
