fix: some edge cases around autoformatting (#485)

* wip: remove comment support from autofmt

* fix: some edge cases around autoformatting

* fix: revert readme

* fix: failing test

packages/autofmt/src/buffer.rs

@@ -1,14 +1,34 @@
-use std::fmt::{Result, Write};
+use std::{
+    collections::HashMap,
+    fmt::{Result, Write},
+};
 
-use dioxus_rsx::BodyNode;
+use dioxus_rsx::{BodyNode, ElementAttr, ElementAttrNamed};
+use proc_macro2::{LineColumn, Span};
+use syn::spanned::Spanned;
 
 #[derive(Default, Debug)]
 pub struct Buffer {
     pub src: Vec<String>,
+    pub cached_formats: HashMap<Location, String>,
     pub buf: String,
     pub indent: usize,
 }
 
+#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)]
+pub struct Location {
+    pub line: usize,
+    pub col: usize,
+}
+impl Location {
+    pub fn new(start: LineColumn) -> Self {
+        Self {
+            line: start.line,
+            col: start.column,
+        }
+    }
+}
+
 impl Buffer {
     // Create a new line and tab it to the current tab level
     pub fn tabbed_line(&mut self) -> Result {
@@ -41,6 +61,7 @@ impl Buffer {
         writeln!(self.buf)
     }
 
+    // Expects to be written directly into place
     pub fn write_ident(&mut self, node: &BodyNode) -> Result {
         match node {
             BodyNode::Element(el) => self.write_element(el),
@@ -62,41 +83,12 @@ impl Buffer {
     pub fn write_body_indented(&mut self, children: &[BodyNode]) -> Result {
         self.indent += 1;
 
-        let mut comments = Vec::new();
+        // let mut had_comments = false;
+        // let mut comments = Vec::new();
 
         for child in children {
             // Exprs handle their own indenting/line breaks
             if !matches!(child, BodyNode::RawExpr(_)) {
-                // collect all comments upwards
-                let start = child.span().start().line;
-
-                for (id, line) in self.src[..start - 1].iter().enumerate().rev() {
-                    if line.trim().starts_with("//") || line.is_empty() {
-                        comments.push(id);
-                    } else {
-                        break;
-                    }
-                }
-
-                if comments.len() == 1 && self.src[comments[0]].is_empty() {
-                    comments.pop();
-                }
-
-                let mut last_was_empty = false;
-                for comment_line in comments.drain(..).rev() {
-                    let line = &self.src[comment_line];
-                    if line.is_empty() {
-                        if !last_was_empty {
-                            self.new_line()?;
-                        }
-                        last_was_empty = true;
-                    } else {
-                        last_was_empty = false;
-                        self.tabbed_line()?;
-                        write!(self.buf, "{}", self.src[comment_line].trim())?;
-                    }
-                }
-
                 self.tabbed_line()?;
             }
 
@@ -106,4 +98,93 @@ impl Buffer {
         self.indent -= 1;
         Ok(())
     }
+
+    pub(crate) fn is_short_attrs(&mut self, attributes: &[ElementAttrNamed]) -> usize {
+        attributes
+            .iter()
+            .map(|attr| match &attr.attr {
+                ElementAttr::AttrText { value, name } => {
+                    value.value().len() + name.span().line_length() + 3
+                }
+                ElementAttr::AttrExpression { name, value } => {
+                    value.span().line_length() + name.span().line_length() + 3
+                }
+                ElementAttr::CustomAttrText { value, name } => {
+                    value.value().len() + name.value().len() + 3
+                }
+                ElementAttr::CustomAttrExpression { name, value } => {
+                    name.value().len() + value.span().line_length() + 3
+                }
+                ElementAttr::EventTokens { tokens, name } => {
+                    let location = Location::new(tokens.span().start());
+
+                    let len = if let std::collections::hash_map::Entry::Vacant(e) =
+                        self.cached_formats.entry(location)
+                    {
+                        let formatted = prettyplease::unparse_expr(tokens);
+                        let len = if formatted.contains('\n') {
+                            10000
+                        } else {
+                            formatted.len()
+                        };
+                        e.insert(formatted);
+                        len
+                    } else {
+                        self.cached_formats[&location].len()
+                    };
+
+                    len + name.span().line_length() + 3
+                }
+            })
+            .sum()
+    }
+
+    pub fn retrieve_formatted_expr(&mut self, location: LineColumn) -> Option<String> {
+        self.cached_formats.remove(&Location::new(location))
+    }
 }
+
+trait SpanLength {
+    fn line_length(&self) -> usize;
+}
+impl SpanLength for Span {
+    fn line_length(&self) -> usize {
+        self.end().line - self.start().line
+    }
+}
+
+// collect all comments upwards
+// let start = child.span().start().line;
+
+// for (id, line) in self.src[..start - 1].iter().enumerate().rev() {
+//     if line.trim().starts_with("//") {
+//         // if line.trim().starts_with("//") || line.is_empty() {
+//         if line.is_empty() && id == 0 {
+//             break;
+//         }
+//         comments.push(id);
+//     } else {
+//         break;
+//     }
+// }
+
+// if comments.len() == 1 && self.src[comments[0]].is_empty() {
+//     comments.pop();
+// }
+
+// // let mut last_line_was_empty = false;
+// had_comments = !comments.is_empty();
+// for comment_line in comments.drain(..).rev() {
+//     let line = &self.src[comment_line];
+//     if line.is_empty() {
+//         continue;
+//         // if last_line_was_empty {
+//         //     continue;
+//         // } else {
+//         //     last_line_was_empty = true;
+//         // }
+//     }
+
+//     self.tabbed_line()?;
+//     write!(self.buf, "{}", self.src[comment_line].trim())?;
+// }

packages/autofmt/src/component.rs

@@ -1,8 +1,8 @@
-use crate::Buffer;
+use crate::{buffer::Location, Buffer};
 use dioxus_rsx::*;
 use quote::ToTokens;
 use std::fmt::{Result, Write};
-use syn::AngleBracketedGenericArguments;
+use syn::{spanned::Spanned, AngleBracketedGenericArguments};
 
 enum ShortOptimization {
     // Special because we want to print the closing bracket immediately
@@ -36,7 +36,8 @@ impl Buffer {
         let mut opt_level = ShortOptimization::NoOpt;
 
         // check if we have a lot of attributes
-        let is_short_attr_list = self.is_short_fields(fields, manual_props).is_some();
+        let attr_len = self.field_len(fields, manual_props);
+        let is_short_attr_list = attr_len < 80;
         let is_small_children = self.is_short_children(children).is_some();
 
         // if we have few attributes and a lot of children, place the attrs on top
@@ -63,6 +64,11 @@ impl Buffer {
             opt_level = ShortOptimization::Empty;
         }
 
+        // multiline handlers bump everything down
+        if attr_len > 1000 {
+            opt_level = ShortOptimization::NoOpt;
+        }
+
         match opt_level {
             ShortOptimization::Empty => {}
             ShortOptimization::Oneliner => {
@@ -179,17 +185,27 @@ impl Buffer {
 
         Ok(())
     }
-    pub fn is_short_fields(
-        &self,
+
+    pub fn field_len(
+        &mut self,
         fields: &[ComponentField],
         manual_props: &Option<syn::Expr>,
-    ) -> Option<usize> {
+    ) -> usize {
         let attr_len = fields
             .iter()
             .map(|field| match &field.content {
-                ContentField::ManExpr(exp) => exp.to_token_stream().to_string().len(),
                 ContentField::Formatted(s) => s.value().len() ,
-                ContentField::OnHandlerRaw(_) => 100000,
+                ContentField::OnHandlerRaw(exp) | ContentField::ManExpr(exp) => {
+
+                    let formatted = prettyplease::unparse_expr(exp);
+                    let len = if formatted.contains('\n') {
+                        10000
+                    } else {
+                        formatted.len()
+                    };
+                    self.cached_formats.insert(Location::new(exp.span().start()) , formatted);
+                    len
+                },
             } + 10)
             .sum::<usize>() + self.indent * 4;
 
@@ -197,18 +213,18 @@ impl Buffer {
             Some(p) => {
                 let content = prettyplease::unparse_expr(p);
                 if content.len() + attr_len > 80 {
-                    return None;
+                    return 100000;
                 }
                 let mut lines = content.lines();
                 lines.next().unwrap();
 
                 if lines.next().is_none() {
-                    Some(attr_len + content.len())
+                    attr_len + content.len()
                 } else {
-                    None
+                    100000
                 }
             }
-            None => Some(attr_len),
+            None => attr_len,
         }
     }
 

packages/autofmt/src/element.rs

@@ -1,7 +1,9 @@
-use crate::{util::*, Buffer};
+use crate::Buffer;
 use dioxus_rsx::*;
 use std::{fmt::Result, fmt::Write};
+use syn::spanned::Spanned;
 
+#[derive(Debug)]
 enum ShortOptimization {
     // Special because we want to print the closing bracket immediately
     Empty,
@@ -41,7 +43,8 @@ impl Buffer {
         let mut opt_level = ShortOptimization::NoOpt;
 
         // check if we have a lot of attributes
-        let is_short_attr_list = is_short_attrs(attributes);
+        let attr_len = self.is_short_attrs(attributes);
+        let is_short_attr_list = attr_len < 80;
         let is_small_children = self.is_short_children(children).is_some();
 
         // if we have few attributes and a lot of children, place the attrs on top
@@ -68,6 +71,11 @@ impl Buffer {
             opt_level = ShortOptimization::Empty;
         }
 
+        // multiline handlers bump everything down
+        if attr_len > 1000 {
+            opt_level = ShortOptimization::NoOpt;
+        }
+
         match opt_level {
             ShortOptimization::Empty => {}
             ShortOptimization::Oneliner => {
@@ -94,13 +102,22 @@ impl Buffer {
                     write!(self.buf, ",")?;
                 }
 
-                self.write_body_indented(children)?;
+                if !children.is_empty() {
+                    self.write_body_indented(children)?;
+                }
                 self.tabbed_line()?;
             }
 
             ShortOptimization::NoOpt => {
                 self.write_attributes(attributes, key, false)?;
-                self.write_body_indented(children)?;
+
+                if !children.is_empty() && !attributes.is_empty() {
+                    write!(self.buf, ",")?;
+                }
+
+                if !children.is_empty() {
+                    self.write_body_indented(children)?;
+                }
                 self.tabbed_line()?;
             }
         }
@@ -171,7 +188,7 @@ impl Buffer {
             }
 
             ElementAttr::EventTokens { name, tokens } => {
-                let out = prettyplease::unparse_expr(tokens);
+                let out = self.retrieve_formatted_expr(tokens.span().start()).unwrap();
 
                 let mut lines = out.split('\n').peekable();
                 let first = lines.next().unwrap();
@@ -204,35 +221,42 @@ impl Buffer {
     // returns the total line length if it's short
     // returns none if the length exceeds the limit
     // I think this eventually becomes quadratic :(
-    pub fn is_short_children(&self, children: &[BodyNode]) -> Option<usize> {
+    pub fn is_short_children(&mut self, children: &[BodyNode]) -> Option<usize> {
         if children.is_empty() {
             // todo: allow elements with comments but no children
             // like div { /* comment */ }
             return Some(0);
         }
 
-        for child in children {
-            'line: for line in self.src[..child.span().start().line - 1].iter().rev() {
-                match (line.trim().starts_with("//"), line.is_empty()) {
-                    (true, _) => return None,
-                    (_, true) => continue 'line,
-                    _ => break 'line,
-                }
-            }
-        }
+        // for child in children {
+        //     'line: for line in self.src[..child.span().start().line - 1].iter().rev() {
+        //         match (line.trim().starts_with("//"), line.is_empty()) {
+        //             (true, _) => return None,
+        //             (_, true) => continue 'line,
+        //             _ => break 'line,
+        //         }
+        //     }
+        // }
 
         match children {
             [BodyNode::Text(ref text)] => Some(text.value().len()),
             [BodyNode::Component(ref comp)] => {
-                let is_short_child = self.is_short_children(&comp.children);
-                let is_short_attrs = self.is_short_fields(&comp.fields, &comp.manual_props);
+                let attr_len = self.field_len(&comp.fields, &comp.manual_props);
 
-                match (is_short_child, is_short_attrs) {
-                    (Some(child_len), Some(attrs_len)) => Some(child_len + attrs_len),
-                    (Some(child_len), None) => Some(child_len),
-                    (None, Some(attrs_len)) => Some(attrs_len),
-                    (None, None) => None,
+                if attr_len > 80 {
+                    None
+                } else {
+                    self.is_short_children(&comp.children)
+                        .map(|child_len| child_len + attr_len)
                 }
+                // let is_short_child = self.is_short_children(&comp.children);
+
+                // match (is_short_child, is_short_attrs) {
+                //     (Some(child_len), Some(attrs_len)) => Some(child_len + attrs_len),
+                //     (Some(child_len), None) => Some(child_len),
+                //     (None, Some(attrs_len)) => Some(attrs_len),
+                //     (None, None) => None,
+                // }
             }
             [BodyNode::RawExpr(ref _expr)] => {
                 // TODO: let rawexprs to be inlined
@@ -245,15 +269,17 @@ impl Buffer {
                 // }
                 None
             }
-            [BodyNode::Element(ref el)] => self
-                .is_short_children(&el.children)
-                .map(|f| f + extract_attr_len(&el.attributes))
-                .and_then(|new_len| if new_len > 80 { None } else { Some(new_len) }),
+            [BodyNode::Element(ref el)] => {
+                let attr_len = self.is_short_attrs(&el.attributes);
+
+                if attr_len > 80 {
+                    None
+                } else {
+                    self.is_short_children(&el.children)
+                        .map(|child_len| child_len + attr_len)
+                }
+            }
             _ => None,
         }
     }
 }
-
-fn is_short_attrs(attrs: &[ElementAttrNamed]) -> bool {
-    extract_attr_len(attrs) < 80
-}

packages/autofmt/src/lib.rs

@@ -37,37 +37,62 @@ pub fn fmt_file(contents: &str) -> Vec<FormattedBlock> {
 
     use triple_accel::{levenshtein_search, Match};
 
-    for Match { end, start, .. } in levenshtein_search(b"rsx! {", contents.as_bytes()) {
+    for Match { end, start, k } in levenshtein_search(b"rsx! {", contents.as_bytes()) {
+        if k > 1 {
+            continue;
+        }
+
         // ensure the marker is not nested
         if start < last_bracket_end {
             continue;
         }
 
+        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,
+            }
+        };
+
         let remaining = &contents[end - 1..];
         let bracket_end = find_bracket_end(remaining).unwrap();
         let sub_string = &contents[end..bracket_end + end - 1];
         last_bracket_end = bracket_end + end - 1;
 
-        let new = fmt_block(sub_string).unwrap();
-        let stripped = &contents[end + 1..bracket_end + end - 1];
+        let mut new = fmt_block(sub_string, indent_level).unwrap();
 
-        if stripped == new {
+        if new.len() <= 80 && !new.contains('\n') {
+            new = format!(" {new} ");
+        }
+
+        let end_marker = end + bracket_end - indent_level * 4 - 1;
+
+        if new == contents[end..end_marker] {
             continue;
         }
 
         formatted_blocks.push(FormattedBlock {
             formatted: new,
             start: end,
-            end: end + bracket_end - 1,
+            end: end_marker,
         });
     }
 
     formatted_blocks
 }
 
-pub fn fmt_block(block: &str) -> Option<String> {
+pub fn fmt_block(block: &str, indent_level: usize) -> Option<String> {
     let mut buf = Buffer {
         src: block.lines().map(|f| f.to_string()).collect(),
+        indent: indent_level,
         ..Buffer::default()
     };
 
@@ -75,5 +100,46 @@ pub fn fmt_block(block: &str) -> Option<String> {
 
     buf.write_body_indented(&body.roots).unwrap();
 
+    // writing idents leaves the final line ended at the end of the last ident
+    if buf.buf.contains('\n') {
+        buf.new_line().unwrap();
+    }
+
     buf.consume()
 }
+
+pub fn apply_format(input: &str, block: FormattedBlock) -> String {
+    let start = block.start;
+    let end = block.end;
+
+    let (left, _) = input.split_at(start);
+    let (_, right) = input.split_at(end);
+
+    dbg!(&block.formatted);
+
+    format!("{}{}{}", left, block.formatted, right)
+}
+
+// Apply all the blocks
+pub fn apply_formats(input: &str, blocks: Vec<FormattedBlock>) -> String {
+    let mut out = String::new();
+
+    let mut last = 0;
+
+    for FormattedBlock {
+        formatted,
+        start,
+        end,
+    } in blocks
+    {
+        let prefix = &input[last..start];
+        out.push_str(prefix);
+        out.push_str(&formatted);
+        last = end;
+    }
+
+    let suffix = &input[last..];
+    out.push_str(suffix);
+
+    out
+}

packages/autofmt/src/util.rs

@@ -1,19 +1,3 @@
-use dioxus_rsx::*;
-
-// todo: use recursive or complete sizeing
-pub fn extract_attr_len(attributes: &[ElementAttrNamed]) -> usize {
-    attributes
-        .iter()
-        .map(|attr| match &attr.attr {
-            ElementAttr::AttrText { value, .. } => value.value().len(),
-            ElementAttr::AttrExpression { .. } => 10,
-            ElementAttr::CustomAttrText { value, .. } => value.value().len(),
-            ElementAttr::CustomAttrExpression { .. } => 10,
-            ElementAttr::EventTokens { .. } => 1000000,
-        })
-        .sum()
-}
-
 pub fn find_bracket_end(contents: &str) -> Option<usize> {
     let mut depth = 0;