Merge pull request #1829 from alexzherdev/1759-state-parameter-names

Don't depend on state parameter name in no-unused-state
jsx-eslint · Oct 1, 2018 · d0765dd · d0765dd
2 parents b77be96 + 1ecb622
commit d0765dd
Show file tree

Hide file tree

Showing 2 changed files with 127 additions and 14 deletions.
diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js
@@ -77,7 +77,38 @@ module.exports = {
     // JSX attributes), then this is again set to null.
     let classInfo = null;
 
-    // Returns true if the given node is possibly a reference to `this.state`, `prevState` or `nextState`.
+    function isStateParameterReference(node) {
+      const classMethods = [
+        'shouldComponentUpdate',
+        'componentWillUpdate',
+        'UNSAFE_componentWillUpdate',
+        'getSnapshotBeforeUpdate',
+        'componentDidUpdate'
+      ];
+
+      let scope = context.getScope();
+      while (scope) {
+        const parent = scope.block && scope.block.parent;
+        if (
+          parent &&
+          parent.type === 'MethodDefinition' && (
+            parent.static && parent.key.name === 'getDerivedStateFromProps' ||
+            classMethods.indexOf(parent.key.name !== -1)
+          ) &&
+          parent.value.type === 'FunctionExpression' &&
+          parent.value.params[1] &&
+          parent.value.params[1].name === node.name
+        ) {
+          return true;
+        }
+        scope = scope.upper;
+      }
+
+      return false;
+    }
+
+    // Returns true if the given node is possibly a reference to `this.state` or the state parameter of
+    // a lifecycle method.
     function isStateReference(node) {
       node = uncast(node);
 
@@ -91,15 +122,7 @@ module.exports = {
         classInfo.aliases &&
         classInfo.aliases.has(node.name);
 
-      const isPrevStateReference =
-        node.type === 'Identifier' &&
-        node.name === 'prevState';
-
-      const isNextStateReference =
-        node.type === 'Identifier' &&
-        node.name === 'nextState';
-
-      return isDirectStateReference || isAliasedStateReference || isPrevStateReference || isNextStateReference;
+      return isDirectStateReference || isAliasedStateReference || isStateParameterReference(node);
     }
 
     // Takes an ObjectExpression node and adds all named Property nodes to the

diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js
@@ -492,15 +492,15 @@ eslintTester.run('no-unused-state', rule, {
       parser: 'babel-eslint'
     },
     {
-      code: `class ESLintExample extends Component {
+      code: `class GetDerivedStateFromPropsTest extends Component {
         constructor(props) {
           super(props);
           this.state = {
             id: 123,
           };
         }
-        static getDerivedStateFromProps(nextProps, prevState) {
-          if (prevState.id === nextProps.id) {
+        static getDerivedStateFromProps(nextProps, otherState) {
+          if (otherState.id === nextProps.id) {
             return {
               selected: true,
             };
@@ -516,7 +516,29 @@ eslintTester.run('no-unused-state', rule, {
       parser: 'babel-eslint'
     },
     {
-      code: `class ESLintExample extends Component {
+      code: `class ComponentDidUpdateTest extends Component {
+        constructor(props) {
+          super(props);
+          this.state = {
+            id: 123,
+          };
+        }
+
+        componentDidUpdate(someProps, someState) {
+          if (someState.id === someProps.id) {
+            doStuff();
+          }
+        }
+        render() {
+          return (
+            <h1>{this.state.selected ? 'Selected' : 'Not selected'}</h1>
+          );
+        }
+      }`,
+      parser: 'babel-eslint'
+    },
+    {
+      code: `class ShouldComponentUpdateTest extends Component {
         constructor(props) {
           super(props);
           this.state = {
@@ -533,6 +555,27 @@ eslintTester.run('no-unused-state', rule, {
         }
       }`,
       parser: 'babel-eslint'
+    },
+    {
+      code: `class NestedScopesTest extends Component {
+        constructor(props) {
+          super(props);
+          this.state = {
+            id: 123,
+          };
+        }
+        shouldComponentUpdate(nextProps, nextState) {
+          return (function() {
+            return nextState.id === nextProps.id;
+          })();
+        }
+        render() {
+          return (
+            <h1>{this.state.selected ? 'Selected' : 'Not selected'}</h1>
+          );
+        }
+      }`,
+      parser: 'babel-eslint'
     }
   ],
 
@@ -824,6 +867,53 @@ eslintTester.run('no-unused-state', rule, {
         }`,
       errors: getErrorMessages(['bar']),
       parser: 'babel-eslint'
+    },
+    {
+      code: `class FakePrevStateVariableTest extends Component {
+        constructor(props) {
+          super(props);
+          this.state = {
+            id: 123,
+            foo: 456
+          };
+        }
+
+        componentDidUpdate(someProps, someState) {
+          if (someState.id === someProps.id) {
+            const prevState = { foo: 789 };
+            console.log(prevState.foo);
+          }
+        }
+        render() {
+          return (
+            <h1>{this.state.selected ? 'Selected' : 'Not selected'}</h1>
+          );
+        }
+      }`,
+      errors: getErrorMessages(['foo']),
+      parser: 'babel-eslint'
+    },
+    {
+      code: `class MissingStateParameterTest extends Component {
+        constructor(props) {
+          super(props);
+          this.state = {
+            id: 123
+          };
+        }
+
+        componentDidUpdate(someProps) {
+          const prevState = { id: 456 };
+          console.log(prevState.id);
+        }
+        render() {
+          return (
+            <h1>{this.state.selected ? 'Selected' : 'Not selected'}</h1>
+          );
+        }
+      }`,
+      errors: getErrorMessages(['id']),
+      parser: 'babel-eslint'
     }
   ]
 });