From 2c2534d1cc6b1799e10badc346d66ee82c4bea36 Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Tue, 18 Jul 2023 17:12:47 -0400 Subject: [PATCH 1/7] feat(check): adds `dx check` At the moment this only checks the Rules of Hooks, ensuring that hook functions (i.e. `use_*`) are being called as expected. https://dioxuslabs.com/docs/0.3/guide/en/interactivity/hooks.html Closes #1202 --- Cargo.toml | 2 + packages/check/Cargo.toml | 25 ++ packages/check/README.md | 49 +++ packages/check/src/check.rs | 226 ++++++++++++++ packages/check/src/issues.rs | 154 ++++++++++ packages/check/src/lib.rs | 479 ++++++++++++++++++++++++++++++ packages/check/src/metadata.rs | 181 +++++++++++ packages/cli/Cargo.toml | 1 + packages/cli/src/cli/check/mod.rs | 128 ++++++++ packages/cli/src/cli/mod.rs | 5 + packages/cli/src/main.rs | 5 + 11 files changed, 1255 insertions(+) create mode 100644 packages/check/Cargo.toml create mode 100644 packages/check/README.md create mode 100644 packages/check/src/check.rs create mode 100644 packages/check/src/issues.rs create mode 100644 packages/check/src/lib.rs create mode 100644 packages/check/src/metadata.rs create mode 100644 packages/cli/src/cli/check/mod.rs diff --git a/Cargo.toml b/Cargo.toml index 722666d95..a493d98b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ members = [ "packages/fermi", "packages/liveview", "packages/autofmt", + "packages/check", "packages/rsx", "packages/dioxus-tui", "packages/rink", @@ -63,6 +64,7 @@ dioxus-interpreter-js = { path = "packages/interpreter" } fermi = { path = "packages/fermi" } dioxus-liveview = { path = "packages/liveview" } dioxus-autofmt = { path = "packages/autofmt" } +dioxus-check = { path = "packages/check" } dioxus-rsx = { path = "packages/rsx" } dioxus-tui = { path = "packages/dioxus-tui" } rink = { path = "packages/rink" } diff --git a/packages/check/Cargo.toml b/packages/check/Cargo.toml new file mode 100644 index 000000000..9757eaa45 --- /dev/null +++ b/packages/check/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "dioxus-check" +version = "0.1.0" +edition = "2021" +authors = ["Dioxus Labs"] +description = "Checks Dioxus RSX files for issues" +license = "MIT/Apache-2.0" +repository = "https://github.com/DioxusLabs/dioxus/" +homepage = "https://dioxuslabs.com" +keywords = ["dom", "ui", "gui", "react"] +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +dioxus-rsx = { workspace = true } +proc-macro2 = { version = "1.0.6", features = ["span-locations"] } +quote = "1.0" +syn = { version = "1.0.11", features = ["full", "extra-traits", "visit"] } +serde = { version = "1.0.136", features = ["derive"] } +prettyplease = { package = "prettier-please", version = "0.1.16", features = [ + "verbatim", +] } +owo-colors = "3.5.0" + +[dev-dependencies] +pretty_assertions = "1.2.1" diff --git a/packages/check/README.md b/packages/check/README.md new file mode 100644 index 000000000..bd934f2c7 --- /dev/null +++ b/packages/check/README.md @@ -0,0 +1,49 @@ +# dioxus-autofmt + + +[![Crates.io][crates-badge]][crates-url] +[![MIT licensed][mit-badge]][mit-url] +[![Build Status][actions-badge]][actions-url] +[![Discord chat][discord-badge]][discord-url] + +[crates-badge]: https://img.shields.io/crates/v/dioxus-autofmt.svg +[crates-url]: https://crates.io/crates/dioxus-autofmt + +[mit-badge]: https://img.shields.io/badge/license-MIT-blue.svg +[mit-url]: https://github.com/dioxuslabs/dioxus/blob/master/LICENSE + +[actions-badge]: https://github.com/dioxuslabs/dioxus/actions/workflows/main.yml/badge.svg +[actions-url]: https://github.com/dioxuslabs/dioxus/actions?query=workflow%3ACI+branch%3Amaster + +[discord-badge]: https://img.shields.io/discord/899851952891002890.svg?logo=discord&style=flat-square +[discord-url]: https://discord.gg/XgGxMSkvUM + +[Website](https://dioxuslabs.com) | +[Guides](https://dioxuslabs.com/docs/0.3/guide/en/) | +[API Docs](https://docs.rs/dioxus-autofmt/latest/dioxus_autofmt) | +[Chat](https://discord.gg/XgGxMSkvUM) + + +## Overview + +`dioxus-autofmt` provides a pretty printer for the `rsx` syntax tree. + + +This is done manually with a via set of formatting rules. The output is not guaranteed to be stable between minor versions of the crate as we might tweak the output. + +`dioxus-autofmt` provides an API to perform precision edits as well as just spit out a block of formatted RSX from any RSX syntax tree. This is used by the `rsx-rosetta` crate which can accept various input languages and output valid RSX. + + +## Contributing + +- Report issues on our [issue tracker](https://github.com/dioxuslabs/dioxus/issues). +- Join the discord and ask questions! + +## License +This project is licensed under the [MIT license]. + +[mit license]: https://github.com/DioxusLabs/dioxus/blob/master/LICENSE-MIT + +Unless you explicitly state otherwise, any contribution intentionally submitted +for inclusion in Dioxus by you shall be licensed as MIT without any additional +terms or conditions. diff --git a/packages/check/src/check.rs b/packages/check/src/check.rs new file mode 100644 index 000000000..7d89bf0a3 --- /dev/null +++ b/packages/check/src/check.rs @@ -0,0 +1,226 @@ +use std::path::PathBuf; + +use syn::{spanned::Spanned, visit::Visit}; + +use crate::{ + issues::{Issue, IssueReport}, + metadata::{ + AnyLoopInfo, ClosureInfo, ComponentInfo, ConditionalInfo, FnInfo, ForInfo, HookInfo, + IfInfo, LoopInfo, MatchInfo, Span, WhileInfo, + }, +}; + +struct VisitHooks { + issues: Vec, + context: Vec, +} + +impl VisitHooks { + const fn new() -> Self { + Self { + issues: vec![], + context: vec![], + } + } +} + +/// Checks a Dioxus file for issues. +pub fn check_file(path: PathBuf, file_content: &str) -> IssueReport { + let file = syn::parse_file(file_content).unwrap(); + let mut visit_hooks = VisitHooks::new(); + visit_hooks.visit_file(&file); + IssueReport { + path, + file_content: file_content.to_string(), + issues: visit_hooks.issues, + } +} + +#[derive(Debug, Clone)] +enum Node { + If(IfInfo), + Match(MatchInfo), + For(ForInfo), + While(WhileInfo), + Loop(LoopInfo), + Closure(ClosureInfo), + ComponentFn(ComponentInfo), + HookFn(HookInfo), + OtherFn(FnInfo), +} + +fn returns_element(ty: &syn::ReturnType) -> bool { + match ty { + syn::ReturnType::Default => false, + syn::ReturnType::Type(_, ref ty) => { + if let syn::Type::Path(ref path) = **ty { + if let Some(segment) = path.path.segments.last() { + if segment.ident == "Element" { + return true; + } + } + } + false + } + } +} + +fn is_hook_ident(ident: &syn::Ident) -> bool { + ident.to_string().starts_with("use_") +} + +fn is_component_fn(item_fn: &syn::ItemFn) -> bool { + returns_element(&item_fn.sig.output) +} + +fn fn_name_and_name_span(item_fn: &syn::ItemFn) -> (String, Span) { + let name = item_fn.sig.ident.to_string(); + let name_span = item_fn.sig.ident.span().into(); + (name, name_span) +} + +impl<'ast> syn::visit::Visit<'ast> for VisitHooks { + fn visit_expr_call(&mut self, i: &'ast syn::ExprCall) { + if let syn::Expr::Path(ref path) = *i.func { + if let Some(segment) = path.path.segments.last() { + if is_hook_ident(&segment.ident) { + let hook_info = HookInfo::new( + i.span().into(), + segment.ident.span().into(), + segment.ident.to_string(), + ); + let mut container_fn: Option = None; + for node in self.context.iter().rev() { + match node { + Node::If(if_info) => { + let issue = Issue::HookInsideConditional( + hook_info.clone(), + ConditionalInfo::If(if_info.clone()), + ); + self.issues.push(issue); + } + Node::Match(match_info) => { + let issue = Issue::HookInsideConditional( + hook_info.clone(), + ConditionalInfo::Match(match_info.clone()), + ); + self.issues.push(issue); + } + Node::For(for_info) => { + let issue = Issue::HookInsideLoop( + hook_info.clone(), + AnyLoopInfo::For(for_info.clone()), + ); + self.issues.push(issue); + } + Node::While(while_info) => { + let issue = Issue::HookInsideLoop( + hook_info.clone(), + AnyLoopInfo::While(while_info.clone()), + ); + self.issues.push(issue); + } + Node::Loop(loop_info) => { + let issue = Issue::HookInsideLoop( + hook_info.clone(), + AnyLoopInfo::Loop(loop_info.clone()), + ); + self.issues.push(issue); + } + Node::Closure(closure_info) => { + let issue = Issue::HookInsideClosure( + hook_info.clone(), + closure_info.clone(), + ); + self.issues.push(issue); + } + Node::ComponentFn(_) | Node::HookFn(_) | Node::OtherFn(_) => { + container_fn = Some(node.clone()); + break; + } + } + } + + if let Some(Node::OtherFn(_)) = container_fn { + let issue = Issue::HookOutsideComponent(hook_info); + self.issues.push(issue); + } + } + } + } + } + + fn visit_item_fn(&mut self, i: &'ast syn::ItemFn) { + let (name, name_span) = fn_name_and_name_span(i); + if is_component_fn(i) { + self.context.push(Node::ComponentFn(ComponentInfo::new( + i.span().into(), + name, + name_span, + ))); + } else if is_hook_ident(&i.sig.ident) { + self.context.push(Node::HookFn(HookInfo::new( + i.span().into(), + i.sig.ident.span().into(), + name, + ))); + } else { + self.context + .push(Node::OtherFn(FnInfo::new(i.span().into(), name, name_span))); + } + syn::visit::visit_item_fn(self, i); + self.context.pop(); + } + + fn visit_expr_if(&mut self, i: &'ast syn::ExprIf) { + self.context.push(Node::If(IfInfo::new( + i.span().into(), + i.if_token.span().into(), + ))); + syn::visit::visit_expr_if(self, i); + self.context.pop(); + } + + fn visit_expr_match(&mut self, i: &'ast syn::ExprMatch) { + self.context.push(Node::Match(MatchInfo::new( + i.span().into(), + i.match_token.span().into(), + ))); + syn::visit::visit_expr_match(self, i); + self.context.pop(); + } + + fn visit_expr_for_loop(&mut self, i: &'ast syn::ExprForLoop) { + self.context.push(Node::For(ForInfo::new( + i.span().into(), + i.for_token.span().into(), + ))); + syn::visit::visit_expr_for_loop(self, i); + self.context.pop(); + } + + fn visit_expr_while(&mut self, i: &'ast syn::ExprWhile) { + self.context.push(Node::While(WhileInfo::new( + i.span().into(), + i.while_token.span().into(), + ))); + syn::visit::visit_expr_while(self, i); + self.context.pop(); + } + + fn visit_expr_loop(&mut self, i: &'ast syn::ExprLoop) { + self.context.push(Node::Loop(LoopInfo::new( + i.span().into(), + i.loop_token.span().into(), + ))); + syn::visit::visit_expr_loop(self, i); + self.context.pop(); + } + + fn visit_expr_closure(&mut self, i: &'ast syn::ExprClosure) { + self.context + .push(Node::Closure(ClosureInfo::new(i.span().into()))); + syn::visit::visit_expr_closure(self, i); + self.context.pop(); + } +} diff --git a/packages/check/src/issues.rs b/packages/check/src/issues.rs new file mode 100644 index 000000000..7f661f88e --- /dev/null +++ b/packages/check/src/issues.rs @@ -0,0 +1,154 @@ +use owo_colors::{ + colors::{css::LightBlue, BrightRed}, + OwoColorize, +}; +use std::{ + fmt::Display, + path::{Path, PathBuf}, +}; + +use crate::metadata::{AnyLoopInfo, ClosureInfo, ConditionalInfo, HookInfo}; + +/// The result of checking a Dioxus file for issues. +pub struct IssueReport { + pub path: PathBuf, + pub file_content: String, + pub issues: Vec, +} + +impl IssueReport { + pub fn new(path: PathBuf, file_content: S, issues: Vec) -> Self { + Self { + path, + file_content: file_content.to_string(), + issues, + } + } +} + +impl Display for IssueReport { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let relative_file = Path::new(&self.path) + .strip_prefix(std::env::current_dir().unwrap()) + .unwrap_or(Path::new(&self.path)) + .display(); + + for (i, issue) in self.issues.iter().enumerate() { + let hook_info = issue.hook_info(); + let hook_span = hook_info.span; + let hook_name_span = hook_info.name_span; + let error_line = format!("{}: {}", "error".fg::(), issue); + writeln!(f, "{}", error_line.bold())?; + writeln!( + f, + " {} {}:{}:{}", + "-->".fg::(), + relative_file, + hook_span.start.line, + hook_span.start.column + 1 + )?; + let max_line_num_len = hook_span.end.line.to_string().len(); + writeln!(f, "{:>max_line_num_len$} {}", "", "|".fg::())?; + for (i, line) in self.file_content.lines().enumerate() { + let line_num = i + 1; + if line_num >= hook_span.start.line && line_num <= hook_span.end.line { + writeln!( + f, + "{:>max_line_num_len$} {} {}", + line_num, + "|".fg::(), + line, + max_line_num_len = max_line_num_len + )?; + if line_num == hook_span.start.line { + let mut caret = String::new(); + for _ in 0..hook_name_span.start.column { + caret.push(' '); + } + for _ in hook_name_span.start.column..hook_name_span.end.column { + caret.push('^'); + } + writeln!( + f, + "{:>max_line_num_len$} {} {}", + "", + "|".fg::(), + caret.fg::(), + max_line_num_len = max_line_num_len + )?; + } + } + } + + if i < self.issues.len() - 1 { + writeln!(f)?; + } + } + + Ok(()) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(clippy::enum_variant_names)] // we'll add non-hook ones in the future +/// Issues that might be found via static analysis of a Dioxus file. +pub enum Issue { + /// https://dioxuslabs.com/docs/0.3/guide/en/interactivity/hooks.html#no-hooks-in-conditionals + HookInsideConditional(HookInfo, ConditionalInfo), + /// https://dioxuslabs.com/docs/0.3/guide/en/interactivity/hooks.html#no-hooks-in-loops + HookInsideLoop(HookInfo, AnyLoopInfo), + /// https://dioxuslabs.com/docs/0.3/guide/en/interactivity/hooks.html#no-hooks-in-closures + HookInsideClosure(HookInfo, ClosureInfo), + HookOutsideComponent(HookInfo), +} + +impl Issue { + pub fn hook_info(&self) -> HookInfo { + match self { + Issue::HookInsideConditional(hook_info, _) + | Issue::HookInsideLoop(hook_info, _) + | Issue::HookInsideClosure(hook_info, _) + | Issue::HookOutsideComponent(hook_info) => hook_info.clone(), + } + } +} + +impl std::fmt::Display for Issue { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Issue::HookInsideConditional(hook_info, conditional_info) => { + write!( + f, + "hook called conditionally: `{}` (inside `{}`)", + hook_info.name, + match conditional_info { + ConditionalInfo::If(_) => "if", + ConditionalInfo::Match(_) => "match", + } + ) + } + Issue::HookInsideLoop(hook_info, loop_info) => { + write!( + f, + "hook called in a loop: `{}` (inside {})", + hook_info.name, + match loop_info { + AnyLoopInfo::For(_) => "`for` loop", + AnyLoopInfo::While(_) => "`while` loop", + AnyLoopInfo::Loop(_) => "`loop`", + } + ) + } + Issue::HookInsideClosure(hook_info, _) => { + write!(f, "hook called inside closure: `{}`", hook_info.name) + } + Issue::HookOutsideComponent(hook_info) => { + write!( + f, + "hook called outside component or hook: `{}`", + hook_info.name + ) + } + } + } +} diff --git a/packages/check/src/lib.rs b/packages/check/src/lib.rs new file mode 100644 index 000000000..712462e40 --- /dev/null +++ b/packages/check/src/lib.rs @@ -0,0 +1,479 @@ +mod check; +mod issues; +mod metadata; + +pub use check::check_file; +pub use issues::{Issue, IssueReport}; + +#[cfg(test)] +mod tests { + use crate::metadata::{ + AnyLoopInfo, ClosureInfo, ConditionalInfo, ForInfo, HookInfo, IfInfo, LineColumn, LoopInfo, + MatchInfo, Span, WhileInfo, + }; + + use super::*; + + #[test] + fn test_no_issues() { + let contents = r#" + fn App(cx: Scope) -> Element { + rsx! { + p { "Hello World" } + } + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + + #[test] + fn test_conditional_hook_if() { + let contents = r#" + fn App(cx: Scope) -> Element { + if you_are_happy && you_know_it { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideConditional( + HookInfo::new( + Span { + start: LineColumn { + line: 4, + column: 36 + }, + end: LineColumn { + line: 4, + column: 61 + } + }, + Span { + start: LineColumn { + line: 4, + column: 36 + }, + end: LineColumn { + line: 4, + column: 45 + } + }, + "use_state".to_string() + ), + ConditionalInfo::If(IfInfo::new( + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 6, + column: 17 + } + }, + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 3, + column: 18 + } + } + )) + )], + ); + } + + #[test] + fn test_conditional_hook_match() { + let contents = r#" + fn App(cx: Scope) -> Element { + match you_are_happy && you_know_it { + true => { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + false => {} + } + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideConditional( + HookInfo::new( + Span { + start: LineColumn { + line: 5, + column: 40 + }, + end: LineColumn { + line: 5, + column: 65 + } + }, + Span { + start: LineColumn { + line: 5, + column: 40 + }, + end: LineColumn { + line: 5, + column: 49 + } + }, + "use_state".to_string() + ), + ConditionalInfo::Match(MatchInfo::new( + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 9, + column: 17 + } + }, + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 3, + column: 21 + } + } + )) + )] + ); + } + + #[test] + fn test_for_loop_hook() { + let contents = r#" + fn App(cx: Scope) -> Element { + for _name in &names { + let is_selected = use_state(cx, || false); + println!("selected: {is_selected}"); + } + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideLoop( + HookInfo::new( + Span { + start: LineColumn { + line: 4, + column: 38 + }, + end: LineColumn { + line: 4, + column: 61 + } + }, + Span { + start: LineColumn { + line: 4, + column: 38 + }, + end: LineColumn { + line: 4, + column: 47 + } + }, + "use_state".to_string() + ), + AnyLoopInfo::For(ForInfo::new( + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 6, + column: 17 + } + }, + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 3, + column: 19 + } + } + )) + )] + ); + } + + #[test] + fn test_while_loop_hook() { + let contents = r#" + fn App(cx: Scope) -> Element { + while true { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideLoop( + HookInfo::new( + Span { + start: LineColumn { + line: 4, + column: 36 + }, + end: LineColumn { + line: 4, + column: 61 + } + }, + Span { + start: LineColumn { + line: 4, + column: 36 + }, + end: LineColumn { + line: 4, + column: 45 + } + }, + "use_state".to_string() + ), + AnyLoopInfo::While(WhileInfo::new( + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 6, + column: 17 + } + }, + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 3, + column: 21 + } + } + )) + )], + ); + } + + #[test] + fn test_loop_hook() { + let contents = r#" + fn App(cx: Scope) -> Element { + loop { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideLoop( + HookInfo::new( + Span { + start: LineColumn { + line: 4, + column: 36 + }, + end: LineColumn { + line: 4, + column: 61 + } + }, + Span { + start: LineColumn { + line: 4, + column: 36 + }, + end: LineColumn { + line: 4, + column: 45 + } + }, + "use_state".to_string() + ), + AnyLoopInfo::Loop(LoopInfo::new( + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 6, + column: 17 + } + }, + Span { + start: LineColumn { + line: 3, + column: 16 + }, + end: LineColumn { + line: 3, + column: 20 + } + } + )) + )], + ); + } + + #[test] + fn test_conditional_okay() { + let contents = r#" + fn App(cx: Scope) -> Element { + let something = use_state(cx, || "hands"); + if you_are_happy && you_know_it { + println!("clap your {something}") + } + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + + #[test] + fn test_closure_hook() { + let contents = r#" + fn App(cx: Scope) -> Element { + let _a = || { + let b = use_state(cx, || 0); + b.get() + }; + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideClosure( + HookInfo::new( + Span { + start: LineColumn { + line: 4, + column: 28 + }, + end: LineColumn { + line: 4, + column: 47 + } + }, + Span { + start: LineColumn { + line: 4, + column: 28 + }, + end: LineColumn { + line: 4, + column: 37 + } + }, + "use_state".to_string() + ), + ClosureInfo::new(Span { + start: LineColumn { + line: 3, + column: 25 + }, + end: LineColumn { + line: 6, + column: 17 + } + }) + )] + ); + } + + #[test] + fn test_hook_outside_component() { + let contents = r#" + fn not_component_or_hook(cx: Scope) { + let _a = use_state(cx, || 0); + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookOutsideComponent(HookInfo::new( + Span { + start: LineColumn { + line: 3, + column: 25 + }, + end: LineColumn { + line: 3, + column: 44 + } + }, + Span { + start: LineColumn { + line: 3, + column: 25 + }, + end: LineColumn { + line: 3, + column: 34 + } + }, + "use_state".to_string() + ))] + ); + } + + #[test] + fn test_hook_inside_hook() { + let contents = r#" + fn use_thing(cx: Scope) { + let _a = use_state(cx, || 0); + } + "#; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } +} diff --git a/packages/check/src/metadata.rs b/packages/check/src/metadata.rs new file mode 100644 index 000000000..d886dcf32 --- /dev/null +++ b/packages/check/src/metadata.rs @@ -0,0 +1,181 @@ +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about a hook call or function. +pub struct HookInfo { + /// The name of the hook, e.g. `use_state`. + pub name: String, + /// The span of the hook, e.g. `use_state(cx, || 0)`. + pub span: Span, + /// The span of the name, e.g. `use_state`. + pub name_span: Span, +} + +impl HookInfo { + pub const fn new(span: Span, name_span: Span, name: String) -> Self { + Self { + span, + name_span, + name, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ConditionalInfo { + If(IfInfo), + Match(MatchInfo), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IfInfo { + /// The span of the `if` statement, e.g. `if true { ... }`. + pub span: Span, + /// The span of the `if` keyword only. + pub keyword_span: Span, +} + +impl IfInfo { + pub const fn new(span: Span, keyword_span: Span) -> Self { + Self { span, keyword_span } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MatchInfo { + /// The span of the `match` statement, e.g. `match true { ... }`. + pub span: Span, + /// The span of the `match` keyword only. + pub keyword_span: Span, +} + +impl MatchInfo { + pub const fn new(span: Span, keyword_span: Span) -> Self { + Self { span, keyword_span } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about one of the possible loop types. +pub enum AnyLoopInfo { + For(ForInfo), + While(WhileInfo), + Loop(LoopInfo), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about a `for` loop. +pub struct ForInfo { + pub span: Span, + pub keyword_span: Span, +} + +impl ForInfo { + pub const fn new(span: Span, keyword_span: Span) -> Self { + Self { span, keyword_span } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about a `while` loop. +pub struct WhileInfo { + pub span: Span, + pub keyword_span: Span, +} + +impl WhileInfo { + pub const fn new(span: Span, keyword_span: Span) -> Self { + Self { span, keyword_span } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about a `loop` loop. +pub struct LoopInfo { + pub span: Span, + pub keyword_span: Span, +} + +impl LoopInfo { + pub const fn new(span: Span, keyword_span: Span) -> Self { + Self { span, keyword_span } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about a closure. +pub struct ClosureInfo { + pub span: Span, +} + +impl ClosureInfo { + pub const fn new(span: Span) -> Self { + Self { span } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about a component function. +pub struct ComponentInfo { + pub span: Span, + pub name: String, + pub name_span: Span, +} + +impl ComponentInfo { + pub const fn new(span: Span, name: String, name_span: Span) -> Self { + Self { + span, + name, + name_span, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// Information about a non-component, non-hook function. +pub struct FnInfo { + pub span: Span, + pub name: String, + pub name_span: Span, +} + +impl FnInfo { + pub const fn new(span: Span, name: String, name_span: Span) -> Self { + Self { + span, + name, + name_span, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// A span of text in a source code file. +pub struct Span { + pub start: LineColumn, + pub end: LineColumn, +} + +impl From for Span { + fn from(span: proc_macro2::Span) -> Self { + Self { + start: span.start().into(), + end: span.end().into(), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +/// A location in a source code file. +pub struct LineColumn { + pub line: usize, + pub column: usize, +} + +impl From for LineColumn { + fn from(lc: proc_macro2::LineColumn) -> Self { + Self { + line: lc.line, + column: lc.column, + } + } +} diff --git a/packages/cli/Cargo.toml b/packages/cli/Cargo.toml index 16b44ebc4..3147990d5 100644 --- a/packages/cli/Cargo.toml +++ b/packages/cli/Cargo.toml @@ -77,6 +77,7 @@ toml_edit = "0.19.11" # dioxus-rsx = "0.0.1" dioxus-autofmt = { workspace = true } +dioxus-check = { workspace = true } rsx-rosetta = { workspace = true } dioxus-rsx = { workspace = true } dioxus-html = { workspace = true, features = ["hot-reload-context"] } diff --git a/packages/cli/src/cli/check/mod.rs b/packages/cli/src/cli/check/mod.rs new file mode 100644 index 000000000..ad086af5a --- /dev/null +++ b/packages/cli/src/cli/check/mod.rs @@ -0,0 +1,128 @@ +use futures::{stream::FuturesUnordered, StreamExt}; +use std::{path::Path, process::exit}; + +use super::*; + +// For reference, the rustfmt main.rs file +// https://github.com/rust-lang/rustfmt/blob/master/src/bin/main.rs + +/// Check the Rust files in the project for issues. +#[derive(Clone, Debug, Parser)] +pub struct Check { + /// Input file + #[clap(short, long)] + pub file: Option, +} + +impl Check { + // Todo: check the entire crate + pub async fn check(self) -> Result<()> { + match self.file { + // Default to checking the project + None => { + if let Err(e) = check_project_and_report().await { + eprintln!("error checking project: {}", e); + exit(1); + } + } + Some(file) => { + if let Err(e) = check_file_and_report(file).await { + eprintln!("failed to check file: {}", e); + exit(1); + } + } + } + + Ok(()) + } +} + +async fn check_file_and_report(path: PathBuf) -> Result<()> { + check_files_and_report(vec![path]).await +} + +/// Read every .rs file accessible when considering the .gitignore and check it +/// +/// Runs using Tokio for multithreading, so it should be really really fast +/// +/// Doesn't do mod-descending, so it will still try to check unreachable files. TODO. +async fn check_project_and_report() -> Result<()> { + let crate_config = crate::CrateConfig::new(None)?; + + let mut files_to_check = vec![]; + collect_rs_files(&crate_config.crate_dir, &mut files_to_check); + check_files_and_report(files_to_check).await +} + +/// Check a list of files and report the issues. +async fn check_files_and_report(files_to_check: Vec) -> Result<()> { + let issue_reports = files_to_check + .into_iter() + .filter(|file| file.components().all(|f| f.as_os_str() != "target")) + .map(|path| async move { + let _path = path.clone(); + let res = tokio::spawn(async move { + tokio::fs::read_to_string(&_path) + .await + .map(|contents| dioxus_check::check_file(_path, &contents)) + }) + .await; + + if res.is_err() { + eprintln!("error checking file: {}", path.display()); + } + + res + }) + .collect::>() + .collect::>() + .await; + + // remove error results which we've already printed + let issue_reports = issue_reports + .into_iter() + .flatten() + .flatten() + .collect::>(); + + let total_issues = issue_reports.iter().map(|r| r.issues.len()).sum::(); + + for report in issue_reports.into_iter() { + if !report.issues.is_empty() { + println!("{}", report); + } + } + + match total_issues { + 0 => println!("No issues found."), + 1 => println!("1 issue found."), + _ => println!("{} issues found.", total_issues), + } + + match total_issues { + 0 => exit(0), + _ => exit(1), + } +} + +fn collect_rs_files(folder: &Path, files: &mut Vec) { + let Ok(folder) = folder.read_dir() else { return }; + + // load the gitignore + + for entry in folder { + let Ok(entry) = entry else { continue; }; + + let path = entry.path(); + + if path.is_dir() { + collect_rs_files(&path, files); + } + + if let Some(ext) = path.extension() { + if ext == "rs" { + files.push(path); + } + } + } +} diff --git a/packages/cli/src/cli/mod.rs b/packages/cli/src/cli/mod.rs index 55f79e7eb..fcbb55dcf 100644 --- a/packages/cli/src/cli/mod.rs +++ b/packages/cli/src/cli/mod.rs @@ -1,6 +1,7 @@ pub mod autoformat; pub mod build; pub mod cfg; +pub mod check; pub mod clean; pub mod config; pub mod create; @@ -67,6 +68,9 @@ pub enum Commands { #[clap(name = "fmt")] Autoformat(autoformat::Autoformat), + #[clap(name = "check")] + Check(check::Check), + /// Dioxus config file controls. #[clap(subcommand)] Config(config::Config), @@ -88,6 +92,7 @@ impl Display for Commands { Commands::Config(_) => write!(f, "config"), Commands::Version(_) => write!(f, "version"), Commands::Autoformat(_) => write!(f, "fmt"), + Commands::Check(_) => write!(f, "check"), #[cfg(feature = "plugin")] Commands::Plugin(_) => write!(f, "plugin"), diff --git a/packages/cli/src/main.rs b/packages/cli/src/main.rs index 45881f906..d17aa652e 100644 --- a/packages/cli/src/main.rs +++ b/packages/cli/src/main.rs @@ -103,6 +103,11 @@ async fn main() -> anyhow::Result<()> { .await .map_err(|e| anyhow!("🚫 Error autoformatting RSX: {}", e)), + Check(opts) => opts + .check() + .await + .map_err(|e| anyhow!("🚫 Error checking RSX: {}", e)), + Version(opt) => { let version = opt.version(); println!("{}", version); From eb4438ef7425a0a8c3d3e480677dab172a2b31dd Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Tue, 18 Jul 2023 17:16:28 -0400 Subject: [PATCH 2/7] docs(check): update README It was just copy-pasted from `dioxus-autofmt`. --- packages/check/README.md | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/check/README.md b/packages/check/README.md index bd934f2c7..512eb579a 100644 --- a/packages/check/README.md +++ b/packages/check/README.md @@ -1,4 +1,4 @@ -# dioxus-autofmt +# dioxus-check [![Crates.io][crates-badge]][crates-url] @@ -26,13 +26,7 @@ ## Overview -`dioxus-autofmt` provides a pretty printer for the `rsx` syntax tree. - - -This is done manually with a via set of formatting rules. The output is not guaranteed to be stable between minor versions of the crate as we might tweak the output. - -`dioxus-autofmt` provides an API to perform precision edits as well as just spit out a block of formatted RSX from any RSX syntax tree. This is used by the `rsx-rosetta` crate which can accept various input languages and output valid RSX. - +`dioxus-check` analyzes Dioxus source code and reports errors and warnings. Primarily, it enforces the [Rules of Hooks](https://dioxuslabs.com/docs/0.3/guide/en/interactivity/hooks.html#no-hooks-in-conditionals). ## Contributing From 899d9562b373e91c18d00b59f5e8cb7d0b2eb5ef Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Tue, 18 Jul 2023 17:23:41 -0400 Subject: [PATCH 3/7] fix(check): colorize line numbers a la `rustc` --- packages/check/src/issues.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/check/src/issues.rs b/packages/check/src/issues.rs index 7f661f88e..30357aa03 100644 --- a/packages/check/src/issues.rs +++ b/packages/check/src/issues.rs @@ -55,10 +55,9 @@ impl Display for IssueReport { writeln!( f, "{:>max_line_num_len$} {} {}", - line_num, + line_num.fg::(), "|".fg::(), line, - max_line_num_len = max_line_num_len )?; if line_num == hook_span.start.line { let mut caret = String::new(); @@ -74,7 +73,6 @@ impl Display for IssueReport { "", "|".fg::(), caret.fg::(), - max_line_num_len = max_line_num_len )?; } } From 1531893a4521e6c4a6952db61f614dcc9a5e01e8 Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Sat, 22 Jul 2023 14:51:25 -0700 Subject: [PATCH 4/7] improve test coverage and display output --- packages/check/Cargo.toml | 3 +- packages/check/src/check.rs | 364 ++++++++++++++++++++++++- packages/check/src/issues.rs | 301 ++++++++++++++++++++- packages/check/src/lib.rs | 473 --------------------------------- packages/check/src/metadata.rs | 55 ++-- 5 files changed, 680 insertions(+), 516 deletions(-) diff --git a/packages/check/Cargo.toml b/packages/check/Cargo.toml index 9757eaa45..701d845b3 100644 --- a/packages/check/Cargo.toml +++ b/packages/check/Cargo.toml @@ -19,7 +19,8 @@ serde = { version = "1.0.136", features = ["derive"] } prettyplease = { package = "prettier-please", version = "0.1.16", features = [ "verbatim", ] } -owo-colors = "3.5.0" +owo-colors = { version = "3.5.0", features = ["supports-colors"] } [dev-dependencies] +indoc = "2.0.3" pretty_assertions = "1.2.1" diff --git a/packages/check/src/check.rs b/packages/check/src/check.rs index 7d89bf0a3..b8298689b 100644 --- a/packages/check/src/check.rs +++ b/packages/check/src/check.rs @@ -29,11 +29,12 @@ pub fn check_file(path: PathBuf, file_content: &str) -> IssueReport { let file = syn::parse_file(file_content).unwrap(); let mut visit_hooks = VisitHooks::new(); visit_hooks.visit_file(&file); - IssueReport { + IssueReport::new( path, - file_content: file_content.to_string(), - issues: visit_hooks.issues, - } + std::env::current_dir().unwrap_or_default(), + file_content.to_string(), + visit_hooks.issues, + ) } #[derive(Debug, Clone)] @@ -175,7 +176,11 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { fn visit_expr_if(&mut self, i: &'ast syn::ExprIf) { self.context.push(Node::If(IfInfo::new( i.span().into(), - i.if_token.span().into(), + i.if_token + .span() + .join(i.cond.span()) + .unwrap_or_else(|| i.span()) + .into(), ))); syn::visit::visit_expr_if(self, i); self.context.pop(); @@ -184,7 +189,11 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { fn visit_expr_match(&mut self, i: &'ast syn::ExprMatch) { self.context.push(Node::Match(MatchInfo::new( i.span().into(), - i.match_token.span().into(), + i.match_token + .span() + .join(i.expr.span()) + .unwrap_or_else(|| i.span()) + .into(), ))); syn::visit::visit_expr_match(self, i); self.context.pop(); @@ -193,7 +202,11 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { fn visit_expr_for_loop(&mut self, i: &'ast syn::ExprForLoop) { self.context.push(Node::For(ForInfo::new( i.span().into(), - i.for_token.span().into(), + i.for_token + .span() + .join(i.expr.span()) + .unwrap_or_else(|| i.span()) + .into(), ))); syn::visit::visit_expr_for_loop(self, i); self.context.pop(); @@ -202,17 +215,19 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { fn visit_expr_while(&mut self, i: &'ast syn::ExprWhile) { self.context.push(Node::While(WhileInfo::new( i.span().into(), - i.while_token.span().into(), + i.while_token + .span() + .join(i.cond.span()) + .unwrap_or_else(|| i.span()) + .into(), ))); syn::visit::visit_expr_while(self, i); self.context.pop(); } fn visit_expr_loop(&mut self, i: &'ast syn::ExprLoop) { - self.context.push(Node::Loop(LoopInfo::new( - i.span().into(), - i.loop_token.span().into(), - ))); + self.context + .push(Node::Loop(LoopInfo::new(i.span().into()))); syn::visit::visit_expr_loop(self, i); self.context.pop(); } @@ -224,3 +239,328 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { self.context.pop(); } } + +#[cfg(test)] +mod tests { + use crate::metadata::{ + AnyLoopInfo, ClosureInfo, ConditionalInfo, ForInfo, HookInfo, IfInfo, LineColumn, LoopInfo, + MatchInfo, Span, WhileInfo, + }; + use indoc::indoc; + use pretty_assertions::assert_eq; + + use super::*; + + #[test] + fn test_no_issues() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + rsx! { + p { "Hello World" } + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + + #[test] + fn test_conditional_hook_if() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + if you_are_happy && you_know_it { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideConditional( + HookInfo::new( + Span::new_from_str( + r#"use_state(cx, || "hands")"#, + LineColumn { line: 3, column: 24 }, + ), + Span::new_from_str( + r#"use_state"#, + LineColumn { line: 3, column: 24 }, + ), + "use_state".to_string() + ), + ConditionalInfo::If(IfInfo::new( + Span::new_from_str( + "if you_are_happy && you_know_it {\n let something = use_state(cx, || \"hands\");\n println!(\"clap your {something}\")\n }", + LineColumn { line: 2, column: 4 }, + ), + Span::new_from_str( + "if you_are_happy && you_know_it", + LineColumn { line: 2, column: 4 } + ) + )) + )], + ); + } + + #[test] + fn test_conditional_hook_match() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + match you_are_happy && you_know_it { + true => { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + false => {} + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideConditional( + HookInfo::new( + Span::new_from_str(r#"use_state(cx, || "hands")"#, LineColumn { line: 4, column: 28 }), + Span::new_from_str(r#"use_state"#, LineColumn { line: 4, column: 28 }), + "use_state".to_string() + ), + ConditionalInfo::Match(MatchInfo::new( + Span::new_from_str( + "match you_are_happy && you_know_it {\n true => {\n let something = use_state(cx, || \"hands\");\n println!(\"clap your {something}\")\n }\n false => {}\n }", + LineColumn { line: 2, column: 4 }, + ), + Span::new_from_str("match you_are_happy && you_know_it", LineColumn { line: 2, column: 4 }) + )) + )] + ); + } + + #[test] + fn test_for_loop_hook() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + for _name in &names { + let is_selected = use_state(cx, || false); + println!("selected: {is_selected}"); + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideLoop( + HookInfo::new( + Span::new_from_str( + "use_state(cx, || false)", + LineColumn { line: 3, column: 26 }, + ), + Span::new_from_str( + "use_state", + LineColumn { line: 3, column: 26 }, + ), + "use_state".to_string() + ), + AnyLoopInfo::For(ForInfo::new( + Span::new_from_str( + "for _name in &names {\n let is_selected = use_state(cx, || false);\n println!(\"selected: {is_selected}\");\n }", + LineColumn { line: 2, column: 4 }, + ), + Span::new_from_str( + "for _name in &names", + LineColumn { line: 2, column: 4 }, + ) + )) + )] + ); + } + + #[test] + fn test_while_loop_hook() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + while true { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideLoop( + HookInfo::new( + Span::new_from_str( + r#"use_state(cx, || "hands")"#, + LineColumn { line: 3, column: 24 }, + ), + Span::new_from_str( + "use_state", + LineColumn { line: 3, column: 24 }, + ), + "use_state".to_string() + ), + AnyLoopInfo::While(WhileInfo::new( + Span::new_from_str( + "while true {\n let something = use_state(cx, || \"hands\");\n println!(\"clap your {something}\")\n }", + LineColumn { line: 2, column: 4 }, + ), + Span::new_from_str( + "while true", + LineColumn { line: 2, column: 4 }, + ) + )) + )], + ); + } + + #[test] + fn test_loop_hook() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + loop { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideLoop( + HookInfo::new( + Span::new_from_str( + r#"use_state(cx, || "hands")"#, + LineColumn { line: 3, column: 24 }, + ), + Span::new_from_str( + "use_state", + LineColumn { line: 3, column: 24 }, + ), + "use_state".to_string() + ), + AnyLoopInfo::Loop(LoopInfo::new(Span::new_from_str( + "loop {\n let something = use_state(cx, || \"hands\");\n println!(\"clap your {something}\")\n }", + LineColumn { line: 2, column: 4 }, + ))) + )], + ); + } + + #[test] + fn test_conditional_okay() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + let something = use_state(cx, || "hands"); + if you_are_happy && you_know_it { + 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#" + fn App(cx: Scope) -> Element { + let _a = || { + let b = use_state(cx, || 0); + b.get() + }; + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookInsideClosure( + HookInfo::new( + Span::new_from_str( + "use_state(cx, || 0)", + LineColumn { + line: 3, + column: 16 + }, + ), + Span::new_from_str( + "use_state", + LineColumn { + line: 3, + column: 16 + }, + ), + "use_state".to_string() + ), + ClosureInfo::new(Span::new_from_str( + "|| {\n let b = use_state(cx, || 0);\n b.get()\n }", + LineColumn { + line: 2, + column: 13 + }, + )) + )] + ); + } + + #[test] + fn test_hook_outside_component() { + let contents = indoc! {r#" + fn not_component_or_hook(cx: Scope) { + let _a = use_state(cx, || 0); + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!( + report.issues, + vec![Issue::HookOutsideComponent(HookInfo::new( + Span::new_from_str( + "use_state(cx, || 0)", + LineColumn { + line: 2, + column: 13 + } + ), + Span::new_from_str( + "use_state", + LineColumn { + line: 2, + column: 13 + }, + ), + "use_state".to_string() + ))] + ); + } + + #[test] + fn test_hook_inside_hook() { + let contents = indoc! {r#" + fn use_thing(cx: Scope) { + let _a = use_state(cx, || 0); + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } +} diff --git a/packages/check/src/issues.rs b/packages/check/src/issues.rs index 30357aa03..9893cde25 100644 --- a/packages/check/src/issues.rs +++ b/packages/check/src/issues.rs @@ -1,62 +1,88 @@ use owo_colors::{ colors::{css::LightBlue, BrightRed}, - OwoColorize, + OwoColorize, Stream, }; use std::{ fmt::Display, path::{Path, PathBuf}, }; -use crate::metadata::{AnyLoopInfo, ClosureInfo, ConditionalInfo, HookInfo}; +use crate::metadata::{ + AnyLoopInfo, ClosureInfo, ConditionalInfo, ForInfo, HookInfo, IfInfo, MatchInfo, WhileInfo, +}; /// The result of checking a Dioxus file for issues. pub struct IssueReport { pub path: PathBuf, + pub crate_root: PathBuf, pub file_content: String, pub issues: Vec, } impl IssueReport { - pub fn new(path: PathBuf, file_content: S, issues: Vec) -> Self { + pub fn new( + path: PathBuf, + crate_root: PathBuf, + file_content: S, + issues: Vec, + ) -> Self { Self { path, + crate_root, file_content: file_content.to_string(), issues, } } } +fn lightblue(text: &str) -> String { + text.if_supports_color(Stream::Stderr, |text| text.fg::()) + .to_string() +} + +fn brightred(text: &str) -> String { + text.if_supports_color(Stream::Stderr, |text| text.fg::()) + .to_string() +} + +fn bold(text: &str) -> String { + text.if_supports_color(Stream::Stderr, |text| text.bold()) + .to_string() +} + impl Display for IssueReport { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let relative_file = Path::new(&self.path) - .strip_prefix(std::env::current_dir().unwrap()) + .strip_prefix(&self.crate_root) .unwrap_or(Path::new(&self.path)) .display(); + let pipe_char = lightblue("|"); + for (i, issue) in self.issues.iter().enumerate() { let hook_info = issue.hook_info(); let hook_span = hook_info.span; let hook_name_span = hook_info.name_span; - let error_line = format!("{}: {}", "error".fg::(), issue); - writeln!(f, "{}", error_line.bold())?; + let error_line = format!("{}: {}", brightred("error"), issue); + writeln!(f, "{}", bold(&error_line))?; writeln!( f, " {} {}:{}:{}", - "-->".fg::(), + lightblue("-->"), relative_file, hook_span.start.line, hook_span.start.column + 1 )?; let max_line_num_len = hook_span.end.line.to_string().len(); - writeln!(f, "{:>max_line_num_len$} {}", "", "|".fg::())?; + writeln!(f, "{:>max_line_num_len$} {}", "", pipe_char)?; for (i, line) in self.file_content.lines().enumerate() { let line_num = i + 1; if line_num >= hook_span.start.line && line_num <= hook_span.end.line { writeln!( f, "{:>max_line_num_len$} {} {}", - line_num.fg::(), - "|".fg::(), + lightblue(&line_num.to_string()), + pipe_char, line, )?; if line_num == hook_span.start.line { @@ -71,13 +97,54 @@ impl Display for IssueReport { f, "{:>max_line_num_len$} {} {}", "", - "|".fg::(), - caret.fg::(), + pipe_char, + brightred(&caret), )?; } } } + let note_text_prefix = format!( + "{:>max_line_num_len$} {}\n{:>max_line_num_len$} {} note:", + "", + pipe_char, + "", + lightblue("=") + ); + + match issue { + Issue::HookInsideConditional( + _, + ConditionalInfo::If(IfInfo { span: _, head_span }), + ) + | Issue::HookInsideConditional( + _, + ConditionalInfo::Match(MatchInfo { span: _, head_span }), + ) => { + if let Some(source_text) = &head_span.source_text { + writeln!( + f, + "{} `{} {{ … }}` is the conditional", + note_text_prefix, source_text, + )?; + } + } + Issue::HookInsideLoop(_, AnyLoopInfo::For(ForInfo { span: _, head_span })) + | Issue::HookInsideLoop(_, AnyLoopInfo::While(WhileInfo { span: _, head_span })) => { + if let Some(source_text) = &head_span.source_text { + writeln!( + f, + "{} `{} {{ … }}` is the loop", + note_text_prefix, source_text, + )?; + } + } + Issue::HookInsideLoop(_, AnyLoopInfo::Loop(_)) => { + writeln!(f, "{} `loop {{ … }}` is the loop", note_text_prefix,)?; + } + Issue::HookOutsideComponent(_) | Issue::HookInsideClosure(_, _) => {} + } + if i < self.issues.len() - 1 { writeln!(f)?; } @@ -138,7 +205,7 @@ impl std::fmt::Display for Issue { ) } Issue::HookInsideClosure(hook_info, _) => { - write!(f, "hook called inside closure: `{}`", hook_info.name) + write!(f, "hook called in a closure: `{}`", hook_info.name) } Issue::HookOutsideComponent(hook_info) => { write!( @@ -150,3 +217,211 @@ impl std::fmt::Display for Issue { } } } + +#[cfg(test)] +mod tests { + use crate::check_file; + use indoc::indoc; + use pretty_assertions::assert_eq; + + #[test] + fn test_issue_report_display_conditional_if() { + owo_colors::set_override(false); + let issue_report = check_file( + "src/main.rs".into(), + indoc! {r#" + fn App(cx: Scope) -> Element { + if you_are_happy && you_know_it { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#}, + ); + + let expected = indoc! {r#" + error: hook called conditionally: `use_state` (inside `if`) + --> src/main.rs:3:25 + | + 3 | let something = use_state(cx, || "hands"); + | ^^^^^^^^^ + | + = note: `if you_are_happy && you_know_it { … }` is the conditional + "#}; + + assert_eq!(expected, issue_report.to_string()); + } + + #[test] + fn test_issue_report_display_conditional_match() { + owo_colors::set_override(false); + let issue_report = check_file( + "src/main.rs".into(), + indoc! {r#" + fn App(cx: Scope) -> Element { + match you_are_happy && you_know_it { + true => { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + _ => {} + } + } + "#}, + ); + + let expected = indoc! {r#" + error: hook called conditionally: `use_state` (inside `match`) + --> src/main.rs:4:29 + | + 4 | let something = use_state(cx, || "hands"); + | ^^^^^^^^^ + | + = note: `match you_are_happy && you_know_it { … }` is the conditional + "#}; + + assert_eq!(expected, issue_report.to_string()); + } + + #[test] + fn test_issue_report_display_for_loop() { + owo_colors::set_override(false); + let issue_report = check_file( + "src/main.rs".into(), + indoc! {r#" + fn App(cx: Scope) -> Element { + for i in 0..10 { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#}, + ); + + let expected = indoc! {r#" + error: hook called in a loop: `use_state` (inside `for` loop) + --> src/main.rs:3:25 + | + 3 | let something = use_state(cx, || "hands"); + | ^^^^^^^^^ + | + = note: `for i in 0..10 { … }` is the loop + "#}; + + assert_eq!(expected, issue_report.to_string()); + } + + #[test] + fn test_issue_report_display_while_loop() { + owo_colors::set_override(false); + let issue_report = check_file( + "src/main.rs".into(), + indoc! {r#" + fn App(cx: Scope) -> Element { + while check_thing() { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#}, + ); + + let expected = indoc! {r#" + error: hook called in a loop: `use_state` (inside `while` loop) + --> src/main.rs:3:25 + | + 3 | let something = use_state(cx, || "hands"); + | ^^^^^^^^^ + | + = note: `while check_thing() { … }` is the loop + "#}; + + assert_eq!(expected, issue_report.to_string()); + } + + #[test] + fn test_issue_report_display_loop() { + owo_colors::set_override(false); + let issue_report = check_file( + "src/main.rs".into(), + indoc! {r#" + fn App(cx: Scope) -> Element { + loop { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + } + } + "#}, + ); + + let expected = indoc! {r#" + error: hook called in a loop: `use_state` (inside `loop`) + --> src/main.rs:3:25 + | + 3 | let something = use_state(cx, || "hands"); + | ^^^^^^^^^ + | + = note: `loop { … }` is the loop + "#}; + + assert_eq!(expected, issue_report.to_string()); + } + + #[test] + fn test_issue_report_display_closure() { + owo_colors::set_override(false); + let issue_report = check_file( + "src/main.rs".into(), + indoc! {r#" + fn App(cx: Scope) -> Element { + let something = || { + let something = use_state(cx, || "hands"); + println!("clap your {something}") + }; + } + "#}, + ); + + let expected = indoc! {r#" + error: hook called in a closure: `use_state` + --> src/main.rs:3:25 + | + 3 | let something = use_state(cx, || "hands"); + | ^^^^^^^^^ + "#}; + + assert_eq!(expected, issue_report.to_string()); + } + + #[test] + fn test_issue_report_display_multiline_hook() { + owo_colors::set_override(false); + let issue_report = check_file( + "src/main.rs".into(), + indoc! {r#" + fn App(cx: Scope) -> Element { + if you_are_happy && you_know_it { + let something = use_state(cx, || { + "hands" + }); + println!("clap your {something}") + } + } + "#}, + ); + + let expected = indoc! {r#" + error: hook called conditionally: `use_state` (inside `if`) + --> src/main.rs:3:25 + | + 3 | let something = use_state(cx, || { + | ^^^^^^^^^ + 4 | "hands" + 5 | }); + | + = note: `if you_are_happy && you_know_it { … }` is the conditional + "#}; + + assert_eq!(expected, issue_report.to_string()); + } +} diff --git a/packages/check/src/lib.rs b/packages/check/src/lib.rs index 712462e40..3870fb610 100644 --- a/packages/check/src/lib.rs +++ b/packages/check/src/lib.rs @@ -4,476 +4,3 @@ mod metadata; pub use check::check_file; pub use issues::{Issue, IssueReport}; - -#[cfg(test)] -mod tests { - use crate::metadata::{ - AnyLoopInfo, ClosureInfo, ConditionalInfo, ForInfo, HookInfo, IfInfo, LineColumn, LoopInfo, - MatchInfo, Span, WhileInfo, - }; - - use super::*; - - #[test] - fn test_no_issues() { - let contents = r#" - fn App(cx: Scope) -> Element { - rsx! { - p { "Hello World" } - } - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!(report.issues, vec![]); - } - - #[test] - fn test_conditional_hook_if() { - let contents = r#" - fn App(cx: Scope) -> Element { - if you_are_happy && you_know_it { - let something = use_state(cx, || "hands"); - println!("clap your {something}") - } - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!( - report.issues, - vec![Issue::HookInsideConditional( - HookInfo::new( - Span { - start: LineColumn { - line: 4, - column: 36 - }, - end: LineColumn { - line: 4, - column: 61 - } - }, - Span { - start: LineColumn { - line: 4, - column: 36 - }, - end: LineColumn { - line: 4, - column: 45 - } - }, - "use_state".to_string() - ), - ConditionalInfo::If(IfInfo::new( - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 6, - column: 17 - } - }, - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 3, - column: 18 - } - } - )) - )], - ); - } - - #[test] - fn test_conditional_hook_match() { - let contents = r#" - fn App(cx: Scope) -> Element { - match you_are_happy && you_know_it { - true => { - let something = use_state(cx, || "hands"); - println!("clap your {something}") - } - false => {} - } - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!( - report.issues, - vec![Issue::HookInsideConditional( - HookInfo::new( - Span { - start: LineColumn { - line: 5, - column: 40 - }, - end: LineColumn { - line: 5, - column: 65 - } - }, - Span { - start: LineColumn { - line: 5, - column: 40 - }, - end: LineColumn { - line: 5, - column: 49 - } - }, - "use_state".to_string() - ), - ConditionalInfo::Match(MatchInfo::new( - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 9, - column: 17 - } - }, - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 3, - column: 21 - } - } - )) - )] - ); - } - - #[test] - fn test_for_loop_hook() { - let contents = r#" - fn App(cx: Scope) -> Element { - for _name in &names { - let is_selected = use_state(cx, || false); - println!("selected: {is_selected}"); - } - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!( - report.issues, - vec![Issue::HookInsideLoop( - HookInfo::new( - Span { - start: LineColumn { - line: 4, - column: 38 - }, - end: LineColumn { - line: 4, - column: 61 - } - }, - Span { - start: LineColumn { - line: 4, - column: 38 - }, - end: LineColumn { - line: 4, - column: 47 - } - }, - "use_state".to_string() - ), - AnyLoopInfo::For(ForInfo::new( - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 6, - column: 17 - } - }, - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 3, - column: 19 - } - } - )) - )] - ); - } - - #[test] - fn test_while_loop_hook() { - let contents = r#" - fn App(cx: Scope) -> Element { - while true { - let something = use_state(cx, || "hands"); - println!("clap your {something}") - } - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!( - report.issues, - vec![Issue::HookInsideLoop( - HookInfo::new( - Span { - start: LineColumn { - line: 4, - column: 36 - }, - end: LineColumn { - line: 4, - column: 61 - } - }, - Span { - start: LineColumn { - line: 4, - column: 36 - }, - end: LineColumn { - line: 4, - column: 45 - } - }, - "use_state".to_string() - ), - AnyLoopInfo::While(WhileInfo::new( - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 6, - column: 17 - } - }, - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 3, - column: 21 - } - } - )) - )], - ); - } - - #[test] - fn test_loop_hook() { - let contents = r#" - fn App(cx: Scope) -> Element { - loop { - let something = use_state(cx, || "hands"); - println!("clap your {something}") - } - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!( - report.issues, - vec![Issue::HookInsideLoop( - HookInfo::new( - Span { - start: LineColumn { - line: 4, - column: 36 - }, - end: LineColumn { - line: 4, - column: 61 - } - }, - Span { - start: LineColumn { - line: 4, - column: 36 - }, - end: LineColumn { - line: 4, - column: 45 - } - }, - "use_state".to_string() - ), - AnyLoopInfo::Loop(LoopInfo::new( - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 6, - column: 17 - } - }, - Span { - start: LineColumn { - line: 3, - column: 16 - }, - end: LineColumn { - line: 3, - column: 20 - } - } - )) - )], - ); - } - - #[test] - fn test_conditional_okay() { - let contents = r#" - fn App(cx: Scope) -> Element { - let something = use_state(cx, || "hands"); - if you_are_happy && you_know_it { - println!("clap your {something}") - } - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!(report.issues, vec![]); - } - - #[test] - fn test_closure_hook() { - let contents = r#" - fn App(cx: Scope) -> Element { - let _a = || { - let b = use_state(cx, || 0); - b.get() - }; - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!( - report.issues, - vec![Issue::HookInsideClosure( - HookInfo::new( - Span { - start: LineColumn { - line: 4, - column: 28 - }, - end: LineColumn { - line: 4, - column: 47 - } - }, - Span { - start: LineColumn { - line: 4, - column: 28 - }, - end: LineColumn { - line: 4, - column: 37 - } - }, - "use_state".to_string() - ), - ClosureInfo::new(Span { - start: LineColumn { - line: 3, - column: 25 - }, - end: LineColumn { - line: 6, - column: 17 - } - }) - )] - ); - } - - #[test] - fn test_hook_outside_component() { - let contents = r#" - fn not_component_or_hook(cx: Scope) { - let _a = use_state(cx, || 0); - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!( - report.issues, - vec![Issue::HookOutsideComponent(HookInfo::new( - Span { - start: LineColumn { - line: 3, - column: 25 - }, - end: LineColumn { - line: 3, - column: 44 - } - }, - Span { - start: LineColumn { - line: 3, - column: 25 - }, - end: LineColumn { - line: 3, - column: 34 - } - }, - "use_state".to_string() - ))] - ); - } - - #[test] - fn test_hook_inside_hook() { - let contents = r#" - fn use_thing(cx: Scope) { - let _a = use_state(cx, || 0); - } - "#; - - let report = check_file("app.rs".into(), contents); - - assert_eq!(report.issues, vec![]); - } -} diff --git a/packages/check/src/metadata.rs b/packages/check/src/metadata.rs index d886dcf32..28ab87b74 100644 --- a/packages/check/src/metadata.rs +++ b/packages/check/src/metadata.rs @@ -29,13 +29,13 @@ pub enum ConditionalInfo { pub struct IfInfo { /// The span of the `if` statement, e.g. `if true { ... }`. pub span: Span, - /// The span of the `if` keyword only. - pub keyword_span: Span, + /// The span of the `if true` part only. + pub head_span: Span, } impl IfInfo { - pub const fn new(span: Span, keyword_span: Span) -> Self { - Self { span, keyword_span } + pub const fn new(span: Span, head_span: Span) -> Self { + Self { span, head_span } } } @@ -43,13 +43,13 @@ impl IfInfo { pub struct MatchInfo { /// The span of the `match` statement, e.g. `match true { ... }`. pub span: Span, - /// The span of the `match` keyword only. - pub keyword_span: Span, + /// The span of the `match true` part only. + pub head_span: Span, } impl MatchInfo { - pub const fn new(span: Span, keyword_span: Span) -> Self { - Self { span, keyword_span } + pub const fn new(span: Span, head_span: Span) -> Self { + Self { span, head_span } } } @@ -65,12 +65,12 @@ pub enum AnyLoopInfo { /// Information about a `for` loop. pub struct ForInfo { pub span: Span, - pub keyword_span: Span, + pub head_span: Span, } impl ForInfo { - pub const fn new(span: Span, keyword_span: Span) -> Self { - Self { span, keyword_span } + pub const fn new(span: Span, head_span: Span) -> Self { + Self { span, head_span } } } @@ -78,12 +78,12 @@ impl ForInfo { /// Information about a `while` loop. pub struct WhileInfo { pub span: Span, - pub keyword_span: Span, + pub head_span: Span, } impl WhileInfo { - pub const fn new(span: Span, keyword_span: Span) -> Self { - Self { span, keyword_span } + pub const fn new(span: Span, head_span: Span) -> Self { + Self { span, head_span } } } @@ -91,12 +91,11 @@ impl WhileInfo { /// Information about a `loop` loop. pub struct LoopInfo { pub span: Span, - pub keyword_span: Span, } impl LoopInfo { - pub const fn new(span: Span, keyword_span: Span) -> Self { - Self { span, keyword_span } + pub const fn new(span: Span) -> Self { + Self { span } } } @@ -151,13 +150,35 @@ impl FnInfo { #[derive(Debug, Clone, PartialEq, Eq)] /// A span of text in a source code file. pub struct Span { + pub source_text: Option, pub start: LineColumn, pub end: LineColumn, } +impl Span { + pub fn new_from_str(source_text: &str, start: LineColumn) -> Self { + let mut lines = source_text.lines(); + let first_line = lines.next().unwrap_or_default(); + let mut end = LineColumn { + line: start.line, + column: start.column + first_line.len(), + }; + for line in lines { + end.line += 1; + end.column = line.len(); + } + Self { + source_text: Some(source_text.to_string()), + start, + end, + } + } +} + impl From for Span { fn from(span: proc_macro2::Span) -> Self { Self { + source_text: span.source_text(), start: span.start().into(), end: span.end().into(), } From 8f8627af1846f571416944692d2e990c17ef4a02 Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Sun, 23 Jul 2023 15:40:25 -0700 Subject: [PATCH 5/7] attempt to hoist `prettier-please` dependencies --- Cargo.toml | 3 +++ packages/autofmt/Cargo.toml | 4 +--- packages/check/Cargo.toml | 4 +--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a493d98b3..93a556b6d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,9 @@ rustc-hash = "1.1.0" wasm-bindgen = "0.2.87" html_parser = "0.7.0" thiserror = "1.0.40" +prettyplease = { package = "prettier-please", version = "0.2", features = [ + "verbatim", +] } # This is a "virtual package" # It is not meant to be published, but is used so "cargo run --example XYZ" works properly diff --git a/packages/autofmt/Cargo.toml b/packages/autofmt/Cargo.toml index 7cebf25bc..c0a3002b3 100644 --- a/packages/autofmt/Cargo.toml +++ b/packages/autofmt/Cargo.toml @@ -16,9 +16,7 @@ proc-macro2 = { version = "1.0.6", features = ["span-locations"] } quote = "1.0" syn = { version = "2.0", features = ["full", "extra-traits", "visit"] } serde = { version = "1.0.136", features = ["derive"] } -prettyplease = { package = "prettier-please", version = "0.2", features = [ - "verbatim", -] } +prettyplease = { workspace = true } [dev-dependencies] pretty_assertions = "1.2.1" diff --git a/packages/check/Cargo.toml b/packages/check/Cargo.toml index 701d845b3..2b793331c 100644 --- a/packages/check/Cargo.toml +++ b/packages/check/Cargo.toml @@ -16,10 +16,8 @@ proc-macro2 = { version = "1.0.6", features = ["span-locations"] } quote = "1.0" syn = { version = "1.0.11", features = ["full", "extra-traits", "visit"] } serde = { version = "1.0.136", features = ["derive"] } -prettyplease = { package = "prettier-please", version = "0.1.16", features = [ - "verbatim", -] } owo-colors = { version = "3.5.0", features = ["supports-colors"] } +prettyplease = { workspace = true } [dev-dependencies] indoc = "2.0.3" From 420bc39ecc00b88f523fb816f947b8396ff18094 Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Sun, 23 Jul 2023 18:57:23 -0700 Subject: [PATCH 6/7] docs(cli): re-sync `dx --help` output --- packages/cli/docs/src/cmd/README.md | 33 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/cli/docs/src/cmd/README.md b/packages/cli/docs/src/cmd/README.md index 2de7cc138..93f6f5573 100644 --- a/packages/cli/docs/src/cmd/README.md +++ b/packages/cli/docs/src/cmd/README.md @@ -5,22 +5,25 @@ In this chapter we will introduce all `dioxus-cli` commands. > You can also use `dx --help` to get cli help info. ``` -dx -Build, bundle, & ship your Dioxus app +Build, Bundle & Ship Dioxus Apps -USAGE: - dx [OPTIONS] +Usage: dx [OPTIONS] -OPTIONS: - -h, --help Print help information - -v Enable verbose logging +Commands: + build Build the Rust WASM app and all of its assets + translate Translate some source file into Dioxus code + serve Build, watch & serve the Rust WASM app and all of its assets + create Init a new project for Dioxus + clean Clean output artifacts + version Print the version of this extension + fmt Format some rsx + check Check the Rust files in the project for issues + config Dioxus config file controls + help Print this message or the help of the given subcommand(s) -SUBCOMMANDS: - build Build the Dioxus application and all of its assets - clean Clean output artifacts - config Dioxus config file controls - create Init a new project for Dioxus - help Print this message or the help of the given subcommand(s) - serve Build, watch & serve the Rust WASM app and all of its assets - translate Translate some html file into a Dioxus component +Options: + -v Enable verbose logging + --bin Specify bin target + -h, --help Print help + -V, --version Print version ``` From 0e841f84117c5261445c80d713cb1f6a65fe7750 Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Mon, 24 Jul 2023 18:58:19 -0700 Subject: [PATCH 7/7] fix(check): allow hook calls inside closure hooks --- packages/check/src/check.rs | 77 ++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/packages/check/src/check.rs b/packages/check/src/check.rs index b8298689b..c3a8ecf2f 100644 --- a/packages/check/src/check.rs +++ b/packages/check/src/check.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use syn::{spanned::Spanned, visit::Visit}; +use syn::{spanned::Spanned, visit::Visit, Pat}; use crate::{ issues::{Issue, IssueReport}, @@ -74,6 +74,20 @@ fn is_component_fn(item_fn: &syn::ItemFn) -> bool { returns_element(&item_fn.sig.output) } +fn get_closure_hook_body(local: &syn::Local) -> Option<&syn::Expr> { + if let Pat::Ident(ident) = &local.pat { + if is_hook_ident(&ident.ident) { + if let Some((_, expr)) = &local.init { + if let syn::Expr::Closure(closure) = &**expr { + return Some(&closure.body); + } + } + } + } + + None +} + fn fn_name_and_name_span(item_fn: &syn::ItemFn) -> (String, Span) { let name = item_fn.sig.ident.to_string(); let name_span = item_fn.sig.ident.span().into(); @@ -173,6 +187,17 @@ impl<'ast> syn::visit::Visit<'ast> for VisitHooks { self.context.pop(); } + fn visit_local(&mut self, i: &'ast syn::Local) { + if let Some(body) = get_closure_hook_body(i) { + // if the closure is a hook, we only visit the body of the closure. + // this prevents adding a ClosureInfo node to the context + syn::visit::visit_expr(self, body); + } else { + // otherwise visit the whole local + syn::visit::visit_local(self, i); + } + } + fn visit_expr_if(&mut self, i: &'ast syn::ExprIf) { self.context.push(Node::If(IfInfo::new( i.span().into(), @@ -252,7 +277,7 @@ mod tests { use super::*; #[test] - fn test_no_issues() { + fn test_no_hooks() { let contents = indoc! {r#" fn App(cx: Scope) -> Element { rsx! { @@ -266,6 +291,54 @@ mod tests { assert_eq!(report.issues, vec![]); } + #[test] + fn test_hook_correctly_used_inside_component() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + let count = use_state(cx, || 0); + rsx! { + p { "Hello World: {count}" } + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + + #[test] + fn test_hook_correctly_used_inside_hook_fn() { + let contents = indoc! {r#" + fn use_thing(cx: Scope) -> UseState { + use_state(cx, || 0) + } + "#}; + + let report = check_file("use_thing.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + + #[test] + fn test_hook_correctly_used_inside_hook_closure() { + let contents = indoc! {r#" + fn App(cx: Scope) -> Element { + let use_thing = || { + use_state(cx, || 0) + }; + let count = use_thing(); + rsx! { + p { "Hello World: {count}" } + } + } + "#}; + + let report = check_file("app.rs".into(), contents); + + assert_eq!(report.issues, vec![]); + } + #[test] fn test_conditional_hook_if() { let contents = indoc! {r#"