From b6ea722d9452b41afcd63d4d9b33d2a92d7fbf77 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 19 Jul 2023 16:59:20 -0700 Subject: [PATCH 1/2] fix some edge cases around multiline attributes --- packages/autofmt/Cargo.toml | 2 +- packages/autofmt/src/collect_macros.rs | 170 ++----------------------- packages/autofmt/src/component.rs | 9 +- packages/autofmt/src/element.rs | 21 ++- packages/autofmt/src/expr.rs | 44 +++---- packages/autofmt/src/lib.rs | 20 ++- packages/cli/src/cli/autoformat.rs | 16 ++- 7 files changed, 77 insertions(+), 205 deletions(-) diff --git a/packages/autofmt/Cargo.toml b/packages/autofmt/Cargo.toml index 254b60093..a76b0fcd4 100644 --- a/packages/autofmt/Cargo.toml +++ b/packages/autofmt/Cargo.toml @@ -14,7 +14,7 @@ keywords = ["dom", "ui", "gui", "react"] 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"] } +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", diff --git a/packages/autofmt/src/collect_macros.rs b/packages/autofmt/src/collect_macros.rs index ebb6314e2..8c2fe22f8 100644 --- a/packages/autofmt/src/collect_macros.rs +++ b/packages/autofmt/src/collect_macros.rs @@ -3,178 +3,24 @@ //! Returns all macros that match a pattern. You can use this information to autoformat them later use proc_macro2::LineColumn; -use syn::{Block, Expr, File, Item, Macro, Stmt}; +use syn::{visit::Visit, File, Macro}; type CollectedMacro<'a> = &'a Macro; -pub fn collect_from_file<'a>(file: &'a File, macros: &mut Vec>) { - for item in file.items.iter() { - collect_from_item(item, macros); - } +pub fn collect_from_file<'a, 'b>(file: &'a File, macros: &'b mut Vec>) { + MacroCollector::visit_file(&mut MacroCollector { macros }, file); } -pub fn collect_from_item<'a>(item: &'a Item, macros: &mut Vec>) { - match item { - Item::Fn(f) => collect_from_block(&f.block, macros), - - // Ignore macros if they're not rsx or render - Item::Macro(macro_) => { - if macro_.mac.path.segments[0].ident == "rsx" - || macro_.mac.path.segments[0].ident == "render" - { - macros.push(¯o_.mac); - } - } - - // Currently disabled since we're not focused on autoformatting these - Item::Impl(_imp) => {} - Item::Trait(_) => {} - - // Global-ish things - Item::Static(f) => collect_from_expr(&f.expr, macros), - Item::Const(f) => collect_from_expr(&f.expr, macros), - Item::Mod(s) => { - if let Some((_, block)) = &s.content { - for item in block { - collect_from_item(item, macros); - } - } - } - - // None of these we can really do anything with at the item level - Item::Macro2(_) - | Item::Enum(_) - | Item::ExternCrate(_) - | Item::ForeignMod(_) - | Item::TraitAlias(_) - | Item::Type(_) - | Item::Struct(_) - | Item::Union(_) - | Item::Use(_) - | Item::Verbatim(_) => {} - _ => {} - } +struct MacroCollector<'a, 'b> { + macros: &'a mut Vec>, } -pub fn collect_from_block<'a>(block: &'a Block, macros: &mut Vec>) { - for stmt in &block.stmts { - match stmt { - Stmt::Item(item) => collect_from_item(item, macros), - Stmt::Local(local) => { - if let Some((_eq, init)) = &local.init { - collect_from_expr(init, macros); - } - } - Stmt::Expr(exp) | Stmt::Semi(exp, _) => collect_from_expr(exp, macros), - } +impl<'a, 'b> Visit<'b> for MacroCollector<'a, 'b> { + fn visit_macro(&mut self, i: &'b Macro) { + self.macros.push(i); } } -pub fn collect_from_expr<'a>(expr: &'a Expr, macros: &mut Vec>) { - // collect an expr from the exprs, descending into blocks - match expr { - Expr::Macro(macro_) => { - if macro_.mac.path.segments[0].ident == "rsx" - || macro_.mac.path.segments[0].ident == "render" - { - macros.push(¯o_.mac); - } - } - - Expr::MethodCall(e) => { - collect_from_expr(&e.receiver, macros); - for expr in e.args.iter() { - collect_from_expr(expr, macros); - } - } - Expr::Assign(exp) => { - collect_from_expr(&exp.left, macros); - collect_from_expr(&exp.right, macros); - } - - Expr::Async(b) => collect_from_block(&b.block, macros), - Expr::Block(b) => collect_from_block(&b.block, macros), - Expr::Closure(c) => collect_from_expr(&c.body, macros), - Expr::Let(l) => collect_from_expr(&l.expr, macros), - Expr::Unsafe(u) => collect_from_block(&u.block, macros), - Expr::Loop(l) => collect_from_block(&l.body, macros), - - Expr::Call(c) => { - collect_from_expr(&c.func, macros); - for expr in c.args.iter() { - collect_from_expr(expr, macros); - } - } - - Expr::ForLoop(b) => { - collect_from_expr(&b.expr, macros); - collect_from_block(&b.body, macros); - } - Expr::If(f) => { - collect_from_expr(&f.cond, macros); - collect_from_block(&f.then_branch, macros); - if let Some((_, else_branch)) = &f.else_branch { - collect_from_expr(else_branch, macros); - } - } - Expr::Yield(y) => { - if let Some(expr) = &y.expr { - collect_from_expr(expr, macros); - } - } - - Expr::Return(r) => { - if let Some(expr) = &r.expr { - collect_from_expr(expr, macros); - } - } - - Expr::Match(l) => { - collect_from_expr(&l.expr, macros); - for arm in l.arms.iter() { - if let Some((_, expr)) = &arm.guard { - collect_from_expr(expr, macros); - } - - collect_from_expr(&arm.body, macros); - } - } - - Expr::While(w) => { - collect_from_expr(&w.cond, macros); - collect_from_block(&w.body, macros); - } - - // don't both formatting these for now - Expr::Array(_) - | Expr::AssignOp(_) - | Expr::Await(_) - | Expr::Binary(_) - | Expr::Box(_) - | Expr::Break(_) - | Expr::Cast(_) - | Expr::Continue(_) - | Expr::Field(_) - | Expr::Group(_) - | Expr::Index(_) - | Expr::Lit(_) - | Expr::Paren(_) - | Expr::Path(_) - | Expr::Range(_) - | Expr::Reference(_) - | Expr::Repeat(_) - | Expr::Struct(_) - | Expr::Try(_) - | Expr::TryBlock(_) - | Expr::Tuple(_) - | Expr::Type(_) - | Expr::Unary(_) - | Expr::Verbatim(_) => {} - - _ => {} - }; -} - pub fn byte_offset(input: &str, location: LineColumn) -> usize { let mut offset = 0; for _ in 1..location.line { diff --git a/packages/autofmt/src/component.rs b/packages/autofmt/src/component.rs index d5d92b79a..8cf36811d 100644 --- a/packages/autofmt/src/component.rs +++ b/packages/autofmt/src/component.rs @@ -165,7 +165,14 @@ impl Writer<'_> { match &field.content { ContentField::ManExpr(exp) => { let out = prettyplease::unparse_expr(exp); - write!(self.out, "{name}: {out}")?; + let mut lines = out.split('\n').peekable(); + let first = lines.next().unwrap(); + write!(self.out, "{name}: {first}")?; + for line in lines { + self.out.new_line()?; + self.out.indented_tab()?; + write!(self.out, "{line}")?; + } } ContentField::Formatted(s) => { write!( diff --git a/packages/autofmt/src/element.rs b/packages/autofmt/src/element.rs index c541c8f5c..dbc336401 100644 --- a/packages/autofmt/src/element.rs +++ b/packages/autofmt/src/element.rs @@ -216,7 +216,26 @@ impl Writer<'_> { } ElementAttr::AttrExpression { name, value } => { let out = prettyplease::unparse_expr(value); - write!(self.out, "{name}: {out}")?; + let mut lines = out.split('\n').peekable(); + let first = lines.next().unwrap(); + + // a one-liner for whatever reason + // Does not need a new line + if lines.peek().is_none() { + write!(self.out, "{name}: {first}")?; + } else { + writeln!(self.out, "{name}: {first}")?; + + while let Some(line) = lines.next() { + self.out.indented_tab()?; + write!(self.out, "{line}")?; + if lines.peek().is_none() { + write!(self.out, "")?; + } else { + writeln!(self.out)?; + } + } + } } ElementAttr::CustomAttrText { name, value } => { diff --git a/packages/autofmt/src/expr.rs b/packages/autofmt/src/expr.rs index ddc0892f8..56f2e2254 100644 --- a/packages/autofmt/src/expr.rs +++ b/packages/autofmt/src/expr.rs @@ -27,41 +27,29 @@ impl Writer<'_> { // If the expr is multiline, we want to collect all of its lines together and write them out properly // This involves unshifting the first line if it's aligned let first_line = &self.src[start.line - 1]; - write!( - self.out, - "{}", - &first_line[start.column - 1..first_line.len()].trim() - )?; + write!(self.out, "{}", &first_line[start.column - 1..].trim_start())?; - let first_prefix = &self.src[start.line - 1][..start.column]; - let offset = match first_prefix.trim() { - "" => 0, - _ => first_prefix - .chars() - .rev() - .take_while(|c| c.is_whitespace()) - .count() as isize, - }; + let prev_block_indent_level = crate::leading_whitespaces(&first_line) / 4; for (id, line) in self.src[start.line..end.line].iter().enumerate() { writeln!(self.out)?; - // trim the leading whitespace - let line = match id { - x if x == (end.line - start.line) - 1 => &line[..end.column], - _ => line, + // check if this is the last line + let line = { + if id == (end.line - start.line) - 1 { + &line[..end.column] + } else { + line + } }; - if offset < 0 { - for _ in 0..-offset { - write!(self.out, " ")?; - } + // trim the leading whitespace + let previous_indent = crate::leading_whitespaces(line) / 4; + let offset = previous_indent.saturating_sub(prev_block_indent_level); + let required_indent = self.out.indent + offset; + self.out.write_tabs(required_indent)?; - write!(self.out, "{line}")?; - } else { - let offset = offset as usize; - let right = &line[offset..]; - write!(self.out, "{right}")?; - } + let line = line.trim_start(); + write!(self.out, "{line}")?; } Ok(()) diff --git a/packages/autofmt/src/lib.rs b/packages/autofmt/src/lib.rs index 0d2426a0b..5f7e4da03 100644 --- a/packages/autofmt/src/lib.rs +++ b/packages/autofmt/src/lib.rs @@ -58,7 +58,7 @@ pub fn fmt_file(contents: &str) -> Vec { let mut writer = Writer::new(contents); - // Dont parse nested macros + // Don't parse nested macros let mut end_span = LineColumn { column: 0, line: 0 }; for item in macros { let macro_path = &item.path.segments[0].ident; @@ -68,16 +68,11 @@ pub fn fmt_file(contents: &str) -> Vec { continue; } - // item.parse_body::(); let body = item.parse_body::().unwrap(); let rsx_start = macro_path.span().start(); - writer.out.indent = &writer.src[rsx_start.line - 1] - .chars() - .take_while(|c| *c == ' ') - .count() - / 4; + writer.out.indent = leading_whitespaces(&writer.src[rsx_start.line - 1]) / 4; write_body(&mut writer, &body); @@ -230,3 +225,14 @@ pub(crate) fn write_ifmt(input: &IfmtInput, writable: &mut impl Write) -> std::f let display = DisplayIfmt(input); write!(writable, "{}", display) } + +pub fn leading_whitespaces(input: &str) -> usize { + input + .chars() + .map_while(|c| match c { + ' ' => Some(1), + '\t' => Some(4), + _ => None, + }) + .sum() +} diff --git a/packages/cli/src/cli/autoformat.rs b/packages/cli/src/cli/autoformat.rs index ea2c27148..a0c0a8066 100644 --- a/packages/cli/src/cli/autoformat.rs +++ b/packages/cli/src/cli/autoformat.rs @@ -109,11 +109,17 @@ async fn autoformat_project(check: bool) -> Result<()> { }) .await; - if res.is_err() { - eprintln!("error formatting file: {}", _path.display()); + match res { + Err(err) => { + eprintln!("error formatting file: {}\n{err}", _path.display()); + None + } + Ok(Err(err)) => { + eprintln!("error formatting file: {}\n{err}", _path.display()); + None + } + Ok(Ok(res)) => Some(res), } - - res }) .collect::>() .collect::>() @@ -122,7 +128,7 @@ async fn autoformat_project(check: bool) -> Result<()> { let files_formatted: usize = counts .into_iter() .map(|f| match f { - Ok(Ok(res)) => res, + Some(res) => res, _ => 0, }) .sum(); From 330950140c0e7477584506f568d1022f6394e4b8 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 19 Jul 2023 18:16:32 -0700 Subject: [PATCH 2/2] fix clippy --- packages/autofmt/src/collect_macros.rs | 2 +- packages/autofmt/src/expr.rs | 2 +- packages/autofmt/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/autofmt/src/collect_macros.rs b/packages/autofmt/src/collect_macros.rs index 8c2fe22f8..10e0d185c 100644 --- a/packages/autofmt/src/collect_macros.rs +++ b/packages/autofmt/src/collect_macros.rs @@ -7,7 +7,7 @@ use syn::{visit::Visit, File, Macro}; type CollectedMacro<'a> = &'a Macro; -pub fn collect_from_file<'a, 'b>(file: &'a File, macros: &'b mut Vec>) { +pub fn collect_from_file<'a>(file: &'a File, macros: &mut Vec>) { MacroCollector::visit_file(&mut MacroCollector { macros }, file); } diff --git a/packages/autofmt/src/expr.rs b/packages/autofmt/src/expr.rs index 56f2e2254..b0257f8c3 100644 --- a/packages/autofmt/src/expr.rs +++ b/packages/autofmt/src/expr.rs @@ -29,7 +29,7 @@ impl Writer<'_> { let first_line = &self.src[start.line - 1]; write!(self.out, "{}", &first_line[start.column - 1..].trim_start())?; - let prev_block_indent_level = crate::leading_whitespaces(&first_line) / 4; + let prev_block_indent_level = crate::leading_whitespaces(first_line) / 4; for (id, line) in self.src[start.line..end.line].iter().enumerate() { writeln!(self.out)?; diff --git a/packages/autofmt/src/lib.rs b/packages/autofmt/src/lib.rs index 5f7e4da03..08e2eadd7 100644 --- a/packages/autofmt/src/lib.rs +++ b/packages/autofmt/src/lib.rs @@ -72,7 +72,7 @@ pub fn fmt_file(contents: &str) -> Vec { let rsx_start = macro_path.span().start(); - writer.out.indent = leading_whitespaces(&writer.src[rsx_start.line - 1]) / 4; + writer.out.indent = leading_whitespaces(writer.src[rsx_start.line - 1]) / 4; write_body(&mut writer, &body);