[patch] no-unsafe: report on the method instead of the entire compo…

…nent This also makes error ordering more consistent since certain eslint/node combinations reverse the ordering
HenryBrown0 · Aug 15, 2023 · 3be1d19 · 3be1d19
1 parent d5178be
commit 3be1d19
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
 

diff --git a/lib/rules/no-unsafe.js b/lib/rules/no-unsafe.js
@@ -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,
@@ -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));
       }
     }
 

diff --git a/tests/lib/rules/no-unsafe.js b/tests/lib/rules/no-unsafe.js
@@ -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',
@@ -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',
@@ -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',
         },
       ],
     },
@@ -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',
@@ -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',
@@ -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',
         },
       ],
     },
@@ -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',
@@ -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',
@@ -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',
         },
       ],
     },
@@ -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',
@@ -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',
@@ -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',
         },
       ],
     },