fix(fmt): preserve syntax across formatting round trips

crates/fmt/src/ast/expr.rs

@@ -42,6 +42,20 @@ fn bin_op_precedence(op: &BinOp) -> u8 {
     }
 }
 
+/// Returns true for operators the parser refuses to parse as a continuation
+/// when they appear at the start of a line: `-` reads as a new expression
+/// statement starting with unary minus, and `<` / `<<` can begin a qualified
+/// type (`<T as Trait>::...`). For these the line break must be placed after
+/// the operator, never before it. Keep in sync with the line-start operator
+/// rejections in `parser/src/parser/expr.rs`.
+fn op_rejected_at_line_start(op: &BinOp) -> bool {
+    use parser::ast::{ArithBinOp, CompBinOp};
+    matches!(
+        op,
+        BinOp::Arith(ArithBinOp::Sub(_) | ArithBinOp::LShift(_)) | BinOp::Comp(CompBinOp::Lt(_))
+    )
+}
+
 /// If expression is a binary expression with the given precedence, return the BinExpr.
 fn as_bin_expr_with_precedence(expr: &ast::Expr, precedence: u8) -> Option<ast::BinExpr> {
     if let ExprKind::Bin(bin) = expr.kind()
@@ -99,15 +113,17 @@ fn format_bin_expr_inner<'a>(
 
     let precedence = bin_op_precedence(&op);
 
-    // Collect all operands and operators at this precedence level
+    // Collect all operands and operators at this precedence level.
+    // The bool records whether the parser rejects the operator at line start,
+    // in which case the break goes after the operator instead of before it.
     let mut operands: Vec<ast::Expr> = Vec::new();
-    let mut operators: Vec<String> = Vec::new();
+    let mut operators: Vec<(String, bool)> = Vec::new();
 
     fn collect<'a>(
         expr: &ast::BinExpr,
         precedence: u8,
         operands: &mut Vec<ast::Expr>,
-        operators: &mut Vec<String>,
+        operators: &mut Vec<(String, bool)>,
         ctx: &'a RewriteContext<'a>,
     ) {
         if let Some(lhs) = expr.lhs() {
@@ -119,7 +135,10 @@ fn format_bin_expr_inner<'a>(
         }
 
         if let Some(op) = expr.op() {
-            operators.push(ctx.snippet_node_or_token(&op.syntax()));
+            operators.push((
+                ctx.snippet_node_or_token(&op.syntax()),
+                op_rejected_at_line_start(&op),
+            ));
         }
 
         if let Some(rhs) = expr.rhs() {
@@ -141,7 +160,7 @@ fn format_bin_expr_inner<'a>(
     let mut result = first.to_doc(ctx);
 
     for (i, operand) in operands.iter().skip(1).enumerate() {
-        let op_str = &operators[i];
+        let (op_str, break_after_op) = &operators[i];
 
         // Check if operand is a higher-precedence binary expression
         let higher_prec_bin = if let ExprKind::Bin(inner_bin) = operand.kind() {
@@ -153,22 +172,26 @@ fn format_bin_expr_inner<'a>(
             None
         };
 
-        if let Some(inner_bin) = higher_prec_bin {
+        let operand_doc = if let Some(inner_bin) = higher_prec_bin {
             // Format inner chain without its own nesting, we control it here
-            let inner_doc = format_bin_expr_inner(&inner_bin, ctx, false);
-            result = result
-                .append(alloc.line())
-                .append(alloc.text(op_str.clone()))
+            format_bin_expr_inner(&inner_bin, ctx, false).nest(indent)
+        } else {
+            operand.to_doc(ctx)
+        };
+
+        result = if *break_after_op {
+            result
                 .append(alloc.text(" "))
-                .append(inner_doc.nest(indent));
+                .append(alloc.text(op_str.clone()))
+                .append(alloc.line())
+                .append(operand_doc)
         } else {
-            let operand_doc = operand.to_doc(ctx);
-            result = result
+            result
                 .append(alloc.line())
                 .append(alloc.text(op_str.clone()))
                 .append(alloc.text(" "))
-                .append(operand_doc);
-        }
+                .append(operand_doc)
+        };
     }
 
     if apply_outer_nest {

crates/fmt/src/ast/items.rs

@@ -20,6 +20,19 @@ macro_rules! token_doc_item_like_if_comments {
     };
 }
 
+fn next_non_trivia_token(token: &parser::SyntaxToken) -> Option<parser::SyntaxToken> {
+    use parser::syntax_kind::SyntaxKind::*;
+
+    let mut next = token.next_token();
+    while let Some(tok) = next {
+        if !matches!(tok.kind(), WhiteSpace | Newline) {
+            return Some(tok);
+        }
+        next = tok.next_token();
+    }
+    None
+}
+
 fn token_piece_basic<'a>(
     ctx: &'a RewriteContext<'a>,
     token: parser::SyntaxToken,
@@ -30,6 +43,11 @@ fn token_piece_basic<'a>(
     let text = alloc.text(token.text().to_string());
     Some(match token.kind() {
         FnKw => TokenPiece::new(alloc.nil()),
+        // `pub` followed by `(` is a visibility restriction (`pub(ingot)`);
+        // keep the paren attached to the keyword.
+        PubKw if next_non_trivia_token(&token).is_some_and(|t| t.kind() == LParen) => {
+            TokenPiece::new(text)
+        }
         PubKw | UnsafeKw | MutKw | StructKw | ContractKw | EnumKw | TraitKw | MsgKw | ModKw
         | UseKw | ConstKw | StaticAssertKw | TypeKw | ExternKw => {
             TokenPiece::new(text).space_after()
@@ -69,6 +87,17 @@ fn token_doc_item_node_piece<'a>(
         };
     }
 
+    // Visibility restriction after `pub(`: the `(` token precedes this node
+    // in the CST, so the piece renders only `ingot)` / `super)`.
+    if let Some(vis) = ast::VisRestriction::cast(node.clone()) {
+        let restriction = if vis.super_kw().is_some() {
+            "super)"
+        } else {
+            "ingot)"
+        };
+        return Some(TokenPiece::new(ctx.alloc.text(restriction)).space_after());
+    }
+
     first_some!(
         piece!(ast::AttrList),
         piece!(ast::FuncSignature),
@@ -116,6 +145,16 @@ fn attrs_doc<'a, N: ast::AttrListOwner + AstNode>(
     }
 }
 
+/// Renders `pub `, `pub(ingot) `, or `pub(super) ` depending on the
+/// visibility restriction attached to the `pub` keyword.
+fn pub_text(vis_restriction: Option<&ast::VisRestriction>) -> &'static str {
+    match vis_restriction {
+        Some(vis) if vis.ingot_kw().is_some() => "pub(ingot) ",
+        Some(vis) if vis.super_kw().is_some() => "pub(super) ",
+        _ => "pub ",
+    }
+}
+
 /// Helper to build item modifier document (pub, unsafe).
 fn modifier_doc<'a, N: ItemModifierOwner + AstNode>(
     node: &N,
@@ -124,7 +163,7 @@ fn modifier_doc<'a, N: ItemModifierOwner + AstNode>(
     let alloc = &ctx.alloc;
     let mut doc = alloc.nil();
     if node.pub_kw().is_some() {
-        doc = doc.append(alloc.text("pub "));
+        doc = doc.append(alloc.text(pub_text(node.vis_restriction().as_ref())));
     }
     if node.unsafe_kw().is_some() {
         doc = doc.append(alloc.text("unsafe "));
@@ -168,6 +207,26 @@ fn where_doc<'a, N: ast::WhereClauseOwner + AstNode>(
     }
 }
 
+/// Counts newlines in a node's leading trivia (before its first non-trivia
+/// token or child node).
+fn node_leading_newlines(ctx: &RewriteContext, node: &parser::SyntaxNode) -> usize {
+    use parser::syntax_kind::SyntaxKind;
+    use parser::syntax_node::NodeOrToken;
+
+    let mut count = 0;
+    for child in node.children_with_tokens() {
+        match child {
+            NodeOrToken::Token(token) => match token.kind() {
+                SyntaxKind::Newline => count += newline_count(ctx.snippet(token.text_range())),
+                SyntaxKind::WhiteSpace => {}
+                _ => break,
+            },
+            NodeOrToken::Node(_) => break,
+        }
+    }
+    count
+}
+
 /// Format a block of items `{ ... }`, preserving whether there was a blank line
 /// between entries in the source (2+ newlines => one blank line; otherwise none).
 /// Takes a syntax node and a function to cast child nodes to the item type.
@@ -187,9 +246,13 @@ fn block_items_doc<'a, T: ToDoc>(
     for child in syntax.children_with_tokens() {
         let entry_doc = match child {
             NodeOrToken::Node(node) => {
-                let Some(item) = cast_fn(node) else {
+                let Some(item) = cast_fn(node.clone()) else {
                     continue;
                 };
+                // Item nodes own their leading comments and newlines, but the
+                // node's doc drops that leading trivia; count it here so blank
+                // lines between entries are preserved without doubling.
+                pending_newlines += node_leading_newlines(ctx, &node);
                 Some(item.to_doc(ctx))
             }
             NodeOrToken::Token(token) => match token.kind() {
@@ -779,7 +842,7 @@ impl ToDoc for ast::RecordFieldDef {
         let mut doc = attrs;
 
         if self.pub_kw().is_some() {
-            doc = doc.append(alloc.text("pub "));
+            doc = doc.append(alloc.text(pub_text(self.vis_restriction().as_ref())));
         }
 
         if self.mut_kw().is_some() {

crates/fmt/src/ast/types.rs

@@ -105,6 +105,26 @@ macro_rules! block_list_auto_impl {
 block_list_auto_impl!(block_list_auto, block_list);
 block_list_auto_impl!(block_list_spaced_auto, block_list_spaced);
 
+/// Returns true if the first non-trivia token of the node is `<` (a qualified
+/// type such as `<T as Trait>::Item`). Such a node must not be emitted
+/// directly after another `<`, or the two would lex as a single `<<` shift
+/// token on reparse; callers insert a space between them.
+pub(crate) fn starts_with_lt(syntax: &parser::SyntaxNode) -> bool {
+    syntax
+        .descendants_with_tokens()
+        .filter_map(|child| child.into_token())
+        .find(|token| {
+            !matches!(
+                token.kind(),
+                SyntaxKind::WhiteSpace
+                    | SyntaxKind::Newline
+                    | SyntaxKind::Comment
+                    | SyntaxKind::DocComment
+            )
+        })
+        .is_some_and(|token| token.kind() == SyntaxKind::Lt)
+}
+
 pub fn has_comment_tokens(syntax: &parser::SyntaxNode) -> bool {
     syntax.children_with_tokens().any(|child| {
         matches!(
@@ -198,6 +218,13 @@ impl<'a> TokenDocBuilder<'a> {
     fn push_piece(&mut self, piece: TokenPiece<'a>) {
         let alloc = &self.ctx.alloc;
 
+        // Leading newlines inside a node are the enclosing container's
+        // concern (it emits the separation between entries); rendering them
+        // here too would add another blank line on every format pass.
+        if self.is_start {
+            self.pending_newlines = 0;
+        }
+
         if self.pending_newlines > 0 {
             let doc = hardlines(alloc, self.pending_newlines).append(piece.doc);
             self.append(if piece.nest {
@@ -980,8 +1007,16 @@ impl ToDoc for ast::QualifiedType {
                 None => return alloc.nil(),
             };
 
+            // A qualified type nested directly inside another (`< <A as B>::C
+            // as D>`) needs a space after the opening `<` so it doesn't lex
+            // as `<<`.
+            let open = if self.ty().is_some_and(|t| starts_with_lt(t.syntax())) {
+                "< "
+            } else {
+                "<"
+            };
             return alloc
-                .text("<")
+                .text(open)
                 .append(ty)
                 .append(alloc.text(" as "))
                 .append(trait_path)
@@ -1012,10 +1047,21 @@ impl ToDoc for ast::QualifiedType {
 impl ToDoc for ast::GenericArgList {
     fn to_doc<'a>(&self, ctx: &'a RewriteContext<'a>) -> Doc<'a> {
         let indent = ctx.config.indent_width as isize;
+        // A first argument that is itself a qualified type starts with `<`;
+        // keep a space after the opening `<` so it doesn't lex as `<<`.
+        let open = if self
+            .into_iter()
+            .next()
+            .is_some_and(|arg| starts_with_lt(arg.syntax()))
+        {
+            "< "
+        } else {
+            "<"
+        };
         block_list_auto(
             ctx,
             self.syntax(),
-            "<",
+            open,
             ">",
             ast::GenericArg::cast,
             indent,