From b34a7f8582396355a33bca46f944c5b3f37eab9a Mon Sep 17 00:00:00 2001 From: vedadeepta Date: Fri, 5 Nov 2021 22:32:38 +0530 Subject: [PATCH] [Fix] `Components`, `jsx`: improve component detection --- lib/util/Components.js | 15 ++- lib/util/jsx.js | 48 +++++++++ tests/lib/rules/destructuring-assignment.js | 108 ++++++++++++++++++++ 3 files changed, 170 insertions(+), 1 deletion(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index 804b4049a0..ab2f219d35 100644 --- 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 index a90b894211..8e84b05d43 100644 --- a/lib/util/jsx.js +++ b/lib/util/jsx.js @@ -148,6 +148,53 @@ 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, @@ -155,4 +202,5 @@ module.exports = { isJSXAttributeKey, isWhiteSpaces, isReturningJSX, + isReturningOnlyNull, }; diff --git a/tests/lib/rules/destructuring-assignment.js b/tests/lib/rules/destructuring-assignment.js index 6efae95582..1eac79122e 100644 --- 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 {y}; + } + return {x} + }; + } + `, + }, + { + 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 + ? {props.editor} + : 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 {props.y}; + } + return {props.x} + }; + } + `, + 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, + }, + ], + }, ]), });