Merge pull request #766 from DioxusLabs/jk/autofmt-loops-and-if

feat: parse entire file instead of hunting for verbatim

packages/autofmt/Cargo.toml

@@ -6,11 +6,10 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-dioxus-rsx = { path = "../rsx", version = "0.0.2"}
+dioxus-rsx = { path = "../rsx", version = "0.0.2" }
 proc-macro2 = { version = "1.0.6", features = ["span-locations"] }
 quote = "1.0"
 syn = { version = "1.0.11", features = ["full", "extra-traits"] }
-triple_accel = "0.4.0"
 serde = { version = "1.0.136", features = ["derive"] }
 prettyplease = { git = "https://github.com/DioxusLabs/prettyplease-macro-fmt.git", features = [
     "verbatim",

packages/autofmt/src/collect_macros.rs

@@ -0,0 +1,198 @@
+//! Collect macros from a file
+//!
+//! 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};
+
+type CollectedMacro<'a> = &'a Macro;
+
+pub fn collect_from_file<'a>(file: &'a File, macros: &mut Vec<CollectedMacro<'a>>) {
+    for item in file.items.iter() {
+        collect_from_item(item, macros);
+    }
+}
+
+pub fn collect_from_item<'a>(item: &'a Item, macros: &mut Vec<CollectedMacro<'a>>) {
+    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(&macro_.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(_) => {}
+        _ => {}
+    }
+}
+
+pub fn collect_from_block<'a>(block: &'a Block, macros: &mut Vec<CollectedMacro<'a>>) {
+    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),
+        }
+    }
+}
+
+pub fn collect_from_expr<'a>(expr: &'a Expr, macros: &mut Vec<CollectedMacro<'a>>) {
+    // 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(&macro_.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(_) => {}
+
+        _ => todo!(),
+    };
+}
+
+pub fn byte_offset(input: &str, location: LineColumn) -> usize {
+    let mut offset = 0;
+    for _ in 1..location.line {
+        offset += input[offset..].find('\n').unwrap() + 1;
+    }
+    offset
+        + input[offset..]
+            .chars()
+            .take(location.column)
+            .map(char::len_utf8)
+            .sum::<usize>()
+}
+
+#[test]
+fn parses_file_and_collects_rsx_macros() {
+    let contents = include_str!("../tests/samples/long.rsx");
+    let parsed = syn::parse_file(contents).unwrap();
+    let mut macros = vec![];
+    collect_from_file(&parsed, &mut macros);
+    assert_eq!(macros.len(), 3);
+}

packages/autofmt/src/component.rs

@@ -19,7 +19,7 @@ enum ShortOptimization {
     NoOpt,
 }
 
-impl Writer {
+impl Writer<'_> {
     pub fn write_component(
         &mut self,
         Component {

packages/autofmt/src/element.rs

@@ -40,7 +40,7 @@ div {
 }
 */
 
-impl Writer {
+impl Writer<'_> {
     pub fn write_element(&mut self, el: &Element) -> Result {
         let Element {
             name,
@@ -276,7 +276,7 @@ impl Writer {
         let start = location.start();
         let line_start = start.line - 1;
 
-        let this_line = self.src[line_start].as_str();
+        let this_line = self.src[line_start];
 
         let beginning = if this_line.len() > start.column {
             this_line[..start.column].trim()

packages/autofmt/src/expr.rs

@@ -5,7 +5,7 @@ use proc_macro2::Span;
 
 use crate::Writer;
 
-impl Writer {
+impl Writer<'_> {
     pub fn write_raw_expr(&mut self, placement: Span) -> Result {
         /*
         We want to normalize the expr to the appropriate indent level.

packages/autofmt/src/lib.rs

@@ -1,13 +1,14 @@
-use dioxus_rsx::CallBody;
-
-use crate::util::*;
 use crate::writer::*;
+use collect_macros::byte_offset;
+use dioxus_rsx::CallBody;
+use proc_macro2::LineColumn;
+use syn::{ExprMacro, MacroDelimiter};
 
 mod buffer;
+mod collect_macros;
 mod component;
 mod element;
 mod expr;
-mod util;
 mod writer;
 
 /// A modification to the original file to be applied by an IDE
@@ -41,65 +42,83 @@ pub struct FormattedBlock {
 /// Nested blocks of RSX will be handled automatically
 pub fn fmt_file(contents: &str) -> Vec<FormattedBlock> {
     let mut formatted_blocks = Vec::new();
-    let mut last_bracket_end = 0;
 
-    use triple_accel::{levenshtein_search, Match};
+    let parsed = syn::parse_file(contents).unwrap();
+
+    let mut macros = vec![];
+    collect_macros::collect_from_file(&parsed, &mut macros);
+
+    // No macros, no work to do
+    if macros.is_empty() {
+        return formatted_blocks;
+    }
+
+    let mut writer = Writer {
+        src: contents.lines().collect::<Vec<_>>(),
+        ..Writer::default()
+    };
 
-    for Match { end, start, k } in levenshtein_search(b"rsx! {", contents.as_bytes()) {
-        let open = end;
+    // Dont parse nested macros
+    let mut end_span = LineColumn { column: 0, line: 0 };
+    for item in macros {
+        let macro_path = &item.path.segments[0].ident;
 
-        if k > 1 {
+        // this macro is inside the last macro we parsed, skip it
+        if macro_path.span().start() < end_span {
             continue;
         }
 
-        // ensure the marker is not nested
-        if start < last_bracket_end {
-            continue;
+        // item.parse_body::<CallBody>();
+        let body = item.parse_body::<CallBody>().unwrap();
+
+        let rsx_start = macro_path.span().start();
+
+        writer.out.indent = &writer.src[rsx_start.line - 1]
+            .chars()
+            .take_while(|c| *c == ' ')
+            .count()
+            / 4;
+
+        // Oneliner optimization
+        if writer.is_short_children(&body.roots).is_some() {
+            writer.write_ident(&body.roots[0]).unwrap();
+        } else {
+            writer.write_body_indented(&body.roots).unwrap();
         }
 
-        let indent_level = {
-            // walk backwards from start until we find a new line
-            let mut lines = contents[..start].lines().rev();
-            match lines.next() {
-                Some(line) => {
-                    if line.starts_with("//") || line.starts_with("///") {
-                        continue;
-                    }
-
-                    line.chars().take_while(|c| *c == ' ').count() / 4
-                }
-                None => 0,
-            }
-        };
+        // writing idents leaves the final line ended at the end of the last ident
+        if writer.out.buf.contains('\n') {
+            writer.out.new_line().unwrap();
+            writer.out.tab().unwrap();
+        }
 
-        let remaining = &contents[open - 1..];
-        let close = find_bracket_end(remaining).unwrap();
-        // Move the last bracket end to the end of this block to avoid nested blocks
-        last_bracket_end = close + open - 1;
+        let span = match item.delimiter {
+            MacroDelimiter::Paren(_) => todo!(),
+            MacroDelimiter::Brace(b) => b.span,
+            MacroDelimiter::Bracket(_) => todo!(),
+        };
 
-        // Format the substring, doesn't include the outer brackets
-        let substring = &remaining[1..close - 1];
+        let mut formatted = String::new();
 
-        // make sure to add back whatever weird whitespace there was at the end
-        let mut remaining_whitespace = substring.chars().rev().take_while(|c| *c == ' ').count();
+        std::mem::swap(&mut formatted, &mut writer.out.buf);
 
-        let mut new = fmt_block(substring, indent_level).unwrap();
+        let start = byte_offset(contents, span.start()) + 1;
+        let end = byte_offset(contents, span.end()) - 1;
 
-        // if the new string is not multiline, don't try to adjust the marker ending
-        // We want to trim off any indentation that there might be
-        if new.len() <= 80 && !new.contains('\n') {
-            new = format!(" {new} ");
-            remaining_whitespace = 0;
+        if formatted.len() <= 80 && !formatted.contains('\n') {
+            formatted = format!(" {} ", formatted);
         }
 
-        if new == substring {
+        end_span = span.end();
+
+        if contents[start..end] == formatted {
             continue;
         }
 
         formatted_blocks.push(FormattedBlock {
-            formatted: new,
-            start: open,
-            end: last_bracket_end - remaining_whitespace - 1,
+            formatted,
+            start,
+            end,
         });
     }
 
@@ -108,7 +127,25 @@ pub fn fmt_file(contents: &str) -> Vec<FormattedBlock> {
 
 pub fn write_block_out(body: CallBody) -> Option<String> {
     let mut buf = Writer {
-        src: vec!["".to_string()],
+        src: vec![""],
+        ..Writer::default()
+    };
+
+    // Oneliner optimization
+    if buf.is_short_children(&body.roots).is_some() {
+        buf.write_ident(&body.roots[0]).unwrap();
+    } else {
+        buf.write_body_indented(&body.roots).unwrap();
+    }
+
+    buf.consume()
+}
+
+pub fn fmt_block_from_expr(raw: &str, expr: ExprMacro) -> Option<String> {
+    let body = syn::parse2::<CallBody>(expr.mac.tokens).unwrap();
+
+    let mut buf = Writer {
+        src: raw.lines().collect(),
         ..Writer::default()
     };
 
@@ -126,7 +163,7 @@ pub fn fmt_block(block: &str, indent_level: usize) -> Option<String> {
     let body = syn::parse_str::<dioxus_rsx::CallBody>(block).unwrap();
 
     let mut buf = Writer {
-        src: block.lines().map(|f| f.to_string()).collect(),
+        src: block.lines().collect(),
         ..Writer::default()
     };
 

packages/autofmt/src/util.rs

@@ -1,20 +0,0 @@
-pub fn find_bracket_end(contents: &str) -> Option<usize> {
-    let mut depth = 0;
-
-    let mut len = 0;
-
-    for c in contents.chars() {
-        len += c.len_utf8();
-        if c == '{' {
-            depth += 1;
-        } else if c == '}' {
-            depth -= 1;
-
-            if depth == 0 {
-                return Some(len);
-            }
-        }
-    }
-
-    None
-}

packages/autofmt/src/writer.rs

@@ -10,8 +10,8 @@ use syn::{spanned::Spanned, Expr, ExprIf};
 use crate::buffer::Buffer;
 
 #[derive(Debug, Default)]
-pub struct Writer {
-    pub src: Vec<String>,
+pub struct Writer<'a> {
+    pub src: Vec<&'a str>,
     pub cached_formats: HashMap<Location, String>,
     pub comments: VecDeque<usize>,
     pub out: Buffer,
@@ -31,7 +31,7 @@ impl Location {
     }
 }
 
-impl Writer {
+impl Writer<'_> {
     // Expects to be written directly into place
     pub fn write_ident(&mut self, node: &BodyNode) -> Result {
         match node {

packages/autofmt/tests/samples.rs

@@ -35,5 +35,6 @@ twoway![
     emoji,
     messy_indent,
     long_exprs,
-    ifchain_forloop
+    ifchain_forloop,
+    t2
 ];

packages/autofmt/tests/samples/messy_indent.rsx

@@ -9,5 +9,5 @@ fn SaveClipboard(cx: Scope) -> Element {
 
     cx.render(rsx! {
         div { "hello world", "hello world", "hello world" }
-})
+    })
 }

packages/autofmt/tests/samples/t2.rsx

@@ -0,0 +1,7 @@
+rsx! {
+    div {}
+    div {
+        // div {
+    }
+}
+

packages/autofmt/tests/samples/tiny.rsx

@@ -1 +1,10 @@
-rsx! { div {} }
+fn ItWorks() {
+    rsx! {
+        div {
+            div {
+                div {}
+                div {}
+            }
+        }
+    }
+}

packages/autofmt/tests/wrong/multiexpr.rsx

@@ -1,3 +1,5 @@
-cx.render(rsx! {
-    div { class: "flex flex-wrap items-center dark:text-white py-16 border-t font-light", left, right }
-})
+fn ItWroks() {
+    cx.render(rsx! {
+        div { class: "flex flex-wrap items-center dark:text-white py-16 border-t font-light", left, right }
+    })
+}

packages/autofmt/tests/wrong/multiexpr.wrong.rsx

@@ -1,3 +1,5 @@
-cx.render(rsx! {
-    div { class: "flex flex-wrap items-center dark:text-white py-16 border-t font-light", left, right }
-})
+fn ItWroks() {
+    cx.render(rsx! {
+        div { class: "flex flex-wrap items-center dark:text-white py-16 border-t font-light", left, right }
+    })
+}