From 30f002204048d9e6e0605af2329e74d360f173a4 Mon Sep 17 00:00:00 2001 From: "Hur Hyeon Bin (Max)" <160996936+hyeonbinHur@users.noreply.github.com> Date: Fri, 23 May 2025 02:33:20 +0900 Subject: [PATCH 1/7] [Fix] `display-name`: avoid false positive when React is shadowed Fixes #3924 --- lib/rules/display-name.js | 193 ++++++++++++++++++++++++-------- tests/lib/rules/display-name.js | 138 ++++++++++++++++++++--- 2 files changed, 271 insertions(+), 60 deletions(-) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index b85ec34f1c..cfa8e89339 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -31,7 +31,8 @@ const messages = { module.exports = { meta: { docs: { - description: 'Disallow missing displayName in a React component definition', + description: + 'Disallow missing displayName in a React component definition', category: 'Best Practices', recommended: true, url: docsUrl('display-name'), @@ -39,24 +40,27 @@ module.exports = { messages, - schema: [{ - type: 'object', - properties: { - ignoreTranspilerName: { - type: 'boolean', - }, - checkContextObjects: { - type: 'boolean', + schema: [ + { + type: 'object', + properties: { + ignoreTranspilerName: { + type: 'boolean', + }, + checkContextObjects: { + type: 'boolean', + }, }, + additionalProperties: false, }, - additionalProperties: false, - }], + ], }, create: Components.detect((context, components, utils) => { const config = context.options[0] || {}; const ignoreTranspilerName = config.ignoreTranspilerName || false; - const checkContextObjects = (config.checkContextObjects || false) && testReactVersion(context, '>= 16.3.0'); + const checkContextObjects = (config.checkContextObjects || false) + && testReactVersion(context, '>= 16.3.0'); const contextObjects = new Map(); @@ -76,10 +80,12 @@ module.exports = { * @returns {boolean} True if React.forwardRef is nested inside React.memo, false if not. */ function isNestedMemo(node) { - return astUtil.isCallExpression(node) + return ( + astUtil.isCallExpression(node) && node.arguments && astUtil.isCallExpression(node.arguments[0]) - && utils.isPragmaComponentWrapper(node); + && utils.isPragmaComponentWrapper(node) + ); } /** @@ -115,52 +121,131 @@ module.exports = { * @returns {boolean} True if component has a name, false if not. */ function hasTranspilerName(node) { - const namedObjectAssignment = ( - node.type === 'ObjectExpression' + const namedObjectAssignment = node.type === 'ObjectExpression' && node.parent && node.parent.parent && node.parent.parent.type === 'AssignmentExpression' - && ( - !node.parent.parent.left.object + && (!node.parent.parent.left.object || node.parent.parent.left.object.name !== 'module' - || node.parent.parent.left.property.name !== 'exports' - ) - ); - const namedObjectDeclaration = ( - node.type === 'ObjectExpression' + || node.parent.parent.left.property.name !== 'exports'); + const namedObjectDeclaration = node.type === 'ObjectExpression' && node.parent && node.parent.parent - && node.parent.parent.type === 'VariableDeclarator' - ); - const namedClass = ( - (node.type === 'ClassDeclaration' || node.type === 'ClassExpression') + && node.parent.parent.type === 'VariableDeclarator'; + const namedClass = (node.type === 'ClassDeclaration' || node.type === 'ClassExpression') && node.id - && !!node.id.name - ); + && !!node.id.name; - const namedFunctionDeclaration = ( - (node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression') + const namedFunctionDeclaration = (node.type === 'FunctionDeclaration' + || node.type === 'FunctionExpression') && node.id - && !!node.id.name - ); + && !!node.id.name; - const namedFunctionExpression = ( - astUtil.isFunctionLikeExpression(node) + const namedFunctionExpression = astUtil.isFunctionLikeExpression(node) && node.parent - && (node.parent.type === 'VariableDeclarator' || node.parent.type === 'Property' || node.parent.method === true) - && (!node.parent.parent || !componentUtil.isES5Component(node.parent.parent, context)) - ); + && (node.parent.type === 'VariableDeclarator' + || node.parent.type === 'Property' + || node.parent.method === true) + && (!node.parent.parent + || !componentUtil.isES5Component(node.parent.parent, context)); if ( - namedObjectAssignment || namedObjectDeclaration + namedObjectAssignment + || namedObjectDeclaration || namedClass - || namedFunctionDeclaration || namedFunctionExpression + || namedFunctionDeclaration + || namedFunctionExpression ) { return true; } return false; } + function hasVariableDeclaration(node, name) { + if (!node) return false; + + if (node.type === 'VariableDeclaration') { + return node.declarations.some((decl) => { + if (!decl.id) return false; + + // const name = ... + if (decl.id.type === 'Identifier' && decl.id.name === name) { + return true; + } + + // const [name] = ... + if (decl.id.type === 'ArrayPattern') { + return decl.id.elements.some( + (el) => el && el.type === 'Identifier' && el.name === name + ); + } + + // const { name } = ... + if (decl.id.type === 'ObjectPattern') { + return decl.id.properties.some( + (prop) => prop.type === 'Property' && prop.key && prop.key.name === name + ); + } + + return false; + }); + } + + if (node.type === 'BlockStatement' && node.body) { + return node.body.some((stmt) => hasVariableDeclaration(stmt, name)); + } + + return false; + } + + function isIdentifierShadowed(node, identifierName) { + while (node && node.parent) { + node = node.parent; + if ( + node.type === 'FunctionDeclaration' + || node.type === 'FunctionExpression' + || node.type === 'ArrowFunctionExpression' + ) { + break; + } + } + + if (!node || !node.body) { + return false; + } + + return hasVariableDeclaration(node.body, identifierName); + } + + /** + * + * Check is current component shadowed + * @param {ASTNode} node The AST node being checked. + * @returns {boolean} True if component has a name, false if not. + */ + + function isShadowedComponent(node) { + if (!node || node.type !== 'CallExpression') { + return false; + } + + if ( + node.callee.type === 'MemberExpression' + && node.callee.object.name === 'React' + ) { + return isIdentifierShadowed(node, 'React'); + } + + if (node.callee.type === 'Identifier') { + const name = node.callee.name; + if (name === 'memo' || name === 'forwardRef') { + return isIdentifierShadowed(node, name); + } + } + + return false; + } + // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- @@ -168,7 +253,10 @@ module.exports = { return { ExpressionStatement(node) { if (checkContextObjects && isCreateContext(node)) { - contextObjects.set(node.expression.left.name, { node, hasDisplayName: false }); + contextObjects.set(node.expression.left.name, { + node, + hasDisplayName: false, + }); } }, VariableDeclarator(node) { @@ -232,7 +320,10 @@ module.exports = { if (ignoreTranspilerName || !hasTranspilerName(node)) { // Search for the displayName declaration node.properties.forEach((property) => { - if (!property.key || !propsUtil.isDisplayNameDeclaration(property.key)) { + if ( + !property.key + || !propsUtil.isDisplayNameDeclaration(property.key) + ) { return; } markDisplayNameAsDeclared(node); @@ -247,7 +338,10 @@ module.exports = { return; } - if (node.arguments.length > 0 && astUtil.isFunctionLikeExpression(node.arguments[0])) { + if ( + node.arguments.length > 0 + && astUtil.isFunctionLikeExpression(node.arguments[0]) + ) { // Skip over React.forwardRef declarations that are embedded within // a React.memo i.e. React.memo(React.forwardRef(/* ... */)) // This means that we raise a single error for the call to React.memo @@ -269,9 +363,16 @@ module.exports = { 'Program:exit'() { const list = components.list(); // Report missing display name for all components - values(list).filter((component) => !component.hasDisplayName).forEach((component) => { - reportMissingDisplayName(component); - }); + values(list) + .filter((component) => { + if (isShadowedComponent(component.node)) { + return false; + } + return !component.hasDisplayName; + }) + .forEach((component) => { + reportMissingDisplayName(component); + }); if (checkContextObjects) { // Report missing display name for all context objects forEach( diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 18fd1ca83c..419c6b869d 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -29,6 +29,56 @@ const parserOptions = { const ruleTester = new RuleTester({ parserOptions }); ruleTester.run('display-name', rule, { valid: parsers.all([ + { + code: ` + import React, { forwardRef } from 'react' + + const TestComponent = function () { + const { forwardRef } = { forwardRef: () => null } + + const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) + + return OtherComp + } + `, + }, + { + code: ` + import React, { memo } from 'react' + + const TestComponent = function () { + const memo = (cb) => cb() + + const Comp = memo(() => { + return
shadowed
+ }) + + return Comp + } + `, + }, + { + code: ` + import React, { memo, forwardRef } from 'react' + + const MixedShadowed = function () { + const memo = (cb) => cb() + const { forwardRef } = { forwardRef: () => null } + const [React] = [{ memo, forwardRef }] + + const Comp = memo(() => { + return
shadowed
+ }) + const ReactMemo = React.memo(() => null) + const ReactForward = React.forwardRef((props, ref) => { + return \`\${props} \${ref}\` + }) + const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) + + return [Comp, ReactMemo, ReactForward, OtherComp] + } + `, + }, { code: ` var Hello = createReactClass({ @@ -848,6 +898,66 @@ ruleTester.run('display-name', rule, { ]), invalid: parsers.all([ + { + code: ` + import React from 'react' + + const Comp = React.forwardRef((props, ref) => { + return
test
+ }) + `, + errors: [ + { + messageId: 'noDisplayName', + line: 4, + }, + ], + }, + { + code: ` + import {forwardRef} from 'react' + + const Comp = forwardRef((props, ref) => { + return
test
+ }) + `, + errors: [ + { + messageId: 'noDisplayName', + line: 4, + }, + ], + }, + { + code: ` + import React from 'react' + + const Comp = React.memo(() => { + return
test
+ }) + `, + errors: [ + { + messageId: 'noDisplayName', + line: 4, + }, + ], + }, + { + code: ` + import { memo } from 'react' + + const Comp = memo(() => { + return
test
+ }) + `, + errors: [ + { + messageId: 'noDisplayName', + line: 4, + }, + ], + }, { code: ` var Hello = createReactClass({ @@ -860,7 +970,8 @@ ruleTester.run('display-name', rule, { errors: [ { messageId: 'noDisplayName', - }], + }, + ], }, { code: ` @@ -1067,9 +1178,9 @@ ruleTester.run('display-name', rule, { errors: [{ messageId: 'noDisplayName' }], }, { - // Only trigger an error for the outer React.memo, - // if the React version is not in the following range: - // ^0.14.10 || ^15.7.0 || >= 16.12.0 + // Only trigger an error for the outer React.memo, + // if the React version is not in the following range: + // ^0.14.10 || ^15.7.0 || >= 16.12.0 code: ` import React from 'react' @@ -1082,7 +1193,8 @@ ruleTester.run('display-name', rule, { errors: [ { messageId: 'noDisplayName', - }], + }, + ], settings: { react: { version: '15.6.0', @@ -1090,9 +1202,9 @@ ruleTester.run('display-name', rule, { }, }, { - // Only trigger an error for the outer React.memo, - // if the React version is not in the following range: - // ^0.14.10 || ^15.7.0 || >= ^16.12.0 + // Only trigger an error for the outer React.memo, + // if the React version is not in the following range: + // ^0.14.10 || ^15.7.0 || >= ^16.12.0 code: ` import React from 'react' @@ -1110,9 +1222,9 @@ ruleTester.run('display-name', rule, { }, }, { - // React does not handle the result of forwardRef being passed into memo - // ComponentWithMemoAndForwardRef gets shown as Memo(Anonymous) - // See https://github.com/facebook/react/issues/16722 + // React does not handle the result of forwardRef being passed into memo + // ComponentWithMemoAndForwardRef gets shown as Memo(Anonymous) + // See https://github.com/facebook/react/issues/16722 code: ` import React from 'react' @@ -1239,9 +1351,7 @@ ruleTester.run('display-name', rule, {
{a} {listItem}
); `, - errors: [ - { message: 'Component definition is missing display name' }, - ], + errors: [{ message: 'Component definition is missing display name' }], }, { code: ` From 6e055364257faf3f1aacd2bcb9bfe301f42895c3 Mon Sep 17 00:00:00 2001 From: "Hur Hyeon Bin (Max)" <160996936+hyeonbinHur@users.noreply.github.com> Date: Fri, 23 May 2025 04:37:11 +0900 Subject: [PATCH 2/7] style: fix lint,prettier errors --- lib/rules/display-name.js | 119 +++++++++++++++----------------- tests/lib/rules/display-name.js | 28 ++++---- 2 files changed, 68 insertions(+), 79 deletions(-) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index cfa8e89339..19f5283865 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -31,8 +31,7 @@ const messages = { module.exports = { meta: { docs: { - description: - 'Disallow missing displayName in a React component definition', + description: 'Disallow missing displayName in a React component definition', category: 'Best Practices', recommended: true, url: docsUrl('display-name'), @@ -40,27 +39,24 @@ module.exports = { messages, - schema: [ - { - type: 'object', - properties: { - ignoreTranspilerName: { - type: 'boolean', - }, - checkContextObjects: { - type: 'boolean', - }, + schema: [{ + type: 'object', + properties: { + ignoreTranspilerName: { + type: 'boolean', + }, + checkContextObjects: { + type: 'boolean', }, - additionalProperties: false, }, - ], + additionalProperties: false, + }], }, create: Components.detect((context, components, utils) => { const config = context.options[0] || {}; const ignoreTranspilerName = config.ignoreTranspilerName || false; - const checkContextObjects = (config.checkContextObjects || false) - && testReactVersion(context, '>= 16.3.0'); + const checkContextObjects = (config.checkContextObjects || false) && testReactVersion(context, '>= 16.3.0'); const contextObjects = new Map(); @@ -80,12 +76,10 @@ module.exports = { * @returns {boolean} True if React.forwardRef is nested inside React.memo, false if not. */ function isNestedMemo(node) { - return ( - astUtil.isCallExpression(node) + return astUtil.isCallExpression(node) && node.arguments && astUtil.isCallExpression(node.arguments[0]) - && utils.isPragmaComponentWrapper(node) - ); + && utils.isPragmaComponentWrapper(node); } /** @@ -121,40 +115,46 @@ module.exports = { * @returns {boolean} True if component has a name, false if not. */ function hasTranspilerName(node) { - const namedObjectAssignment = node.type === 'ObjectExpression' + const namedObjectAssignment = ( + node.type === 'ObjectExpression' && node.parent && node.parent.parent && node.parent.parent.type === 'AssignmentExpression' - && (!node.parent.parent.left.object + && ( + !node.parent.parent.left.object || node.parent.parent.left.object.name !== 'module' - || node.parent.parent.left.property.name !== 'exports'); - const namedObjectDeclaration = node.type === 'ObjectExpression' + || node.parent.parent.left.property.name !== 'exports' + ) + ); + const namedObjectDeclaration = ( + node.type === 'ObjectExpression' && node.parent && node.parent.parent - && node.parent.parent.type === 'VariableDeclarator'; - const namedClass = (node.type === 'ClassDeclaration' || node.type === 'ClassExpression') + && node.parent.parent.type === 'VariableDeclarator' + ); + const namedClass = ( + (node.type === 'ClassDeclaration' || node.type === 'ClassExpression') && node.id - && !!node.id.name; + && !!node.id.name + ); - const namedFunctionDeclaration = (node.type === 'FunctionDeclaration' - || node.type === 'FunctionExpression') + const namedFunctionDeclaration = ( + (node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression') && node.id - && !!node.id.name; + && !!node.id.name + ); - const namedFunctionExpression = astUtil.isFunctionLikeExpression(node) + const namedFunctionExpression = ( + astUtil.isFunctionLikeExpression(node) && node.parent - && (node.parent.type === 'VariableDeclarator' - || node.parent.type === 'Property' - || node.parent.method === true) - && (!node.parent.parent - || !componentUtil.isES5Component(node.parent.parent, context)); + && (node.parent.type === 'VariableDeclarator' || node.parent.type === 'Property' || node.parent.method === true) + && (!node.parent.parent || !componentUtil.isES5Component(node.parent.parent, context)) + ); if ( - namedObjectAssignment - || namedObjectDeclaration + namedObjectAssignment || namedObjectDeclaration || namedClass - || namedFunctionDeclaration - || namedFunctionExpression + || namedFunctionDeclaration || namedFunctionExpression ) { return true; } @@ -199,31 +199,29 @@ module.exports = { } function isIdentifierShadowed(node, identifierName) { - while (node && node.parent) { - node = node.parent; + let currentNode = node; + while (currentNode && currentNode.parent) { + currentNode = currentNode.parent; if ( - node.type === 'FunctionDeclaration' - || node.type === 'FunctionExpression' - || node.type === 'ArrowFunctionExpression' + currentNode.type === 'FunctionDeclaration' + || currentNode.type === 'FunctionExpression' + || currentNode.type === 'ArrowFunctionExpression' ) { break; } } - if (!node || !node.body) { + if (!currentNode || !currentNode.body) { return false; } - return hasVariableDeclaration(node.body, identifierName); + return hasVariableDeclaration(currentNode.body, identifierName); } - /** - * - * Check is current component shadowed - * @param {ASTNode} node The AST node being checked. - * @returns {boolean} True if component has a name, false if not. + * Checks whether the component wrapper (e.g. React.memo or forwardRef) is shadowed in the current scope. + * @param {ASTNode} node - The CallExpression AST node representing a potential component wrapper. + * @returns {boolean} True if the wrapper identifier (e.g. 'React', 'memo', 'forwardRef') is shadowed, false otherwise. */ - function isShadowedComponent(node) { if (!node || node.type !== 'CallExpression') { return false; @@ -253,10 +251,7 @@ module.exports = { return { ExpressionStatement(node) { if (checkContextObjects && isCreateContext(node)) { - contextObjects.set(node.expression.left.name, { - node, - hasDisplayName: false, - }); + contextObjects.set(node.expression.left.name, { node, hasDisplayName: false }); } }, VariableDeclarator(node) { @@ -320,10 +315,7 @@ module.exports = { if (ignoreTranspilerName || !hasTranspilerName(node)) { // Search for the displayName declaration node.properties.forEach((property) => { - if ( - !property.key - || !propsUtil.isDisplayNameDeclaration(property.key) - ) { + if (!property.key || !propsUtil.isDisplayNameDeclaration(property.key)) { return; } markDisplayNameAsDeclared(node); @@ -338,10 +330,7 @@ module.exports = { return; } - if ( - node.arguments.length > 0 - && astUtil.isFunctionLikeExpression(node.arguments[0]) - ) { + if (node.arguments.length > 0 && astUtil.isFunctionLikeExpression(node.arguments[0])) { // Skip over React.forwardRef declarations that are embedded within // a React.memo i.e. React.memo(React.forwardRef(/* ... */)) // This means that we raise a single error for the call to React.memo diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 419c6b869d..82fea3526e 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -970,8 +970,7 @@ ruleTester.run('display-name', rule, { errors: [ { messageId: 'noDisplayName', - }, - ], + }], }, { code: ` @@ -1178,9 +1177,9 @@ ruleTester.run('display-name', rule, { errors: [{ messageId: 'noDisplayName' }], }, { - // Only trigger an error for the outer React.memo, - // if the React version is not in the following range: - // ^0.14.10 || ^15.7.0 || >= 16.12.0 + // Only trigger an error for the outer React.memo, + // if the React version is not in the following range: + // ^0.14.10 || ^15.7.0 || >= 16.12.0 code: ` import React from 'react' @@ -1193,8 +1192,7 @@ ruleTester.run('display-name', rule, { errors: [ { messageId: 'noDisplayName', - }, - ], + }], settings: { react: { version: '15.6.0', @@ -1202,9 +1200,9 @@ ruleTester.run('display-name', rule, { }, }, { - // Only trigger an error for the outer React.memo, - // if the React version is not in the following range: - // ^0.14.10 || ^15.7.0 || >= ^16.12.0 + // Only trigger an error for the outer React.memo, + // if the React version is not in the following range: + // ^0.14.10 || ^15.7.0 || >= ^16.12.0 code: ` import React from 'react' @@ -1222,9 +1220,9 @@ ruleTester.run('display-name', rule, { }, }, { - // React does not handle the result of forwardRef being passed into memo - // ComponentWithMemoAndForwardRef gets shown as Memo(Anonymous) - // See https://github.com/facebook/react/issues/16722 + // React does not handle the result of forwardRef being passed into memo + // ComponentWithMemoAndForwardRef gets shown as Memo(Anonymous) + // See https://github.com/facebook/react/issues/16722 code: ` import React from 'react' @@ -1351,7 +1349,9 @@ ruleTester.run('display-name', rule, {
{a} {listItem}
); `, - errors: [{ message: 'Component definition is missing display name' }], + errors: [ + { message: 'Component definition is missing display name' }, + ], }, { code: ` From 16e8aefb48dae55d14b13831f145a447368eea72 Mon Sep 17 00:00:00 2001 From: "Hur Hyeon Bin (Max)" <160996936+hyeonbinHur@users.noreply.github.com> Date: Fri, 23 May 2025 15:04:20 +0900 Subject: [PATCH 3/7] Update lib/rules/display-name.js Co-authored-by: Jordan Harband --- lib/rules/display-name.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 19f5283865..64ad7dfe60 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -353,15 +353,8 @@ module.exports = { const list = components.list(); // Report missing display name for all components values(list) - .filter((component) => { - if (isShadowedComponent(component.node)) { - return false; - } - return !component.hasDisplayName; - }) - .forEach((component) => { - reportMissingDisplayName(component); - }); + .filter((component) => !isShadowedComponent(component.node) && !component.hasDisplayName) + .forEach((component) => { reportMissingDisplayName(component); }); if (checkContextObjects) { // Report missing display name for all context objects forEach( From 88092cd676c16037e4083e4189c245f8d2c7123d Mon Sep 17 00:00:00 2001 From: "Hur Hyeon Bin (Max)" <160996936+hyeonbinHur@users.noreply.github.com> Date: Fri, 23 May 2025 15:04:35 +0900 Subject: [PATCH 4/7] Update tests/lib/rules/display-name.js Co-authored-by: Jordan Harband --- tests/lib/rules/display-name.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 82fea3526e..b4e7e97828 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -903,7 +903,7 @@ ruleTester.run('display-name', rule, { import React from 'react' const Comp = React.forwardRef((props, ref) => { - return
test
+ return
test
}) `, errors: [ From 48491b9d24a0cd146f53382db45e242e26a45c10 Mon Sep 17 00:00:00 2001 From: "Hur Hyeon Bin (Max)" <160996936+hyeonbinHur@users.noreply.github.com> Date: Mon, 26 May 2025 19:16:58 +0900 Subject: [PATCH 5/7] fix(display-name): expand shadowing test coverage and functionalities --- lib/rules/display-name.js | 48 ++++++- tests/lib/rules/display-name.js | 242 +++++++++++++++++++++++++++++++- 2 files changed, 279 insertions(+), 11 deletions(-) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 64ad7dfe60..4816304c3f 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -200,22 +200,56 @@ module.exports = { function isIdentifierShadowed(node, identifierName) { let currentNode = node; + while (currentNode && currentNode.parent) { currentNode = currentNode.parent; + if ( currentNode.type === 'FunctionDeclaration' - || currentNode.type === 'FunctionExpression' - || currentNode.type === 'ArrowFunctionExpression' + || currentNode.type === 'FunctionExpression' + || currentNode.type === 'ArrowFunctionExpression' ) { - break; + if (currentNode.body && hasVariableDeclaration(currentNode.body, identifierName)) { + return true; + } } - } - if (!currentNode || !currentNode.body) { - return false; + if (currentNode.type === 'BlockStatement') { + if (hasVariableDeclaration(currentNode, identifierName)) { + return true; + } + } + + if ( + (currentNode.type === 'FunctionDeclaration' + || currentNode.type === 'FunctionExpression' + || currentNode.type === 'ArrowFunctionExpression') + && currentNode.params + ) { + const isParamShadowed = currentNode.params.some((param) => { + if (param.type === 'Identifier' && param.name === identifierName) { + return true; + } + if (param.type === 'ObjectPattern') { + return param.properties.some( + (prop) => prop.type === 'Property' && prop.key && prop.key.name === identifierName + ); + } + if (param.type === 'ArrayPattern') { + return param.elements.some( + (el) => el && el.type === 'Identifier' && el.name === identifierName + ); + } + return false; + }); + + if (isParamShadowed) { + return true; + } + } } - return hasVariableDeclaration(currentNode.body, identifierName); + return false; } /** * Checks whether the component wrapper (e.g. React.memo or forwardRef) is shadowed in the current scope. diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index b4e7e97828..2b98500c85 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -29,15 +29,76 @@ const parserOptions = { const ruleTester = new RuleTester({ parserOptions }); ruleTester.run('display-name', rule, { valid: parsers.all([ + { + code: ` + import React, { memo, forwardRef } from 'react' + + const TestComponent = function () { + { + const memo = (cb) => cb() + const forwardRef = (cb) => cb() + const React = { memo, forwardRef } + const BlockReactShadowedMemo = React.memo(() => { + return
shadowed
+ }) + const BlockShadowedMemo = memo(() => { + return
shadowed
+ }) + const BlockShadowedForwardRef = forwardRef((props, ref) => { + return \`\${props} \${ref}\` + }) + } + return null + } + `, + }, + { + code: ` + import React, { memo } from 'react' + + const TestComponent = function (memo) { + const Comp = memo(() => { + return
param shadowed
+ }) + return Comp + } + `, + }, + { + code: ` + import React, { memo } from 'react' + + const TestComponent = function ({ memo }) { + const Comp = memo(() => { + return
destructured param shadowed
+ }) + return Comp + } + `, + }, + { + code: ` + import React, { memo, forwardRef } from 'react' + + const TestComponent = function () { + function innerFunction() { + const memo = (cb) => cb() + const React = { forwardRef } + const Comp = memo(() =>
nested shadowed
) + const ForwardComp = React.forwardRef(() =>
nested
) + return [Comp, ForwardComp] + } + return innerFunction() + } + `, + }, { code: ` import React, { forwardRef } from 'react' const TestComponent = function () { const { forwardRef } = { forwardRef: () => null } - const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) - return OtherComp } `, @@ -48,11 +109,9 @@ ruleTester.run('display-name', rule, { const TestComponent = function () { const memo = (cb) => cb() - const Comp = memo(() => { return
shadowed
}) - return Comp } `, @@ -898,6 +957,181 @@ ruleTester.run('display-name', rule, { ]), invalid: parsers.all([ + { + code: ` + import React, { memo, forwardRef } from 'react' + + const TestComponent = function () { + { + const BlockReactMemo = React.memo(() => { + return
not shadowed
+ }) + + const BlockMemo = memo(() => { + return
not shadowed
+ }) + + const BlockForwardRef = forwardRef((props, ref) => { + return \`\${props} \${ref}\` + }) + } + + return null + } + `, + errors: [ + { + messageId: 'noDisplayName', + line: 6, + }, + { + messageId: 'noDisplayName', + line: 10, + }, + { + messageId: 'noDisplayName', + line: 14, + }, + ], + }, + + { + code: ` + import React, { memo } from 'react' + + const TestComponent = function () { + const Comp = memo(() => { + return
not param shadowed
+ }) + + return Comp + } + `, + errors: [ + { + messageId: 'noDisplayName', + line: 5, + }, + ], + }, + + { + code: ` + import React, { memo } from 'react' + + const TestComponent = function () { + const Comp = memo(() => { + return
not destructured param shadowed
+ }) + + return Comp + } + `, + errors: [ + { + messageId: 'noDisplayName', + line: 5, + }, + ], + }, + + { + code: ` + import React, { memo, forwardRef } from 'react' + + const TestComponent = function () { + function innerFunction() { + const Comp = memo(() =>
nested not shadowed
) + const ForwardComp = React.forwardRef(() =>
nested
) + + return [Comp, ForwardComp] + } + + return innerFunction() + } + `, + errors: [ + { + messageId: 'noDisplayName', + line: 6, + }, + { + messageId: 'noDisplayName', + line: 7, + }, + ], + }, + { + code: ` + import React, { forwardRef } from 'react' + + const TestComponent = function () { + const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) + + return OtherComp + } + `, + errors: [ + { + messageId: 'noDisplayName', + line: 5, + }, + ], + }, + { + code: ` + import React, { memo } from 'react' + + const TestComponent = function () { + const Comp = memo(() => { + return
not shadowed
+ }) + return Comp + } + `, + errors: [ + { + messageId: 'noDisplayName', + line: 5, + }, + ], + }, + { + code: ` + import React, { memo, forwardRef } from 'react' + + const MixedNotShadowed = function () { + const Comp = memo(() => { + return
not shadowed
+ }) + const ReactMemo = React.memo(() => null) + const ReactForward = React.forwardRef((props, ref) => { + return \`\${props} \${ref}\` + }) + const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) + + return [Comp, ReactMemo, ReactForward, OtherComp] + } + `, + errors: [ + { + messageId: 'noDisplayName', + line: 5, + }, + { + messageId: 'noDisplayName', + line: 8, + }, + { + messageId: 'noDisplayName', + line: 9, + }, + { + messageId: 'noDisplayName', + line: 12, + }, + ], + }, { code: ` import React from 'react' From 513cdf9de694d88d5f0e46bec338c9ad33bf2d8d Mon Sep 17 00:00:00 2001 From: "Hur Hyeon Bin (Max)" <160996936+hyeonbinHur@users.noreply.github.com> Date: Mon, 26 May 2025 19:44:34 +0900 Subject: [PATCH 6/7] fix(display-name): optimize shadowing test cases --- tests/lib/rules/display-name.js | 266 ++++---------------------------- 1 file changed, 32 insertions(+), 234 deletions(-) diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 2b98500c85..44b271de55 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -32,21 +32,16 @@ ruleTester.run('display-name', rule, { { code: ` import React, { memo, forwardRef } from 'react' - + const TestComponent = function () { { const memo = (cb) => cb() const forwardRef = (cb) => cb() const React = { memo, forwardRef } - const BlockReactShadowedMemo = React.memo(() => { - return
shadowed
- }) - const BlockShadowedMemo = memo(() => { - return
shadowed
- }) - const BlockShadowedForwardRef = forwardRef((props, ref) => { - return \`\${props} \${ref}\` - }) + + const BlockMemo = memo(() =>
shadowed
) + const BlockForwardRef = forwardRef(() =>
shadowed
) + const BlockReactMemo = React.memo(() =>
shadowed
) } return null } @@ -54,37 +49,27 @@ ruleTester.run('display-name', rule, { }, { code: ` - import React, { memo } from 'react' - - const TestComponent = function (memo) { - const Comp = memo(() => { - return
param shadowed
- }) - return Comp + import React, { memo, forwardRef } from 'react' + + const Test1 = function (memo) { + return memo(() =>
param shadowed
) } - `, - }, - { - code: ` - import React, { memo } from 'react' - - const TestComponent = function ({ memo }) { - const Comp = memo(() => { - return
destructured param shadowed
- }) - return Comp + + const Test2 = function ({ forwardRef }) { + return forwardRef(() =>
destructured param
) } `, }, { code: ` import React, { memo, forwardRef } from 'react' - + const TestComponent = function () { function innerFunction() { const memo = (cb) => cb() const React = { forwardRef } - const Comp = memo(() =>
nested shadowed
) + + const Comp = memo(() =>
nested
) const ForwardComp = React.forwardRef(() =>
nested
) return [Comp, ForwardComp] } @@ -92,30 +77,6 @@ ruleTester.run('display-name', rule, { } `, }, - { - code: ` - import React, { forwardRef } from 'react' - - const TestComponent = function () { - const { forwardRef } = { forwardRef: () => null } - const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) - return OtherComp - } - `, - }, - { - code: ` - import React, { memo } from 'react' - - const TestComponent = function () { - const memo = (cb) => cb() - const Comp = memo(() => { - return
shadowed
- }) - return Comp - } - `, - }, { code: ` import React, { memo, forwardRef } from 'react' @@ -125,13 +86,9 @@ ruleTester.run('display-name', rule, { const { forwardRef } = { forwardRef: () => null } const [React] = [{ memo, forwardRef }] - const Comp = memo(() => { - return
shadowed
- }) + const Comp = memo(() =>
shadowed
) const ReactMemo = React.memo(() => null) - const ReactForward = React.forwardRef((props, ref) => { - return \`\${props} \${ref}\` - }) + const ReactForward = React.forwardRef((props, ref) => \`\${props} \${ref}\`) const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) return [Comp, ReactMemo, ReactForward, OtherComp] @@ -960,7 +917,7 @@ ruleTester.run('display-name', rule, { { code: ` import React, { memo, forwardRef } from 'react' - + const TestComponent = function () { { const BlockReactMemo = React.memo(() => { @@ -980,120 +937,33 @@ ruleTester.run('display-name', rule, { } `, errors: [ - { - messageId: 'noDisplayName', - line: 6, - }, - { - messageId: 'noDisplayName', - line: 10, - }, - { - messageId: 'noDisplayName', - line: 14, - }, + { messageId: 'noDisplayName' }, + { messageId: 'noDisplayName' }, + { messageId: 'noDisplayName' } ], }, - { code: ` - import React, { memo } from 'react' - - const TestComponent = function () { - const Comp = memo(() => { - return
not param shadowed
- }) - - return Comp - } - `, - errors: [ - { - messageId: 'noDisplayName', - line: 5, - }, - ], - }, + import React, { memo, forwardRef } from 'react' - { - code: ` - import React, { memo } from 'react' - - const TestComponent = function () { - const Comp = memo(() => { - return
not destructured param shadowed
- }) - + const Test1 = function () { + const Comp = memo(() =>
not param shadowed
) return Comp } - `, - errors: [ - { - messageId: 'noDisplayName', - line: 5, - }, - ], - }, - { - code: ` - import React, { memo, forwardRef } from 'react' - - const TestComponent = function () { + const Test2 = function () { function innerFunction() { const Comp = memo(() =>
nested not shadowed
) const ForwardComp = React.forwardRef(() =>
nested
) - return [Comp, ForwardComp] } - return innerFunction() } `, errors: [ - { - messageId: 'noDisplayName', - line: 6, - }, - { - messageId: 'noDisplayName', - line: 7, - }, - ], - }, - { - code: ` - import React, { forwardRef } from 'react' - - const TestComponent = function () { - const OtherComp = forwardRef((props, ref) => \`\${props} \${ref}\`) - - return OtherComp - } - `, - errors: [ - { - messageId: 'noDisplayName', - line: 5, - }, - ], - }, - { - code: ` - import React, { memo } from 'react' - - const TestComponent = function () { - const Comp = memo(() => { - return
not shadowed
- }) - return Comp - } - `, - errors: [ - { - messageId: 'noDisplayName', - line: 5, - }, + { messageId: 'noDisplayName' }, + { messageId: 'noDisplayName' }, + { messageId: 'noDisplayName' } ], }, { @@ -1114,82 +984,10 @@ ruleTester.run('display-name', rule, { } `, errors: [ - { - messageId: 'noDisplayName', - line: 5, - }, - { - messageId: 'noDisplayName', - line: 8, - }, - { - messageId: 'noDisplayName', - line: 9, - }, - { - messageId: 'noDisplayName', - line: 12, - }, - ], - }, - { - code: ` - import React from 'react' - - const Comp = React.forwardRef((props, ref) => { - return
test
- }) - `, - errors: [ - { - messageId: 'noDisplayName', - line: 4, - }, - ], - }, - { - code: ` - import {forwardRef} from 'react' - - const Comp = forwardRef((props, ref) => { - return
test
- }) - `, - errors: [ - { - messageId: 'noDisplayName', - line: 4, - }, - ], - }, - { - code: ` - import React from 'react' - - const Comp = React.memo(() => { - return
test
- }) - `, - errors: [ - { - messageId: 'noDisplayName', - line: 4, - }, - ], - }, - { - code: ` - import { memo } from 'react' - - const Comp = memo(() => { - return
test
- }) - `, - errors: [ - { - messageId: 'noDisplayName', - line: 4, - }, + { messageId: 'noDisplayName' }, + { messageId: 'noDisplayName' }, + { messageId: 'noDisplayName' }, + { messageId: 'noDisplayName' } ], }, { From f6800d3f7c427a1225375e14e9c7daff083c2927 Mon Sep 17 00:00:00 2001 From: "Hur Hyeon Bin (Max)" <160996936+hyeonbinHur@users.noreply.github.com> Date: Mon, 26 May 2025 20:02:25 +0900 Subject: [PATCH 7/7] Update display-name.js --- lib/rules/display-name.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 4816304c3f..9c812767be 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -206,8 +206,8 @@ module.exports = { if ( currentNode.type === 'FunctionDeclaration' - || currentNode.type === 'FunctionExpression' - || currentNode.type === 'ArrowFunctionExpression' + || currentNode.type === 'FunctionExpression' + || currentNode.type === 'ArrowFunctionExpression' ) { if (currentNode.body && hasVariableDeclaration(currentNode.body, identifierName)) { return true; @@ -222,9 +222,9 @@ module.exports = { if ( (currentNode.type === 'FunctionDeclaration' - || currentNode.type === 'FunctionExpression' - || currentNode.type === 'ArrowFunctionExpression') - && currentNode.params + || currentNode.type === 'FunctionExpression' + || currentNode.type === 'ArrowFunctionExpression') + && currentNode.params ) { const isParamShadowed = currentNode.params.some((param) => { if (param.type === 'Identifier' && param.name === identifierName) {