fix(autofmt): formatting idempotency for long if/else attrs, empty bodies, and split_line_attributes

Three fixes for non-idempotent formatting where running the formatter
twice produces different output:

1. `write_attribute_if_chain` unconditionally inlined if/else attribute
   values on one line. For long values this exceeds 80 chars, causing
   ShortOptimization to oscillate. Now checks inline length against a
   budget and uses a deterministic multiline layout when exceeded.

2. `write_todo_body` emitted a newline even with no comments between
   braces, causing blank lines in empty component bodies. Now only
   emits when actual `//` comments are found.

3. The `split_line_attributes` override forced `NoOpt` even for empty
   blocks, causing `Foo {}` to expand with blank lines. Now skips the
   override when the block is `Empty`.

Fixes #5508

Author: micahkepe

packages/autofmt/src/lib.rs

@@ -113,7 +113,6 @@ pub fn try_fmt_file(
 
         let span = item.delimiter.span().join();
         let mut formatted = writer.out.buf.split_off(0);
-
         let start = collect_macros::byte_offset(contents, span.start()) + 1;
         let end = collect_macros::byte_offset(contents, span.end()) - 1;
 

packages/autofmt/src/writer.rs

@@ -59,7 +59,10 @@ impl<'a> Writer<'a> {
         if let Some(span) = body.span {
             self.out.indent_level += 1;
             let comments = self.accumulate_full_line_comments(span.span().end());
-            if !comments.is_empty() {
+            let has_real_comment = comments
+                .iter()
+                .any(|&id| self.src.get(id).is_some_and(|l| l.trim().starts_with("//")));
+            if has_real_comment {
                 self.out.new_line()?;
                 self.apply_line_comments(comments)?;
                 self.out.buf.pop(); // remove the trailing newline, forcing us to end at the end of the comment
@@ -252,11 +255,19 @@ impl<'a> Writer<'a> {
 
     pub fn write_body_nodes(&mut self, children: &[BodyNode]) -> Result {
         let mut iter = children.iter().peekable();
+        let mut is_first = true;
 
         while let Some(child) = iter.next() {
             if self.current_span_is_primary(child.span().start()) {
-                self.write_comments(child.span().start())?;
+                let comments = self.accumulate_full_line_comments(child.span().start());
+                let has_real_comment = comments
+                    .iter()
+                    .any(|&id| self.src.get(id).is_some_and(|l| l.trim().starts_with("//")));
+                if has_real_comment || !is_first {
+                    self.apply_line_comments(comments)?;
+                }
             };
+            is_first = false;
             self.out.tab()?;
             self.write_ident(child)?;
             if iter.peek().is_some() {
@@ -360,8 +371,10 @@ impl<'a> Writer<'a> {
             self.write_todo_body(brace)?;
         }
 
-        // multiline handlers bump everything down
-        if attr_len > 1000 || self.out.indent.split_line_attributes() {
+        // multiline handlers bump everything down, but not empty blocks
+        if !matches!(opt_level, ShortOptimization::Empty)
+            && (attr_len > 1000 || self.out.indent.split_line_attributes())
+        {
             opt_level = ShortOptimization::NoOpt;
         }
 
@@ -410,7 +423,9 @@ impl<'a> Writer<'a> {
                 self.write_attributes(attributes, spreads, false, brace, has_children)?;
 
                 if !children.is_empty() {
-                    self.out.new_line()?;
+                    if !attributes.is_empty() || !spreads.is_empty() {
+                        self.out.new_line()?;
+                    }
                     self.write_body_indented(children)?;
                 }
 
@@ -425,7 +440,10 @@ impl<'a> Writer<'a> {
         ) && self.leading_row_is_empty(brace.span.span().end())
         {
             let comments = self.accumulate_full_line_comments(brace.span.span().end());
-            if !comments.is_empty() {
+            let has_real_comment = comments
+                .iter()
+                .any(|&id| self.src.get(id).is_some_and(|l| l.trim().starts_with("//")));
+            if has_real_comment {
                 // Undo the tab from tabbed_line(). It positioned for the closing
                 // brace, but trailing comments need child-level indentation
                 let tab_width = self.out.indent.indent_str().len() * self.out.indent_level;
@@ -526,8 +544,20 @@ impl<'a> Writer<'a> {
     fn write_attribute(&mut self, attr: &Attribute) -> Result {
         self.write_attribute_name(&attr.name)?;
 
-        // if the attribute is a shorthand, we don't need to write the colon, just the name
         if !attr.can_be_shorthand() {
+            if let AttributeValue::IfExpr(if_chain) = &attr.value {
+                let inline_len = self.attr_value_len(&attr.value);
+                let line_budget = 80usize.saturating_sub(self.out.indent_level * 4);
+                if inline_len > line_budget {
+                    write!(self.out, ":")?;
+                    self.out.indent_level += 1;
+                    self.out.new_line()?;
+                    self.out.tab()?;
+                    self.write_attribute_if_chain_multiline(if_chain)?;
+                    self.out.indent_level -= 1;
+                    return Ok(());
+                }
+            }
             write!(self.out, ": ")?;
             self.write_attribute_value(&attr.value)?;
         }
@@ -570,14 +600,25 @@ impl<'a> Writer<'a> {
     }
 
     fn write_attribute_if_chain(&mut self, if_chain: &IfAttributeValue) -> Result {
+        let inline_len = self.attr_value_len(&AttributeValue::IfExpr(if_chain.clone()));
+        let line_budget = 80usize.saturating_sub(self.out.indent_level * 4);
+
+        if inline_len <= line_budget {
+            self.write_attribute_if_chain_inline(if_chain)
+        } else {
+            self.write_attribute_if_chain_multiline(if_chain)
+        }
+    }
+
+    fn write_attribute_if_chain_inline(&mut self, if_chain: &IfAttributeValue) -> Result {
         let cond = self.unparse_expr(&if_chain.if_expr.cond);
         write!(self.out, "if {cond} {{ ")?;
         self.write_attribute_value(&if_chain.then_value)?;
         write!(self.out, " }}")?;
         match if_chain.else_value.as_deref() {
             Some(AttributeValue::IfExpr(else_if_chain)) => {
                 write!(self.out, " else ")?;
-                self.write_attribute_if_chain(else_if_chain)?;
+                self.write_attribute_if_chain_inline(else_if_chain)?;
             }
             Some(other) => {
                 write!(self.out, " else {{ ")?;
@@ -586,7 +627,40 @@ impl<'a> Writer<'a> {
             }
             None => {}
         }
+        Ok(())
+    }
 
+    fn write_attribute_if_chain_multiline(&mut self, if_chain: &IfAttributeValue) -> Result {
+        let base = self.out.indent_level;
+        let cond = self.unparse_expr(&if_chain.if_expr.cond);
+        write!(self.out, "if {cond} {{")?;
+        self.out.indent_level = base + 1;
+        self.out.new_line()?;
+        self.out.tab()?;
+        self.write_attribute_value(&if_chain.then_value)?;
+        self.out.indent_level = base;
+        self.out.new_line()?;
+        self.out.tab()?;
+        write!(self.out, "}}")?;
+        match if_chain.else_value.as_deref() {
+            Some(AttributeValue::IfExpr(else_if_chain)) => {
+                write!(self.out, " else ")?;
+                self.write_attribute_if_chain_multiline(else_if_chain)?;
+            }
+            Some(other) => {
+                write!(self.out, " else {{")?;
+                self.out.indent_level = base + 1;
+                self.out.new_line()?;
+                self.out.tab()?;
+                self.write_attribute_value(other)?;
+                self.out.indent_level = base;
+                self.out.new_line()?;
+                self.out.tab()?;
+                write!(self.out, "}}")?;
+            }
+            None => {}
+        }
+        self.out.indent_level = base;
         Ok(())
     }
 
@@ -838,18 +912,24 @@ impl<'a> Writer<'a> {
             return Ok(());
         }
 
+        let comments: Vec<&str> = (start.line..end.line)
+            .filter_map(|idx| {
+                let line = self.src.get(idx)?;
+                line.trim().starts_with("//").then_some(line.trim())
+            })
+            .collect();
+
+        if comments.is_empty() {
+            return Ok(());
+        }
+
         writeln!(self.out)?;
 
-        for idx in start.line..end.line {
-            let Some(line) = self.src.get(idx) else {
-                continue;
-            };
-            if line.trim().starts_with("//") {
-                for _ in 0..self.out.indent_level + 1 {
-                    write!(self.out, "    ")?
-                }
-                writeln!(self.out, "{}", line.trim())?;
+        for comment in &comments {
+            for _ in 0..self.out.indent_level + 1 {
+                write!(self.out, "    ")?
             }
+            writeln!(self.out, "{comment}")?;
         }
 
         for _ in 0..self.out.indent_level {

packages/autofmt/tests/samples.rs

@@ -74,6 +74,9 @@ twoway![
     commented_rsx_block_only,
     commented_rsx_block_between,
     commented_rsx_block_deep,
+    long_if_else_attr,
+    empty_component_body,
+    empty_braces_oneliner,
 ];
 
 fn assert_idempotent(src: &str) {
@@ -109,3 +112,46 @@ fn comments_attr_expr_blocks_is_idempotent() {
 fn comments_expr_with_strings_is_idempotent() {
     assert_idempotent(include_str!("./samples/comments_expr_with_strings.rsx"));
 }
+
+#[test]
+fn long_if_else_attr_is_idempotent() {
+    assert_idempotent(include_str!("./samples/long_if_else_attr.rsx"));
+}
+
+#[test]
+fn empty_component_body_is_idempotent() {
+    assert_idempotent(include_str!("./samples/empty_component_body.rsx"));
+}
+
+#[test]
+fn empty_braces_match_arm_is_idempotent() {
+    let src = include_str!("./samples/empty_braces_match_arm.rsx");
+    let once = dioxus_autofmt::apply_formats(src, dioxus_autofmt::fmt_file(src, Default::default()));
+    let twice = dioxus_autofmt::apply_formats(&once, dioxus_autofmt::fmt_file(&once, Default::default()));
+    let thrice = dioxus_autofmt::apply_formats(&twice, dioxus_autofmt::fmt_file(&twice, Default::default()));
+    pretty_assertions::assert_eq!(&once, &twice, "pass 1 vs pass 2");
+    pretty_assertions::assert_eq!(&twice, &thrice, "pass 2 vs pass 3");
+}
+
+#[test]
+fn empty_braces_no_space_is_idempotent() {
+    let src = "rsx! { Router::<Route>{}}";
+    let once = dioxus_autofmt::apply_formats(src, dioxus_autofmt::fmt_file(src, Default::default()));
+    let twice = dioxus_autofmt::apply_formats(&once, dioxus_autofmt::fmt_file(&once, Default::default()));
+    let thrice = dioxus_autofmt::apply_formats(&twice, dioxus_autofmt::fmt_file(&twice, Default::default()));
+    eprintln!("=== ONCE ===\n{once}");
+    eprintln!("=== TWICE ===\n{twice}");
+    eprintln!("=== THRICE ===\n{thrice}");
+    pretty_assertions::assert_eq!(&once, &twice, "pass 1 vs pass 2");
+    pretty_assertions::assert_eq!(&twice, &thrice, "pass 2 vs pass 3");
+}
+
+#[test]
+fn empty_braces_oneliner_is_idempotent() {
+    let src = r#"rsx! { Router::<Route>{}}"#;
+    let once = dioxus_autofmt::apply_formats(src, dioxus_autofmt::fmt_file(src, Default::default()));
+    let twice = dioxus_autofmt::apply_formats(&once, dioxus_autofmt::fmt_file(&once, Default::default()));
+    let thrice = dioxus_autofmt::apply_formats(&twice, dioxus_autofmt::fmt_file(&twice, Default::default()));
+    pretty_assertions::assert_eq!(&once, &twice, "pass 1 vs pass 2");
+    pretty_assertions::assert_eq!(&twice, &thrice, "pass 2 vs pass 3");
+}

packages/autofmt/tests/samples/empty_braces_match_arm.rsx

@@ -0,0 +1,12 @@
+rsx! {
+    {
+        match state() {
+            State::Loading => rsx! {
+                LoadingScreen {}
+            },
+            State::Ready => rsx! {
+                ReadyScreen { name }
+            },
+        }
+    }
+}

packages/autofmt/tests/samples/empty_braces_oneliner.rsx

@@ -0,0 +1,5 @@
+rsx! {
+    Router::<Route> {}
+    LoadingScreen {}
+    AuthenticatingScreen {}
+}

packages/autofmt/tests/samples/empty_component_body.rsx

@@ -0,0 +1,8 @@
+rsx! {
+    Pagination {
+        PaginationContent {
+            PaginationPrevious { onclick: move |_| on_prev(()) }
+            PaginationNext { onclick: move |_| on_next(()) }
+        }
+    }
+}

packages/autofmt/tests/samples/long_if_else_attr.rsx

@@ -0,0 +1,10 @@
+rsx! {
+    button { class:
+        if is_active {
+            "w-full text-left px-3 py-1 text-xs font-mono text-primary bg-select"
+        } else {
+            "w-full text-left px-3 py-1 text-xs font-mono text-secondary hover:bg-select hover:text-primary"
+        },
+        "Click me"
+    }
+}