[Fix] destructuring-assignment, component detection: improve compon…

…ent detection Fixes #3119
jsx-eslint · Nov 5, 2021 · 7b3d728 · 7b3d728
1 parent 0743c41
commit 7b3d728
Show file tree

Hide file tree

Showing 4 changed files with 172 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,10 +21,12 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 * [`no-unstable-components`]: improve handling of objects containing render function properties ([#3111] @fizwidget)
 * [`prop-types`], `propTypes`: add forwardRef<>, ForwardRefRenderFunction<> prop-types ([#3112] @vedadeepta)
 * [`no-typos`]: prevent a crash when using private methods (@ljharb)
+* [`destructuring-assignment`], component detection: improve component detection ([#3122] @vedadeepta)
 
 ### Changed
 * [Tests] test on the new babel eslint parser ([#3113] @ljharb)
 
+[#3122]: https://github.com/yannickcr/eslint-plugin-react/pull/3122
 [#3113]: https://github.com/yannickcr/eslint-plugin-react/pull/3113
 [#3112]: https://github.com/yannickcr/eslint-plugin-react/pull/3112
 [#3111]: https://github.com/yannickcr/eslint-plugin-react/pull/3111

diff --git a/lib/util/Components.js b/lib/util/Components.js
@@ -334,6 +334,10 @@ function componentRule(rule, context) {
       return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, context, strict);
     },
 
+    isReturningOnlyNull(ASTNode) {
+      return jsxUtil.isReturningOnlyNull(this.isCreateElement.bind(this), ASTNode, context);
+    },
+
     getPragmaComponentWrapper(node) {
       let isPragmaComponentWrapper;
       let currentNode = node;
@@ -556,6 +560,16 @@ function componentRule(rule, context) {
           return undefined;
         }
 
+        // case: function any() { return (props) { return not-jsx-and-not-null } }
+        if (node.parent.type === 'ReturnStatement' && !utils.isReturningJSX(node) && !utils.isReturningOnlyNull(node)) {
+          return undefined;
+        }
+
+        // for case abc = { [someobject.somekey]: props => { ... return not-jsx } }
+        if (node.parent && node.parent.key && node.parent.key.type === 'MemberExpression' && !utils.isReturningJSX(node) && !utils.isReturningOnlyNull(node)) {
+          return undefined;
+        }
+
         // Case like `React.memo(() => <></>)` or `React.forwardRef(...)`
         const pragmaComponentWrapper = utils.getPragmaComponentWrapper(node);
         if (pragmaComponentWrapper) {
@@ -805,7 +819,6 @@ function componentRule(rule, context) {
       }
 
       const component = utils.getParentComponent();
-
       if (
         !component
         || (component.parent && component.parent.type === 'JSXExpressionContainer')

diff --git a/lib/util/jsx.js b/lib/util/jsx.js
@@ -148,11 +148,59 @@ function isReturningJSX(isCreateElement, ASTnode, context, strict, ignoreNull) {
   return found;
 }
 
+/**
+ * Check if the node is returning only null values
+ *
+ * @param {Function} isCreateElement Function to determine if a CallExpresion is
+ *   a createElement one
+ * @param {ASTNode} ASTnode The AST node being checked
+ * @param {Context} context The context of `ASTNode`.
+ * @returns {Boolean} True if the node is returning only null values
+ */
+function isReturningOnlyNull(isCreateElement, ASTnode, context) {
+  let found = false;
+  let foundSomethingElse = false;
+  astUtil.traverseReturns(ASTnode, context, (node) => {
+    // Traverse return statement
+    astUtil.traverse(node, {
+      enter(childNode) {
+        const setFound = () => {
+          found = true;
+          this.skip();
+        };
+        const setFoundSomethingElse = () => {
+          foundSomethingElse = true;
+          this.skip();
+        };
+        switch (childNode.type) {
+          case 'ReturnStatement':
+            break;
+          case 'ConditionalExpression':
+            if (childNode.consequent.value === null && childNode.alternate.value === null) {
+              setFound();
+            }
+            break;
+          case 'Literal':
+            if (childNode.value === null) {
+              setFound();
+            }
+            break;
+          default:
+            setFoundSomethingElse();
+        }
+      },
+    });
+  });
+
+  return found && !foundSomethingElse;
+}
+
 module.exports = {
   isDOMComponent,
   isFragment,
   isJSX,
   isJSXAttributeKey,
   isWhiteSpaces,
   isReturningJSX,
+  isReturningOnlyNull,
 };
diff --git a/tests/lib/rules/destructuring-assignment.js b/tests/lib/rules/destructuring-assignment.js
@@ -20,6 +20,56 @@ const parserOptions = {
 const ruleTester = new RuleTester({ parserOptions });
 ruleTester.run('destructuring-assignment', rule, {
   valid: parsers.all([
+    {
+      code: `
+        export const revisionStates2 = {
+            [A.b]: props => {
+              return props.editor !== null
+                ? 'xyz'
+                : 'abc'
+            },
+        };
+      `,
+    },
+    {
+      code: `
+        export function hof(namespace) {
+          const initialState = {
+            bounds: null,
+            search: false,
+          };
+          return (props) => {
+            const {x, y} = props
+            if (y) {
+              return <span>{y}</span>;
+            }
+            return <span>{x}</span>
+          };
+        }
+      `,
+    },
+    {
+      code: `
+        export function hof(namespace) {
+          const initialState = {
+            bounds: null,
+            search: false,
+          };
+
+          return (state = initialState, action) => {
+            if (action.type === 'ABC') {
+              return {...state, bounds: stuff ? action.x : null};
+            }
+
+            if (action.namespace !== namespace) {
+              return state;
+            }
+
+            return null
+          };
+        }
+      `,
+    },
     {
       code: `
         const MyComponent = ({ id, className }) => (
@@ -615,5 +665,63 @@ ruleTester.run('destructuring-assignment', rule, {
         },
       ],
     },
+    {
+      code: `
+        export const revisionStates2 = {
+            [A.b]: props => {
+              return props.editor !== null
+                ? <span>{props.editor}</span>
+                : null
+            },
+        };
+      `,
+      parser: parsers.BABEL_ESLINT,
+      errors: [
+        {
+          messageId: 'useDestructAssignment',
+          data: { type: 'props' },
+          line: 4,
+        },
+        {
+          messageId: 'useDestructAssignment',
+          data: { type: 'props' },
+          line: 5,
+        },
+      ],
+    },
+    {
+      code: `
+        export function hof(namespace) {
+          const initialState = {
+            bounds: null,
+            search: false,
+          };
+          return (props) => {
+            if (props.y) {
+              return <span>{props.y}</span>;
+            }
+            return <span>{props.x}</span>
+          };
+        }
+      `,
+      parser: parsers.BABEL_ESLINT,
+      errors: [
+        {
+          messageId: 'useDestructAssignment',
+          data: { type: 'props' },
+          line: 8,
+        },
+        {
+          messageId: 'useDestructAssignment',
+          data: { type: 'props' },
+          line: 9,
+        },
+        {
+          messageId: 'useDestructAssignment',
+          data: { type: 'props' },
+          line: 11,
+        },
+      ],
+    },
   ]),
 });