dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 04:08:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(linter/react): fix loop hooks false positives. (#3297)

Browse source 
It is a temporary fix for false positives like
[this](https://github.com/oxc-project/oxc/issues/3257#issuecomment-2110199442).
Uses the node's ancestors for now instead of the cfg.
This commit is contained in:
Ali Rezvani 2024-05-16 06:33:54 +03:30 committed by GitHub
parent cc1882d221
commit d46538ed68
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 52 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

63
crates/oxc_linter/src/rules/react/rules_of_hooks.rs
View file

@ -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<State>({
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 <div>test</div>;
};
",
];

6
crates/oxc_linter/src/snapshots/rules_of_hooks.snap
View file

@ -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();