From 543df6e3a8909899c0fc51d2de40fcd7febf7df7 Mon Sep 17 00:00:00 2001 From: camc314 <18101008+camc314@users.noreply.github.com> Date: Tue, 3 Dec 2024 23:54:21 +0000 Subject: [PATCH] fix(linter): fix false positives in exhaustive-deps (#7615) part of https://github.com/oxc-project/oxc/issues/7246 --- .../src/rules/react/exhaustive_deps.rs | 69 ++++++++++--------- .../src/snapshots/react_exhaustive_deps.snap | 34 +++++---- 2 files changed, 59 insertions(+), 44 deletions(-) diff --git a/crates/oxc_linter/src/rules/react/exhaustive_deps.rs b/crates/oxc_linter/src/rules/react/exhaustive_deps.rs index e678663ca37e4..72725f22f1a21 100644 --- a/crates/oxc_linter/src/rules/react/exhaustive_deps.rs +++ b/crates/oxc_linter/src/rules/react/exhaustive_deps.rs @@ -1032,22 +1032,30 @@ impl<'a> Visit<'a> for ExhaustiveDepsVisitor<'a, '_> { return; } + let is_parent_call_expr = self + .stack + .get(self.stack.len() - 2) + .is_some_and(|kind| matches!(kind, AstKind::CallExpression(_))); + match analyze_property_chain(&it.object, self.semantic) { Ok(source) => { if let Some(source) = source { - let new_chain = Vec::from([it.property.name.clone()]); - - self.found_dependencies.insert(Dependency { - name: source.name.clone(), - reference_id: source.reference_id, - span: source.span, - chain: [source.chain, new_chain].concat(), - symbol_id: self - .semantic - .symbols() - .get_reference(source.reference_id) - .symbol_id(), - }); + if is_parent_call_expr { + self.found_dependencies.insert(source); + } else { + let new_chain = Vec::from([it.property.name.clone()]); + self.found_dependencies.insert(Dependency { + name: source.name.clone(), + reference_id: source.reference_id, + span: source.span, + chain: [source.chain.clone(), new_chain].concat(), + symbol_id: self + .semantic + .symbols() + .get_reference(source.reference_id) + .symbol_id(), + }); + } } let cur_skip_reporting_dependency = self.skip_reporting_dependency; @@ -1612,16 +1620,15 @@ fn test() { }, []); return
; }", - // TODO: fix, this shouldn't report because `myRef` comes from props - // r"function useMyThing(myRef) { - // useEffect(() => { - // const handleMove = () => {}; - // myRef.current = {}; - // return () => { - // console.log(myRef.current.toString()) - // }; - // }, [myRef]); - // }", + r"function useMyThing(myRef) { + useEffect(() => { + const handleMove = () => {}; + myRef.current = {}; + return () => { + console.log(myRef.current.toString()) + }; + }, [myRef]); + }", r"function MyComponent() { const myRef = useRef(); useEffect(() => { @@ -2631,14 +2638,14 @@ fn test() { } }, []); }", - // r"function MyComponent(props) { - // const [skillsCount] = useState(); - // useEffect(() => { - // if (skillsCount === 0 && !props.isEditMode) { - // props.toggleEditMode(); - // } - // }, [skillsCount, props.isEditMode, props.toggleEditMode]); - // }", + r"function MyComponent(props) { + const [skillsCount] = useState(); + useEffect(() => { + if (skillsCount === 0 && !props.isEditMode) { + props.toggleEditMode(); + } + }, [skillsCount, props.isEditMode, props.toggleEditMode]); + }", r"function MyComponent(props) { const [skillsCount] = useState(); useEffect(() => { diff --git a/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap b/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap index 6be9889e18f36..7b25f6b42c264 100644 --- a/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap +++ b/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap @@ -1,8 +1,7 @@ --- source: crates/oxc_linter/src/tester.rs -snapshot_kind: text --- - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useCallback has a missing dependency: 'props.foo.toString' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useCallback has a missing dependency: 'props.foo' ╭─[exhaustive_deps.tsx:4:14] 3 │ console.log(props.foo?.toString()); 4 │ }, []); @@ -29,7 +28,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useCallback has a missing dependency: 'props.foo.bar.toString' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useCallback has a missing dependency: 'props.foo.bar' ╭─[exhaustive_deps.tsx:4:14] 3 │ console.log(props.foo?.bar.toString()); 4 │ }, []); @@ -299,7 +298,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'history.listen' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'history' ╭─[exhaustive_deps.tsx:4:14] 3 │ return history.listen(); 4 │ }, []); @@ -308,7 +307,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'history.foo.bar' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'history.foo', and 'history.foo.bar' ╭─[exhaustive_deps.tsx:7:14] 6 │ ]; 7 │ }, []); @@ -1035,7 +1034,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'props.onChange' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.onChange', and 'props' ╭─[exhaustive_deps.tsx:6:14] 5 │ } 6 │ }, []); @@ -1044,7 +1043,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'props.onChange' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.onChange', and 'props' ╭─[exhaustive_deps.tsx:6:14] 5 │ } 6 │ }, []); @@ -1053,7 +1052,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.onPause', and 'props.onPlay' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'props' ╭─[exhaustive_deps.tsx:9:14] 8 │ } 9 │ }, []); @@ -1062,7 +1061,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'props.foo.onChange' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.foo.onChange', and 'props.foo' ╭─[exhaustive_deps.tsx:6:14] 5 │ } 6 │ }, []); @@ -1071,7 +1070,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.onChange', and 'props.foo.onChange' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.foo', 'props', and 'props.foo.onChange' ╭─[exhaustive_deps.tsx:7:14] 6 │ } 7 │ }, []); @@ -1080,7 +1079,16 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'skillsCount', 'props.toggleEditMode', and 'props.isEditMode' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'props' + ╭─[exhaustive_deps.tsx:7:14] + 6 │ } + 7 │ }, [skillsCount, props.isEditMode, props.toggleEditMode]); + · ───────────────────────────────────────────────────── + 8 │ } + ╰──── + help: Either include it or remove the dependency array. + + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'skillsCount', 'props.isEditMode', and 'props' ╭─[exhaustive_deps.tsx:7:14] 6 │ } 7 │ }, []); @@ -1089,7 +1097,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.onChange', and 'props' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'props' ╭─[exhaustive_deps.tsx:5:14] 4 │ props.onChange(); 5 │ }, []); @@ -1098,7 +1106,7 @@ snapshot_kind: text ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'props.onChange', and 'props' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'props' ╭─[exhaustive_deps.tsx:5:14] 4 │ externalCall(props); 5 │ }, []);