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

Fixes #3119

Author: vedadeepta

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

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')

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,
 };

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,
+ },
+ ],
+ },
  ]),
 });