diff --git a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs index 0e5c33111..60c4b6a10 100644 --- a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs +++ b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs @@ -42,7 +42,7 @@ mod diagnostics { .with_label(span) } - pub(super) fn look_hook(span: Span, hook_name: &str) -> D { + pub(super) fn loop_hook(span: Span, hook_name: &str) -> D { D::error(format!( "eslint-plugin-react-hooks(rules-of-hooks): \ React Hook {hook_name:?} may be executed more than once. Possibly \ @@ -237,8 +237,8 @@ impl Rule for RulesOfHooks { } // Is this node cyclic? - if self.is_cyclic(ctx, node_cfg_ix) { - return ctx.diagnostic(diagnostics::look_hook(span, hook_name)); + if self.is_cyclic(ctx, node, parent_func) { + return ctx.diagnostic(diagnostics::loop_hook(span, hook_name)); } if self.is_conditional(ctx, func_cfg_ix, node_cfg_ix) @@ -254,12 +254,19 @@ impl Rule for RulesOfHooks { impl RulesOfHooks { #![allow(clippy::unused_self, clippy::inline_always)] #[inline(always)] - fn is_cyclic(&self, ctx: &LintContext, node_cfg_ix: NodeIndex) -> bool { - let graph = &ctx.semantic().cfg().graph; - petgraph::algo::dijkstra(graph, node_cfg_ix, None, |_| 0) - .into_keys() - .flat_map(|it| graph.edges_directed(it, petgraph::Direction::Outgoing)) - .any(|edge| matches!(edge.weight(), EdgeType::Backedge)) + fn is_cyclic(&self, ctx: &LintContext, node: &AstNode, func: &AstNode) -> bool { + // TODO: use cfg instead + ctx.nodes().ancestors(node.id()).take_while(|id| *id != func.id()).any(|id| { + let maybe_loop = ctx.nodes().kind(id); + matches! { + maybe_loop, + | AstKind::DoWhileStatement(_) + | AstKind::ForInStatement(_) + | AstKind::ForOfStatement(_) + | AstKind::ForStatement(_) + | AstKind::WhileStatement(_) + } + }) } #[inline(always)] @@ -832,12 +839,40 @@ fn test() { } ", " - function useLabeledBlock() { - label: { - useHook(); - if (a) break label; - } + function useLabeledBlock() { + label: { + useHook(); + if (a) break label; } + } + ", + " + export const FalsePositive = ({ editor, anchorElem, isLink, linkNodeUrl, close }: Props) => { + // This custom hook invocation seems to trigger false positives below + const [state, setState] = useCustomHook({ + inputLinkUrl: linkNodeUrl ?? '', + editable: !isLink, + lastLinkUrl: '', + lastSelection: null + }); + + const [someThing, setSomeThing] = useState(true); + + const onEdit = useCallback(() => setSomeThing(false), [inputLinkUrl, setSomeThing]); + + const updateLinkEditor = useCallback(() => { + const rootElement = editor.getRootElement(); + + if (nativeSelection.anchorNode === rootElement) { + let inner = rootElement; + while (inner.firstElementChild !== null) { + inner = inner.firstElementChild as HTMLElement; + } + } + }, [anchorElem, editor, setSomeThing]); + + return
test
; + }; ", ]; diff --git a/crates/oxc_linter/src/snapshots/rules_of_hooks.snap b/crates/oxc_linter/src/snapshots/rules_of_hooks.snap index f7baebf1d..3a3151bf5 100644 --- a/crates/oxc_linter/src/snapshots/rules_of_hooks.snap +++ b/crates/oxc_linter/src/snapshots/rules_of_hooks.snap @@ -218,7 +218,7 @@ expression: rules_of_hooks 5 │ if (b) return; ╰──── - × eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook2" is called conditionally. React Hooks must be called in the exact same order in every component render. + × eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook2" may be executed more than once. Possibly because it is called in a loop. React Hooks must be called in the exact same order in every component render. ╭─[rules_of_hooks.tsx:6:25] 5 │ if (b) return; 6 │ useHook2(); @@ -234,7 +234,7 @@ expression: rules_of_hooks 10 │ if (d) return; ╰──── - × eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook4" is called conditionally. React Hooks must be called in the exact same order in every component render. + × eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook4" may be executed more than once. Possibly because it is called in a loop. React Hooks must be called in the exact same order in every component render. ╭─[rules_of_hooks.tsx:11:25] 10 │ if (d) return; 11 │ useHook4(); @@ -250,7 +250,7 @@ expression: rules_of_hooks 5 │ if (b) continue; ╰──── - × eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook2" is called conditionally. React Hooks must be called in the exact same order in every component render. + × eslint-plugin-react-hooks(rules-of-hooks): React Hook "useHook2" may be executed more than once. Possibly because it is called in a loop. React Hooks must be called in the exact same order in every component render. ╭─[rules_of_hooks.tsx:6:21] 5 │ if (b) continue; 6 │ useHook2();