Skip to content

Commit

Permalink
[patch] no-unsafe: report on the method instead of the entire compo…
Browse files Browse the repository at this point in the history
…nent

This also makes error ordering more consistent since certain eslint/node combinations reverse the ordering
  • Loading branch information
ljharb committed Aug 15, 2023
1 parent d5178be commit 3be1d19
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

### Fixed
* [`no-deprecated`]: prevent false positive on commonjs import ([#3614][] @akulsr0)
* [`no-unsafe`]: report on the method instead of the entire component (@ljharb)

[#3614]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3614

Expand Down
9 changes: 7 additions & 2 deletions lib/rules/no-unsafe.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,11 @@ module.exports = {
const newMethod = meta.newMethod;
const details = meta.details;

const propertyNode = astUtil.getComponentProperties(node)
.find((property) => astUtil.getPropertyName(property) === method);

report(context, messages.unsafeMethod, 'unsafeMethod', {
node,
node: propertyNode,
data: {
method,
newMethod,
Expand All @@ -135,7 +138,9 @@ module.exports = {
function checkLifeCycleMethods(node) {
if (componentUtil.isES5Component(node, context) || componentUtil.isES6Component(node, context)) {
const methods = getLifeCycleMethods(node);
methods.forEach((method) => checkUnsafe(node, method));
methods
.sort((a, b) => a.localeCompare(b))
.forEach((method) => checkUnsafe(node, method));
}
}

Expand Down
72 changes: 36 additions & 36 deletions tests/lib/rules/no-unsafe.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidMount',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 9,
type: 'ClassDeclaration',
line: 3,
column: 11,
type: 'MethodDefinition',
},
{
messageId: 'unsafeMethod',
Expand All @@ -162,9 +162,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'getDerivedStateFromProps',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 9,
type: 'ClassDeclaration',
line: 4,
column: 11,
type: 'MethodDefinition',
},
{
messageId: 'unsafeMethod',
Expand All @@ -173,9 +173,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidUpdate',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 9,
type: 'ClassDeclaration',
line: 5,
column: 11,
type: 'MethodDefinition',
},
],
},
Expand All @@ -196,9 +196,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidMount',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 9,
type: 'ClassDeclaration',
line: 3,
column: 11,
type: 'MethodDefinition',
},
{
messageId: 'unsafeMethod',
Expand All @@ -207,9 +207,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'getDerivedStateFromProps',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 9,
type: 'ClassDeclaration',
line: 4,
column: 11,
type: 'MethodDefinition',
},
{
messageId: 'unsafeMethod',
Expand All @@ -218,9 +218,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidUpdate',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 9,
type: 'ClassDeclaration',
line: 5,
column: 11,
type: 'MethodDefinition',
},
],
},
Expand All @@ -243,9 +243,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidMount',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 38,
type: 'ObjectExpression',
line: 3,
column: 11,
type: 'Property',
},
{
messageId: 'unsafeMethod',
Expand All @@ -254,9 +254,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'getDerivedStateFromProps',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 38,
type: 'ObjectExpression',
line: 4,
column: 11,
type: 'Property',
},
{
messageId: 'unsafeMethod',
Expand All @@ -265,9 +265,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidUpdate',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 38,
type: 'ObjectExpression',
line: 5,
column: 11,
type: 'Property',
},
],
},
Expand All @@ -288,9 +288,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidMount',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 38,
type: 'ObjectExpression',
line: 3,
column: 11,
type: 'Property',
},
{
messageId: 'unsafeMethod',
Expand All @@ -299,9 +299,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'getDerivedStateFromProps',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 38,
type: 'ObjectExpression',
line: 4,
column: 11,
type: 'Property',
},
{
messageId: 'unsafeMethod',
Expand All @@ -310,9 +310,9 @@ ruleTester.run('no-unsafe', rule, {
newMethod: 'componentDidUpdate',
details: 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.',
},
line: 2,
column: 38,
type: 'ObjectExpression',
line: 5,
column: 11,
type: 'Property',
},
],
},
Expand Down

0 comments on commit 3be1d19

Please sign in to comment.