Skip to content

fix(autofmt): stop dropping comments on attributes, spreads, control flow and empty macros - #5883

Open
nicoburns wants to merge 6 commits into
mainfrom
devin/1791038588-autofmt-comment-tests
Open

nicoburns wants to merge 6 commits into
mainfrom
devin/1791038588-autofmt-comment-tests

Conversation

@nicoburns

@nicoburns nicoburns commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Summary

dx fmt deleted // comments in a number of positions, and in a few of them wrote output that no longer parses. This fixes those and adds a comment test matrix for the block syntax. It is the bottom of a stack: #5730 and #5731 build on the refactor in the first commit.

Output that no longer parsed (the comment swallows the closing brace):

// before                            // on main
div {                                div { class: "a", // note "child" }
    class: "a", // note
    "child"
}

Comments that were deleted

  • after an attribute of an element short enough to collapse onto one line, and after the last attribute of an element with no children
  • above or after a ..spread
  • above a node or attribute when the line in between has trailing whitespace
  • in for / if / else bodies: after the opening brace, before the closing brace, in a body containing nothing else, and after the closing brace of an else if chain
  • between the closing brace of a branch and its else
  • the only thing in an rsx! call, or after its opening delimiter (also for an rsx! nested in an expression)
  • after anything on a line containing multi-byte characters (columns were used as byte offsets)
  • in the middle of a construct: between the name of an element or component and its {, between the name of an attribute and its value, inside the branches of an if attribute value, and inside the header of a for or an if

Smaller changes in behaviour

  • A comment-only body keeps one blank line between its comments, and indents them with the configured indentation rather than four spaces.
  • An if with empty bodies is written if a {} else {} rather than with a blank line in each body.
  • div {..attrs, (spread only, children below) gets its missing space.
  • A long if attribute value in an element with one attribute per line starts on the line of its name (class: if a {). It was written below the name at the same indentation.

Unbalanced braces from an rsx! nested in a closure

The pass that carries comments and blank lines from the source of an expression over to its formatted lines also ran over the lines of a nested rsx!, which are already formatted and no longer line up with the source. Where children had been joined onto one line, source lines could be copied over them:

// before                              // on main (inner macro)
{                                      tr {
    items.iter().map(|x| {                 td {
                                           a { href: "x",
        let y = f(x);                      td { "two" }
                                           td {
        // a comment                           a { href: "x", "four" }
        rsx! {                             }
            tr {                       }
                td {
                    "one"
                }
                td {
                    "two"
                }
                td {
                    a { href: "x",
                        "four"
                    }
                }
            }
        }
    })
}

unparse_expr_with_nested_rsx now also returns the lines of each nested rsx! in the source and in the formatted expression. write_partial_expr writes the lines between its delimiters as they are, and skips the matching source lines (apart from comments before the closing delimiter, which still come from the source). Test: messy input nested-rsx-closure.

How

  • First commit, no behaviour change: the comment helpers took a &Brace but only used the positions of its two tokens. They now take a BodyDelimiters { open, close }, so they work for any delimited body.

  • for / if bodies go through the same open-comment, comment-only-body and closing-comment helpers as elements (write_block_body, write_comment_only_body, write_closing_line).

  • Attributes with comments are written one per line. The exception is a comment after the last attribute when children follow, which keeps the existing div { class: "a", // note layout.

  • A comment can't share a line with else, so comments before one are left in place and the else goes on its own line (write_else):

    if a {
        "a"
    } // after the brace
    // above the else
    else {
        "b"
    }
  • A nested rsx! with a comment after its opening brace is always written across several lines, so the existing expression comment pass finds the line to attach it to.

  • text_before / text_after convert a column to a byte offset before slicing a source line.

  • has_leading_comments replaces three copies of the "are there comments above this" loop, which stopped at whitespace-only lines.

  • Comments in the middle of a construct (last commit). These constructs are rebuilt from their tokens, so each needs a place to put a comment:

    div { // was between `div` and `{`
        class:
            // was between `class:` and its value; the value is still formatted
            "a",
        id: if a {
            // inside a branch; the chain is still formatted
            "x"
        } else {
            "y" // after the value of a branch
        },
    }
    for item in items
        // inside a header: the header is written as it is in the source, re-indented
        .iter()
    {
        div {}
    }

    A header or condition is only taken from the source if line_comments, a small scanner that skips strings, characters and block comments, finds a comment in it.

Tests

  • Idempotency samples: comments_attributes, comments_control_flow, comments_positions.
  • Idempotency sample comments_constructs, and messy input comments-constructs-4sp / comments-constructs-tab, for comments in the middle of a construct.
  • Messy input, with spaces and with tabs: comments-messy-4sp, comments-messy-tab.

All of these fail on main.

Not fixed here

  • /* */ comments are still deleted everywhere (dx fmt eats block comments #2751).
  • Comments inside the arguments of a call in an attribute value (format!(a, // note …) are deleted when the expression is reflowed.
  • A comment or blank line in a closure whose first line is reflowed (items.iter().map(|x| { split over three lines) can still be dropped, as in the example above: the output is now valid, but without // a comment.
  • With tabs or split_line_attributes, expressions that contain comments and a nested rsx! are not idempotent, and with split_line_attributes can lose comments.

Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/21d36a6323ce400dba42a729314e41b0
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/21d36a6323ce400dba42a729314e41b0?variant=devin-insiders
Requested by: @nicoburns

@staging-devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

@staging-devin-ai-integration
staging-devin-ai-integration Bot added this pull request to stack #5884 October 3, 2026 14:58
@staging-devin-ai-integration
staging-devin-ai-integration Bot force-pushed the devin/1791038588-autofmt-comment-tests branch from 820709a to fc50406 Compare October 3, 2026 15:28
…brace

The comment helpers took a `&Brace`, but only ever used the positions of
its two tokens. Pass those positions instead so the same helpers can be
used for any delimited body. No behaviour change.
…empty macros

dx fmt deleted comments, and in a few cases wrote code that no longer
parsed, when they were:

- after an attribute of an element short enough to be put on one line
  (the comment swallowed the rest of the line, including the closing brace)
- after the last attribute of an element with no children
- above or after a spread
- after the opening brace or before the closing brace of a for/if body,
  in a for/if body with nothing else in it, or after the closing brace of
  an else-if chain
- the only thing in an rsx! call, or after its opening brace
- after anything on a line containing multi-byte characters

Also keep one blank line between the comments of a comment-only body,
indent those comments with the configured indentation rather than four
spaces, and write empty if bodies as `{}`.
… whitespace

A line that only had whitespace on it ended the search for the comments
above a node or attribute, so the element was collapsed onto one line and
the comments were dropped. Use one helper for the three copies of that
search.
…e of a nested rsx!

A comment between the closing brace of a branch and its `else` was
deleted. It is now left where it is, with the `else` on its own line.

A comment after the opening brace of an `rsx!` inside an expression was
deleted when the body was short enough to be written on that same line.
…d for/if headers

Comments in the middle of a construct were dropped, as the construct is rebuilt from its
tokens. This keeps them in four places:

- between the name of an element or component and its opening brace, by moving them after
  the brace
- between the name of an attribute and its value, by writing the value on a line of its
  own below them
- inside the branches of an `if` attribute value, which is still formatted
- inside the header of a `for` or an `if`, by writing the header as it is in the source

A long `if` attribute value on a line of its own now also starts on the line of its name,
rather than below it at the same indentation.
The pass that carries comments and blank lines from the source of an expression
over to its formatted lines also ran over the lines of a nested rsx!, which are
already formatted. Where those lines had been joined, source lines could be
copied over them, giving unbalanced braces.
@staging-devin-ai-integration
staging-devin-ai-integration Bot force-pushed the devin/1791038588-autofmt-comment-tests branch from fc50406 to 760ef21 Compare October 12, 2026 17:55
Comment on lines 1504 to 1514
for (i, src_line) in ml.iter().enumerate() {
let indent = src_line.chars().take_while(|c| c.is_whitespace()).count();
out.push_str(&target);
for _ in 0..indent.saturating_sub(base_indent) {
out.push(' ');
}
out.push_str(src_line.trim());
if i < ml.len() - 1 {
out.push('\n');
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should only let the source lines stand in for the pretty line when they add up to it. With the lines of a nested rsx! left out of the source matching, the source can run ahead of the formatted lines in a reflowed closure, so the closing }) meets only the outer } and that } is written in its place. The input below formats to code that no longer parses, and main formats it correctly.

I would strongly suggest we discuss adding a roundtrip fuzzer to these type of methods, it would help catch these things trivially (as the one I dirtily just drafted did).

{
    items.iter().map(|x| {
        // c1
        rsx! { // c2
            li { "{x}" }
        }
    })
}
Suggested change
// The source lines only stand in for the pretty line if they add up to it
if !acc.contains(&compacted) {
out.push_str(line);
} else {
for (i, src_line) in ml.iter().enumerate() {
let indent = src_line.chars().take_while(|c| c.is_whitespace()).count();
out.push_str(&target);
for _ in 0..indent.saturating_sub(base_indent) {
out.push(' ');
}
out.push_str(src_line.trim());
if i < ml.len() - 1 {
out.push('\n');
}
}
}

}
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Red test for the aforementioned case.

Suggested change
}
}
fn let_before_macro() -> Element {
rsx! {
{
items.iter().map(|x| {
let y = x + 1;
rsx! {
li { "{x}" }
li { "{y}" }
}
})
}
}
}

}
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the expected output.

Suggested change
}
}
fn let_before_macro() -> Element {
rsx! {
{
items
.iter()
.map(|x| {
let y = x + 1;
rsx! {
li { "{x}" }
li { "{y}" }
}
})
}
}
}

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants