Skip to content

[Fix] display-name: avoid false positive when React is shadowed #3926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[Fix] display-name: avoid false positive when React is shadowed #3926

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
30f0022
[Fix] `display-name`: avoid false positive when React is shadowed
hyeonbinHur May 22, 2025
6e05536
style: fix lint,prettier errors
hyeonbinHur May 22, 2025
16e8aef
Update lib/rules/display-name.js
hyeonbinHur May 23, 2025
88092cd
Update tests/lib/rules/display-name.js
hyeonbinHur May 23, 2025
48491b9
fix(display-name): expand shadowing test coverage and functionalities
hyeonbinHur May 26, 2025
513cdf9
fix(display-name): optimize shadowing test cases
hyeonbinHur May 26, 2025
f6800d3
Update display-name.js
hyeonbinHur May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 147 additions & 46 deletions lib/rules/display-name.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,32 +31,36 @@
module.exports = {
meta: {
docs: {
description: 'Disallow missing displayName in a React component definition',
description:
'Disallow missing displayName in a React component definition',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please don't run prettier on projects that don't use it :-) and please revert all unrelated style changes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I forgot to disable some style-related settings, which caused unnecessary formatting changes. I’ll remove the unnecessary style edits and commit again soon. Thanks for the review and the feedback!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion is to keep those always disabled, and only use eslint to format files.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except eslint recommends to use formatters instead.

category: 'Best Practices',
recommended: true,
url: docsUrl('display-name'),
},

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();

Expand All @@ -76,10 +80,12 @@
* @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)
);
}

/**
Expand Down Expand Up @@ -115,60 +121,142 @@
* @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;

Check warning on line 173 in lib/rules/display-name.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/display-name.js#L173 

Added line #L173 was not covered by tests
}

// 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;

Check warning on line 214 in lib/rules/display-name.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/display-name.js#L214 

Added line #L214 was not covered by tests
}

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
// --------------------------------------------------------------------------

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) {
Expand Down Expand Up @@ -232,7 +320,10 @@
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);
Expand All @@ -247,7 +338,10 @@
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
Expand All @@ -269,9 +363,16 @@
'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(
Expand Down
Loading