diff --git a/packages/check/src/check.rs b/packages/check/src/check.rs index 1b0c01cb9..0335b82f0 100644 --- a/packages/check/src/check.rs +++ b/packages/check/src/check.rs @@ -5,8 +5,8 @@ use syn::{spanned::Spanned, visit::Visit, Pat}; use crate::{ issues::{Issue, IssueReport}, metadata::{ - AnyLoopInfo, ClosureInfo, ComponentInfo, ConditionalInfo, FnInfo, ForInfo, HookInfo, - IfInfo, LoopInfo, MatchInfo, Span, WhileInfo, + AnyLoopInfo, AsyncInfo, ClosureInfo, ComponentInfo, ConditionalInfo, FnInfo, ForInfo, + HookInfo, IfInfo, LoopInfo, MatchInfo, Span, WhileInfo, }, }; @@ -46,6 +46,7 @@ enum Node { While(WhileInfo), Loop(LoopInfo), Closure(ClosureInfo), + Async(AsyncInfo), ComponentFn(ComponentInfo), HookFn(HookInfo), OtherFn(FnInfo), @@ -107,7 +108,7 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { ); let mut container_fn: Option = None; for node in self.context.iter().rev() { - match node { + match &node { Node::If(if_info) => { let issue = Issue::HookInsideConditional( hook_info.clone(), @@ -150,6 +151,11 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { ); self.issues.push(issue); } + Node::Async(async_info) => { + let issue = + Issue::HookInsideAsync(hook_info.clone(), async_info.clone()); + self.issues.push(issue); + } Node::ComponentFn(_) | Node::HookFn(_) | Node::OtherFn(_) => { container_fn = Some(node.clone()); break; @@ -164,6 +170,7 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { } } } + syn::visit::visit_expr_call(self, i); } fn visit_item_fn(&mut self, i: &'ast syn::ItemFn) { @@ -208,7 +215,11 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { .unwrap_or_else(|| i.span()) .into(), ))); - syn::visit::visit_expr_if(self, i); + // only visit the body and else branch, calling hooks inside the expression is not conditional + self.visit_block(&i.then_branch); + if let Some(it) = &i.else_branch { + self.visit_expr(&(it).1); + } self.context.pop(); } @@ -221,7 +232,10 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { .unwrap_or_else(|| i.span()) .into(), ))); - syn::visit::visit_expr_match(self, i); + // only visit the arms, calling hooks inside the expression is not conditional + for it in &i.arms { + self.visit_arm(it); + } self.context.pop(); } @@ -264,6 +278,13 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { syn::visit::visit_expr_closure(self, i); self.context.pop(); } + + fn visit_expr_async(&mut self, i: &'ast syn::ExprAsync) { + self.context + .push(Node::Async(AsyncInfo::new(i.span().into()))); + syn::visit::visit_expr_async(self, i); + self.context.pop(); + } } #[cfg(test)] @@ -416,6 +437,22 @@ mod tests { ); } + #[test] + fn test_use_in_match_expr() { + let contents = indoc! {r#" + fn use_thing() { + match use_resource(|| async {}) { + Ok(_) => {} + Err(_) => {} + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + #[test] fn test_for_loop_hook() { let contents = indoc! {r#" @@ -549,6 +586,21 @@ mod tests { assert_eq!(report.issues, vec![]); } + #[test] + fn test_conditional_expr_okay() { + let contents = indoc! {r#" + fn App() -> Element { + if use_signal(|| true) { + println!("clap your {something}") + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + #[test] fn test_closure_hook() { let contents = indoc! {r#" @@ -637,4 +689,155 @@ mod tests { assert_eq!(report.issues, vec![]); } + + #[test] + fn test_hook_inside_hook_initialization() { + let contents = indoc! {r#" + fn use_thing() { + let _a = use_signal(|| use_signal(|| 0)); + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideClosure( + HookInfo::new( + Span::new_from_str( + "use_signal(|| 0)", + LineColumn { + line: 2, + column: 27, + }, + ), + Span::new_from_str( + "use_signal", + LineColumn { + line: 2, + column: 27, + }, + ), + "use_signal".to_string() + ), + ClosureInfo::new(Span::new_from_str( + "|| use_signal(|| 0)", + LineColumn { + line: 2, + column: 24, + }, + )) + ),] + ); + } + + #[test] + fn test_hook_inside_hook_async_initialization() { + let contents = indoc! {r#" + fn use_thing() { + let _a = use_future(|| async move { use_signal(|| 0) }); + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![ + Issue::HookInsideAsync( + HookInfo::new( + Span::new_from_str( + "use_signal(|| 0)", + LineColumn { + line: 2, + column: 40, + }, + ), + Span::new_from_str( + "use_signal", + LineColumn { + line: 2, + column: 40, + }, + ), + "use_signal".to_string() + ), + AsyncInfo::new(Span::new_from_str( + "async move { use_signal(|| 0) }", + LineColumn { + line: 2, + column: 27, + }, + )) + ), + Issue::HookInsideClosure( + HookInfo::new( + Span::new_from_str( + "use_signal(|| 0)", + LineColumn { + line: 2, + column: 40, + }, + ), + Span::new_from_str( + "use_signal", + LineColumn { + line: 2, + column: 40, + }, + ), + "use_signal".to_string() + ), + ClosureInfo::new(Span::new_from_str( + "|| async move { use_signal(|| 0) }", + LineColumn { + line: 2, + column: 24, + }, + )) + ), + ] + ); + } + + #[test] + fn test_hook_inside_spawn() { + let contents = indoc! {r#" + fn use_thing() { + let _a = spawn(async move { use_signal(|| 0) }); + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideAsync( + HookInfo::new( + Span::new_from_str( + "use_signal(|| 0)", + LineColumn { + line: 2, + column: 32, + }, + ), + Span::new_from_str( + "use_signal", + LineColumn { + line: 2, + column: 32, + }, + ), + "use_signal".to_string() + ), + AsyncInfo::new(Span::new_from_str( + "async move { use_signal(|| 0) }", + LineColumn { + line: 2, + column: 19, + }, + )) + ),] + ); + } } diff --git a/packages/check/src/issues.rs b/packages/check/src/issues.rs index a76c8e678..621fd4925 100644 --- a/packages/check/src/issues.rs +++ b/packages/check/src/issues.rs @@ -8,7 +8,8 @@ use std::{ }; use crate::metadata::{ - AnyLoopInfo, ClosureInfo, ConditionalInfo, ForInfo, HookInfo, IfInfo, MatchInfo, WhileInfo, + AnyLoopInfo, AsyncInfo, ClosureInfo, ConditionalInfo, ForInfo, HookInfo, IfInfo, MatchInfo, + WhileInfo, }; /// The result of checking a Dioxus file for issues. @@ -142,7 +143,9 @@ impl Display for IssueReport { Issue::HookInsideLoop(_, AnyLoopInfo::Loop(_)) => { writeln!(f, "{} `loop {{ … }}` is the loop", note_text_prefix,)?; } - Issue::HookOutsideComponent(_) | Issue::HookInsideClosure(_, _) => {} + Issue::HookOutsideComponent(_) + | Issue::HookInsideClosure(_, _) + | Issue::HookInsideAsync(_, _) => {} } if i < self.issues.len() - 1 { @@ -165,6 +168,7 @@ pub enum Issue { HookInsideLoop(HookInfo, AnyLoopInfo), /// HookInsideClosure(HookInfo, ClosureInfo), + HookInsideAsync(HookInfo, AsyncInfo), HookOutsideComponent(HookInfo), } @@ -174,6 +178,7 @@ impl Issue { Issue::HookInsideConditional(hook_info, _) | Issue::HookInsideLoop(hook_info, _) | Issue::HookInsideClosure(hook_info, _) + | Issue::HookInsideAsync(hook_info, _) | Issue::HookOutsideComponent(hook_info) => hook_info.clone(), } } @@ -208,6 +213,9 @@ impl std::fmt::Display for Issue { Issue::HookInsideClosure(hook_info, _) => { write!(f, "hook called in a closure: `{}`", hook_info.name) } + Issue::HookInsideAsync(hook_info, _) => { + write!(f, "hook called in an async block: `{}`", hook_info.name) + } Issue::HookOutsideComponent(hook_info) => { write!( f, diff --git a/packages/check/src/metadata.rs b/packages/check/src/metadata.rs index 11777e6f2..b36f7bab4 100644 --- a/packages/check/src/metadata.rs +++ b/packages/check/src/metadata.rs @@ -111,6 +111,18 @@ impl ClosureInfo { } } +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about an async block. +pub struct AsyncInfo { + pub span: Span, +} + +impl AsyncInfo { + pub const fn new(span: Span) -> Self { + Self { span } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] /// Information about a component function. pub struct ComponentInfo { diff --git a/packages/core/src/scope_context.rs b/packages/core/src/scope_context.rs index 4fae8c1e4..2012b2d6f 100644 --- a/packages/core/src/scope_context.rs +++ b/packages/core/src/scope_context.rs @@ -424,6 +424,8 @@ impl Scope { You likely used the hook in a conditional. Hooks rely on consistent ordering between renders. Functions prefixed with "use" should never be called conditionally. + + Help: Run `dx check` to look for check for some common hook errors. "#, ) }