chore: cleanup.

crates/oxc_linter/src/rules/react/rules_of_hooks.rs

@@ -104,6 +104,7 @@ impl Rule for RulesOfHooks {
  let is_use = call.callee_name().is_some_and(|name| name == "use");
 
  match parent_func.kind() {
+ // We are in a named function that isn't a hook or component, which is illegal
  AstKind::Function(Function { id: Some(id), .. })
  if !is_react_component_or_hook_name(&id.name) =>
  {
@@ -113,6 +114,7 @@ impl Rule for RulesOfHooks {
  func: id.span,
  });
  }
+ // Hooks can't be called from async function.
  AstKind::Function(Function { id: Some(id), r#async: true, .. }) => {
  return ctx.diagnostic(RulesOfHooksDiagnostic::AsyncComponent(id.span));
  }
@@ -121,6 +123,7 @@ impl Rule for RulesOfHooks {
  AstKind::Function(Function { id: None, .. }) | AstKind::ArrowFunctionExpression(_)
  if is_non_react_func_arg(nodes, parent_func.id()) =>
  {
+ // This rules doesn't apply to `use(...)`.
  if !is_use && is_somewhere_inside_component_or_hook(nodes, parent_func.id()) {
  ctx.diagnostic(RulesOfHooksDiagnostic::GenericError(call.span));
  }
@@ -130,6 +133,34 @@ impl Rule for RulesOfHooks {
  | AstKind::ArrowFunctionExpression(ArrowFunctionExpression { span, .. }) => {
  let ident = get_declaration_identifier(nodes, parent_func.id());
 
+ // Hooks cannot be called inside of export default functions or used in a function
+ // declaration outside of a react component or hook.
+ // For example these are invalid examples:
+ // function ComponentWithHookInsideCallback() {
+ // function handleClick() {
+ // useState();
+ // }
+ // }
+ // --------------
+ // function createComponent() {
+ // return function ComponentWithHookInsideCallback() {
+ // function handleClick() {
+ // useState();
+ // }
+ // }
+ // }
+ // --------------
+ // export default () => {
+ // if (isVal) {
+ // useState(0);
+ // }
+ // }
+ // --------------
+ // export default function() {
+ // if (isVal) {
+ // useState(0);
+ // }
+ // }
  if ident.is_some_and(|name| !is_react_component_or_hook_name(name))
  || is_export_default(nodes, parent_func.id())
  {
@@ -150,7 +181,6 @@ impl Rule for RulesOfHooks {
  return;
  }
 
- let graph = &semantic.cfg().graph;
  let node_cfg_ix = node.cfg_ix();
  let func_cfg_ix = parent_func.cfg_ix();
 
@@ -159,26 +189,68 @@ impl Rule for RulesOfHooks {
  return;
  }
 
- if !petgraph::algo::has_path_connecting(graph, func_cfg_ix, node_cfg_ix, None) {
+ if !petgraph::algo::has_path_connecting(
+ &semantic.cfg().graph,
+ func_cfg_ix,
+ node_cfg_ix,
+ None,
+ ) {
  // There should always be a control flow path between a parent and child node.
  // If there is none it means we always do an early exit before reaching our hook call.
  // In some cases it might mean that we are operating on an invalid `cfg` but in either
  // case, It is somebody else's problem so we just return.
  return;
  }
 
- // TODO: all `dijkstra` algorithms can be merged together for better performance.
-
  // Is this node cyclic?
- if petgraph::algo::dijkstra(graph, node_cfg_ix, None, |_| 0)
+ if self.is_cyclic(ctx, node_cfg_ix) {
+ return ctx.diagnostic(RulesOfHooksDiagnostic::LoopHook(call.span));
+ }
+
+ if self.is_conditional(ctx, func_cfg_ix, node_cfg_ix)
+ || self.breaks_early(ctx, func_cfg_ix, node_cfg_ix)
+ {
+ #[allow(clippy::needless_return)]
+ return ctx.diagnostic(RulesOfHooksDiagnostic::ConditionalHook(call.span));
+ }
+ }
+}
+
+// TODO: all `dijkstra` algorithms can be merged together for better performance.
+impl RulesOfHooks {
+ #[inline(always)]
+ fn is_cyclic<'a>(&self, ctx: &LintContext<'a>, 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))
- {
- return ctx.diagnostic(RulesOfHooksDiagnostic::LoopHook(call.span));
- }
- let res = neighbors_filtered_by_edge_weight(
- graph,
+ }
+
+ #[inline(always)]
+ fn is_conditional<'a>(
+ &self,
+ ctx: &LintContext<'a>,
+ func_cfg_ix: NodeIndex,
+ node_cfg_ix: NodeIndex,
+ ) -> bool {
+ let graph = &ctx.semantic().cfg().graph;
+ // All nodes should be reachable from our hook, Otherwise we have a conditional/branching flow.
+ petgraph::algo::dijkstra(graph, func_cfg_ix, Some(node_cfg_ix), |_| 0)
+ .into_iter()
+ .any(|(f, _)| !petgraph::algo::has_path_connecting(graph, f, node_cfg_ix, None))
+ }
+
+ #[inline(always)]
+ fn breaks_early<'a>(
+ &self,
+ ctx: &LintContext<'a>,
+ func_cfg_ix: NodeIndex,
+ node_cfg_ix: NodeIndex,
+ ) -> bool {
+ let cfg = ctx.semantic().cfg();
+ neighbors_filtered_by_edge_weight(
+ &cfg.graph,
  func_cfg_ix,
  &|e| match e {
  EdgeType::Normal => None,
@@ -189,8 +261,7 @@ impl Rule for RulesOfHooks {
  return (state, false);
  }
 
- let (push, keep_walking) = semantic
- .cfg()
+ let (push, keep_walking) = cfg
  .basic_block_by_index(*ix)
  .iter()
  .fold_while((false, true), |acc, it| match it {
@@ -209,24 +280,11 @@ impl Rule for RulesOfHooks {
  }
  (state, keep_walking)
  },
- );
-
- let res = res.iter().flat_map(|it| it.0.iter()).next().is_some();
- // All nodes should be reachable from our hook, Otherwise we have a conditional/branching flow.
- if petgraph::algo::dijkstra(graph, func_cfg_ix, Some(node_cfg_ix), |_| 0).into_iter().any(
- |(f, _)| {
- !petgraph::algo::has_path_connecting(graph, f, node_cfg_ix, None)
- || semantic
- .cfg()
- .basic_block_by_index(f)
- .iter()
- .any(|it| matches!(it, BasicBlockElement::Break(_)))
- },
- ) || res
- {
- #[allow(clippy::needless_return)]
- return ctx.diagnostic(RulesOfHooksDiagnostic::ConditionalHook(call.span));
- }
+ )
+ .iter()
+ .flat_map(|it| it.0.iter())
+ .next()
+ .is_some()
  }
 }
 
@@ -738,6 +796,14 @@ fn test() {
  return <Child data={data} />
  }
  ",
+ "
+ function useLabeledBlock() {
+ label: {
+ useHook();
+ if (a) break label;
+ }
+ }
+ ",
  ];
 
  let fail = vec![
@@ -1031,7 +1097,6 @@ fn test() {
  // Invalid because it's dangerous and might not warn otherwise.
  // This *must* be invalid.
  // errors: [conditionalError('useHook')],
- // TODO: FIX ME!
  "
  function useLabeledBlock() {
  label: {
@@ -1324,7 +1389,8 @@ fn test() {
  }
  }
  ",
- // TODO: this should error but doesn't.
+ // TODO: This should error but doesn't.
+ // Original rule also fails to raise this error.
  // errors: [genericError('useState')],
  // "
  // function notAComponent() {