From c73dc432339c5cfca185f357b04591f8eafdacfc Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Tue, 7 Apr 2026 16:19:00 -0700 Subject: [PATCH] fix(autofmt): add support for handling comments in various expression types (#5461) * fix(autofmt): add support for handling comments in various expression types * fmt --- packages/autofmt/src/writer.rs | 43 +++++++++++++++++-- packages/autofmt/tests/samples.rs | 38 ++++++++++++++++ .../tests/samples/comments_async_closure.rsx | 10 +++++ .../samples/comments_attr_expr_blocks.rsx | 32 ++++++++++++++ .../samples/comments_expr_with_strings.rsx | 20 +++++++++ .../samples/comments_nested_closures.rsx | 25 +++++++++++ 6 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 packages/autofmt/tests/samples/comments_async_closure.rsx create mode 100644 packages/autofmt/tests/samples/comments_attr_expr_blocks.rsx create mode 100644 packages/autofmt/tests/samples/comments_expr_with_strings.rsx create mode 100644 packages/autofmt/tests/samples/comments_nested_closures.rsx diff --git a/packages/autofmt/src/writer.rs b/packages/autofmt/src/writer.rs index cac750b4b..d9dfbd3ec 100644 --- a/packages/autofmt/src/writer.rs +++ b/packages/autofmt/src/writer.rs @@ -712,6 +712,14 @@ impl<'a> Writer<'a> { Ok(()) } + fn span_has_line_comments(&self, span: Span) -> bool { + span.source_text().is_some_and(|source| { + source + .lines() + .any(|line| line.trim_start().starts_with("//")) + }) + } + fn attr_value_len(&mut self, value: &AttributeValue) -> usize { match value { AttributeValue::IfExpr(if_chain) => { @@ -734,11 +742,23 @@ impl<'a> Writer<'a> { } AttributeValue::AttrExpr(expr) => expr .as_expr() - .map(|expr| self.attr_expr_len(&expr)) + .map(|expr| { + if self.span_has_line_comments(expr.span()) { + 100000 + } else { + self.attr_expr_len(&expr) + } + }) .unwrap_or(100000), AttributeValue::EventTokens(closure) => closure .as_expr() - .map(|expr| self.attr_expr_len(&expr)) + .map(|expr| { + if self.span_has_line_comments(expr.span()) { + 100000 + } else { + self.attr_expr_len(&expr) + } + }) .unwrap_or(100000), } } @@ -846,6 +866,9 @@ impl<'a> Writer<'a> { let pretty = self.retrieve_formatted_expr(&expr).to_string(); let source = src_span.source_text().unwrap_or_default(); + let source_has_line_comments = source + .lines() + .any(|line| line.trim_start().starts_with("//")); let mut src_lines = source.lines().peekable(); // Comments already in pretty output (from nested rsx!) - skip these from source @@ -940,7 +963,9 @@ impl<'a> Writer<'a> { let is_call = src_trimmed.ends_with('(') || src_trimmed.ends_with(',') || src_trimmed.ends_with('{'); - if !is_call { + let is_commented_block = + source_has_line_comments && src_trimmed.ends_with('{'); + if is_commented_block || !is_call { multiline = Some(vec![*src]); break; } @@ -1008,7 +1033,17 @@ impl<'a> Writer<'a> { } // Write multi-line with adjusted indentation - let base_indent = ml[0].chars().take_while(|c| c.is_whitespace()).count(); + let base_indent = if source_has_line_comments && ml[0].trim_end().ends_with('{') + { + ml.iter() + .skip(1) + .filter(|line| !line.trim().is_empty()) + .map(|line| line.chars().take_while(|c| c.is_whitespace()).count()) + .min() + .unwrap_or(0) + } else { + ml[0].chars().take_while(|c| c.is_whitespace()).count() + }; let target: String = line.chars().take_while(|c| c.is_whitespace()).collect(); for (i, src_line) in ml.iter().enumerate() { diff --git a/packages/autofmt/tests/samples.rs b/packages/autofmt/tests/samples.rs index 4bb494929..7a8257f73 100644 --- a/packages/autofmt/tests/samples.rs +++ b/packages/autofmt/tests/samples.rs @@ -28,7 +28,11 @@ twoway![ attributes, basic_expr, collapse_expr, + comments_attr_expr_blocks, comments, + comments_async_closure, + comments_expr_with_strings, + comments_nested_closures, commentshard, complex, docsite, @@ -66,3 +70,37 @@ twoway![ blank_lines_preserved, forloop_tuple, ]; + +fn assert_idempotent(src: &str) { + let src = src.replace("\r", ""); + + let once = + dioxus_autofmt::apply_formats(&src, dioxus_autofmt::fmt_file(&src, Default::default())) + .replace("\r", ""); + let twice = + dioxus_autofmt::apply_formats(&once, dioxus_autofmt::fmt_file(&once, Default::default())) + .replace("\r", ""); + + pretty_assertions::assert_eq!(src, once); + pretty_assertions::assert_eq!(once, twice); +} + +#[test] +fn comments_async_closure_is_idempotent() { + assert_idempotent(include_str!("./samples/comments_async_closure.rsx")); +} + +#[test] +fn comments_nested_closures_is_idempotent() { + assert_idempotent(include_str!("./samples/comments_nested_closures.rsx")); +} + +#[test] +fn comments_attr_expr_blocks_is_idempotent() { + assert_idempotent(include_str!("./samples/comments_attr_expr_blocks.rsx")); +} + +#[test] +fn comments_expr_with_strings_is_idempotent() { + assert_idempotent(include_str!("./samples/comments_expr_with_strings.rsx")); +} diff --git a/packages/autofmt/tests/samples/comments_async_closure.rsx b/packages/autofmt/tests/samples/comments_async_closure.rsx new file mode 100644 index 000000000..75a819833 --- /dev/null +++ b/packages/autofmt/tests/samples/comments_async_closure.rsx @@ -0,0 +1,10 @@ +rsx! { + button { + class: "control-button", + onclick: move |_| async move { + // comment + () + }, + "print content" + } +} diff --git a/packages/autofmt/tests/samples/comments_attr_expr_blocks.rsx b/packages/autofmt/tests/samples/comments_attr_expr_blocks.rsx new file mode 100644 index 000000000..0d3d41057 --- /dev/null +++ b/packages/autofmt/tests/samples/comments_attr_expr_blocks.rsx @@ -0,0 +1,32 @@ +rsx! { + Component { + label: { + let url = "https://example.com"; + + // comment before formatted tail expr + format!("{url}/docs") + }, + } + + Component { + status: { + // comment before branch + if is_ready() { + "ready" + } else { + "waiting" + } + }, + } + + div { + data_mode: { + // comment before block expr + if is_ready() { + "ready" + } else { + "waiting" + } + }, + } +} diff --git a/packages/autofmt/tests/samples/comments_expr_with_strings.rsx b/packages/autofmt/tests/samples/comments_expr_with_strings.rsx new file mode 100644 index 000000000..fecf54d16 --- /dev/null +++ b/packages/autofmt/tests/samples/comments_expr_with_strings.rsx @@ -0,0 +1,20 @@ +rsx! { + button { + onclick: move |_| async move { + let url = "https://example.com"; + + // keep this comment with the block, not the string literal + println!("{url}"); + }, + "strings" + } + + Component { + message: { + let prefix = "value://"; + + // comment before final expression that contains // + format!("{prefix}done") + }, + } +} diff --git a/packages/autofmt/tests/samples/comments_nested_closures.rsx b/packages/autofmt/tests/samples/comments_nested_closures.rsx new file mode 100644 index 000000000..1e0822f28 --- /dev/null +++ b/packages/autofmt/tests/samples/comments_nested_closures.rsx @@ -0,0 +1,25 @@ +rsx! { + button { + onclick: move |_| async move { + let nested = move || { + // nested closure comment + () + }; + + nested(); + }, + "nested closure" + } + + button { + onclick: move |_| async move { + let nested = move || async move { + // nested async closure comment + () + }; + + nested().await; + }, + "nested async closure" + } +}