bug: false negatives (rare) - does not analyze derived `FunctionDeclaration` bodies

[NickvanDyke]

Did you check docs and existing issues?

• I have read the plugin docs
• I have searched the existing issues
• I have read You Might Not Need An Effect and am reasonably confident that I do need an effect

Plugin version

0.5.1

ESLint version

9.0.0

Describe the bug

no-derived-state (and getUpstreamVariables, specifically) does not descend into local functions to determine whether the function is "internal" (i.e. pure and only references state and/or props).

Initially I avoided this because it was too unreliable, but now that I know better and the plugin has better internals, it shouuuld be possible.

I believe we still have to skip functions imported from other files - we don't have their implementation available to analyze.

Thanks to @TkDodo for reminding me of this 🙂

Steps To Reproduce

Lint the following code:

useEffect(() => {  
  // ❌ no error here  
  setNames(computeNames(firstName, lastName))  
  // ✅ this errors  
  setNames(firstName + lastName)  
}, [firstName, lastName])

Expected Behavior

Should receive message to avoid derived state on names.

[NickvanDyke]

Ah, looking at existing test cases more closely, I think this is covered, but only when the local function is a VariableDeclaration, i.e. const computeNames = () => { ... }. getUpstreamVariables does not descend into FunctionDeclaration nodes.

[NickvanDyke]

Ah the real source is a little more complicated than that. It does descend into such functions, but some faulty generalized logic considers functions that only reference their params to be impure. Still unclear why that's not the case for arrow functions. 🤔

[NickvanDyke]

Very related to #38