fix(no-derived-state): account for non-calls when flagging single setter calls

Author: NickvanDyke

src/no-derived-state.js

@@ -10,9 +10,9 @@ import {
   isHOCProp,
   getUpstreamReactVariables,
   isState,
+  countCalls,
 } from "./util/react.js";
 import { getCallExpr, getDownstreamRefs } from "./util/ast.js";
-import { arraysEqual } from "./util/javascript.js";
 
 export const name = "no-derived-state";
 export const messages = {
@@ -48,36 +48,26 @@ export const rule = {
           const stateName = (
             useStateNode.id.elements[0] ?? useStateNode.id.elements[1]
           )?.name;
-          const argsRefs = callExpr.arguments.flatMap((arg) =>
-            getDownstreamRefs(context, arg),
-          );
-          const argsUpstreamVariables = argsRefs.flatMap((ref) =>
+
+          const argsUpstreamVars = callExpr.arguments
+            .flatMap((arg) => getDownstreamRefs(context, arg))
+            .flatMap((ref) => getUpstreamReactVariables(context, ref.resolved));
+          const depsUpstreamVars = depsRefs.flatMap((ref) =>
             getUpstreamReactVariables(context, ref.resolved),
           );
-          const isAllArgsInternal = argsUpstreamVariables.notEmptyEvery(
+          const isAllArgsInternal = argsUpstreamVars.notEmptyEvery(
             (variable) =>
               isState(variable) || (isProp(variable) && !isHOCProp(variable)),
           );
-          const isAllArgsInDeps = argsRefs
-            .filter((ref) =>
-              ref.resolved.defs.every((def) => def.type !== "Parameter"),
-            )
-            .notEmptyEvery((argRef) =>
-              depsRefs.some((depRef) =>
-                arraysEqual(
-                  getUpstreamReactVariables(context, argRef.resolved),
-                  getUpstreamReactVariables(context, depRef.resolved),
-                ),
-              ),
-            );
-          // In this case the derived state will always be in sync,
-          // thus it could be computed directly during render
-          const isStateSetterCalledOnce =
-            ref.resolved.references.length - 1 === 1;
+          const isAllArgsInDeps = argsUpstreamVars.notEmptyEvery((argVar) =>
+            depsUpstreamVars.some((depVar) => argVar.name === depVar.name),
+          );
 
           if (
             isAllArgsInternal ||
-            (isAllArgsInDeps && isStateSetterCalledOnce)
+            // In this case the derived state will always be in sync,
+            // thus it could be computed directly during render
+            (isAllArgsInDeps && countCalls(ref) === 1)
           ) {
             context.report({
               node: callExpr,

src/util/javascript.js

@@ -1,10 +1,3 @@
 Array.prototype.notEmptyEvery = function (predicate) {
   return this.length > 0 && this.every(predicate);
 };
-
-export const arraysEqual = (arr1, arr2) => {
-  if (arr1.length !== arr2.length) {
-    return false;
-  }
-  return arr1.every((element, index) => element === arr2[index]);
-};

src/util/react.js

@@ -242,8 +242,10 @@ const countUseStates = (context, componentNode) => {
   return count;
 };
 
-export const countStateSetterCalls = (stateSetterRef) =>
-  stateSetterRef.resolved.references.length - 1; // -1 for the initial declaration
+export const countCalls = (ref) =>
+  ref.resolved.references.filter(
+    (ref) => ref.identifier.parent.type === "CallExpression",
+  ).length;
 
 // Returns the component or custom hook that contains the `useEffect` node
 const findContainingNode = (node) => {

test/no-derived-state.test.js

@@ -147,20 +147,6 @@ new MyRuleTester().run(name, rule, {
         }
       `,
     },
-    {
-      name: "From external state via member function",
-      code: js`
-        function Counter() {
-          const countGetter = useSomeAPI();
-          const [count, setCount] = useState(0);
-
-          useEffect(() => {
-            const newCount = countGetter.getCount();
-            setCount(newCount);
-          }, [countGetter, setCount]);
-        }
-      `,
-    },
     {
       name: "Via pure local function",
       code: js`
@@ -445,7 +431,7 @@ new MyRuleTester().run(name, rule, {
 
           useEffect(() => {
             setSelectedPost(posts[0]);
-          }, [posts]);
+          }, [posts, setSelectedPost]);
         }
       `,
       errors: [
@@ -465,7 +451,7 @@ new MyRuleTester().run(name, rule, {
           useEffect(() => {
             const prefixedName = 'Dr. ' + name;
             setFullName(prefixedName) 
-          }, [name]);
+          }, [name, setFullName]);
         }
       `,
       errors: [
@@ -486,7 +472,7 @@ new MyRuleTester().run(name, rule, {
 
           useEffect(() => {
             setLocation(history.location);
-          }, [history.location]);
+          }, [history.location, setLocation]);
         });
       `,
       errors: [