commit 09e523bd51185ae8d71c347ce606e3b74dea1d5b (tree)
parent 92915a42b55f2916f36ed6b1a2d51363331f47f8
Author: Kendall Condon <goon.pri.low@gmail.com>
Date: Sun, 22 Mar 2026 17:42:57 -0400
zig fmt: fix overindent tracking in sub-renders
This problem also affected determining if an expression became multiline
as that depends on if the line is overindented. As such,
`becomesMultilineExpr` has been replaced by `rendersMultiline` which
constructs a temporary writer which returns `error.WriteFailed` when
newlines are written. This new approach also has the advantage of being
more maintainable.
Diffstat:
2 files changed, 90 insertions(+), 372 deletions(-)
diff --git a/lib/std/zig/Ast/Render.zig b/lib/std/zig/Ast/Render.zig
@@ -652,10 +652,12 @@ fn renderExpression(r: *Render, node: Ast.Node.Index, space: Space) Error!void {
const lhs, const rhs = tree.nodeData(node).node_and_node;
const lbracket = tree.firstToken(rhs) - 1;
const rbracket = tree.lastToken(rhs) + 1;
+ try renderExpression(r, lhs, .none);
+ // One lien check must come after rendering lhs since it can influence
+ // isLineOverIndented
const one_line = tree.tokensOnSameLine(lbracket, rbracket) and
- !becomesMultilineExpr(tree, rhs);
+ !try rendersMultiline(r, rhs);
const inner_space = if (one_line) Space.none else Space.newline;
- try renderExpression(r, lhs, .none);
try ais.pushIndent(.normal);
try renderToken(r, lbracket, inner_space); // [
try renderExpression(r, rhs, inner_space);
@@ -951,380 +953,61 @@ fn renderExpressionFixup(r: *Render, node: Ast.Node.Index, space: Space) Error!v
}
}
-/// Same as becomesMultilineExpr, but returns false when `node == .none`
-fn optBecomesMultilineExpr(tree: Ast, node: Ast.Node.OptionalIndex) bool {
- return if (node.unwrap()) |payload| becomesMultilineExpr(tree, payload) else false;
-}
+fn drainNoNewline(w: *Writer, data: []const []const u8, splat: usize) Writer.Error!usize {
+ if (std.mem.indexOfScalar(u8, w.buffered(), '\n') != null) {
+ return error.WriteFailed;
+ }
-/// May return false if `node` is already multiline
-fn becomesMultilineExpr(tree: Ast, node: Ast.Node.Index) bool {
- // Conditions related to comments, doc comments, and multiline string literals are ignored
- // since they always go to the end of the line, which already make them a multi-line
- // expression (since they contain a newline).
- switch (tree.nodeTag(node)) {
- .identifier,
- .number_literal,
- .char_literal,
- .unreachable_literal,
- .anyframe_literal,
- .string_literal,
- .multiline_string_literal,
- .error_value,
- .enum_literal,
- => return false,
- .container_decl_trailing,
- .container_decl_arg_trailing,
- .container_decl_two_trailing,
- .tagged_union_trailing,
- .tagged_union_enum_tag_trailing,
- .tagged_union_two_trailing,
- .switch_comma,
- .builtin_call_two_comma,
- .builtin_call_comma,
- .call_one_comma,
- .call_comma,
- .struct_init_one_comma,
- .struct_init_dot_two_comma,
- .struct_init_dot_comma,
- .struct_init_comma,
- .array_init_one_comma,
- .array_init_dot_two_comma,
- .array_init_dot_comma,
- .array_init_comma,
- // The following always have a non-zero amount of members
- // which is also the condition for them to be multi-line.
- .block,
- .block_semicolon,
- => return true,
- .block_two,
- .block_two_semicolon,
- => return tree.nodeData(node).opt_node_and_opt_node[0] != .none,
- .container_decl,
- .container_decl_arg,
- .container_decl_two,
- .tagged_union,
- .tagged_union_enum_tag,
- .tagged_union_two,
- => {
- var buf: [2]Ast.Node.Index = undefined;
- const full = tree.fullContainerDecl(&buf, node).?;
- if (full.ast.arg.unwrap()) |arg| {
- if (becomesMultilineExpr(tree, arg))
- return true;
- }
- // This does the same checks as `isOneLineContainerDecl`, however it avoids unnecessary
- // checks related to comments and multiline strings, which would mean the container is
- // already multiple lines.
- for (full.ast.members) |member| {
- if (tree.fullContainerField(member)) |field_full| {
- for ([_]Ast.Node.OptionalIndex{
- field_full.ast.type_expr,
- field_full.ast.align_expr,
- field_full.ast.value_expr,
- }) |opt_expr| {
- if (opt_expr.unwrap()) |expr| {
- if (becomesMultilineExpr(tree, expr))
- return true;
- }
- }
- } else return true;
- }
- return false;
- },
- .error_set_decl => {
- const lbrace, const rbrace = tree.nodeData(node).token_and_token;
- return !isOneLineErrorSetDecl(tree, lbrace, rbrace);
- },
- .@"switch" => {
- const op, const extra_index = tree.nodeData(node).node_and_extra;
- const case_range = tree.extraData(extra_index, Ast.Node.SubRange);
- return @intFromEnum(case_range.end) - @intFromEnum(case_range.start) != 0 or
- becomesMultilineExpr(tree, op);
- },
- .for_simple, .@"for" => {
- const full = tree.fullFor(node).?;
- if (becomesMultilineExpr(tree, full.ast.then_expr) or
- optBecomesMultilineExpr(tree, full.ast.else_expr))
- return true;
-
- for (full.ast.inputs) |expr| {
- if (if (tree.nodeTag(expr) == .for_range) blk: {
- const lhs, const rhs = tree.nodeData(expr).node_and_opt_node;
- break :blk becomesMultilineExpr(tree, lhs) or optBecomesMultilineExpr(tree, rhs);
- } else becomesMultilineExpr(tree, expr))
- return true;
- }
- const final_input_expr = full.ast.inputs[full.ast.inputs.len - 1];
- if (tree.tokenTag(tree.lastToken(final_input_expr) + 1) == .comma)
- return true;
-
- const token_tags = tree.tokens.items(.tag);
- const payload = full.payload_token;
- const pipe = std.mem.indexOfScalarPos(Token.Tag, token_tags, payload, .pipe).?;
- return token_tags[@intCast(pipe - 1)] == .comma;
- },
- .while_simple,
- .while_cont,
- .@"while",
- => {
- const full = tree.fullWhile(node).?;
- return becomesMultilineExpr(tree, full.ast.cond_expr) or
- becomesMultilineExpr(tree, full.ast.then_expr) or
- optBecomesMultilineExpr(tree, full.ast.cont_expr) or
- optBecomesMultilineExpr(tree, full.ast.else_expr);
- },
- .if_simple,
- .@"if",
- => {
- const full = tree.fullIf(node).?;
- return becomesMultilineExpr(tree, full.ast.cond_expr) or
- becomesMultilineExpr(tree, full.ast.then_expr) or
- optBecomesMultilineExpr(tree, full.ast.else_expr);
- },
- .fn_proto_simple,
- .fn_proto_multi,
- .fn_proto_one,
- .fn_proto,
- => {
- var buf: [1]Ast.Node.Index = undefined;
- const fn_proto = tree.fullFnProto(&buf, node).?;
-
- for ([_]Ast.Node.OptionalIndex{
- fn_proto.ast.return_type,
- fn_proto.ast.align_expr,
- fn_proto.ast.addrspace_expr,
- fn_proto.ast.section_expr,
- fn_proto.ast.callconv_expr,
- }) |opt_expr| {
- if (opt_expr.unwrap()) |expr| {
- if (becomesMultilineExpr(tree, expr))
- return true;
- }
- }
- for (fn_proto.ast.params) |expr| {
- if (becomesMultilineExpr(tree, expr))
- return true;
- }
+ var n: usize = 0;
+ for (data[0 .. data.len - 1]) |v| {
+ if (std.mem.indexOfScalar(u8, v, '\n') != null) {
+ return error.WriteFailed;
+ }
+ n += v.len;
+ }
- const lparen = fn_proto.ast.fn_token + 1;
- const return_type = fn_proto.ast.return_type.unwrap().?;
- const maybe_bang = tree.firstToken(return_type) - 1;
- const rparen = fnProtoRparen(tree, fn_proto, maybe_bang);
- return !isOneLineFnProto(tree, fn_proto, lparen, rparen);
- },
- .asm_simple,
- => {
- const lhs = tree.nodeData(node).node_and_token[0];
- return becomesMultilineExpr(tree, lhs);
- },
- .@"asm",
- => {
- const lhs, const extra_index = tree.nodeData(node).node_and_extra;
- const asm_extra = tree.extraData(extra_index, Ast.Node.Asm);
- return @intFromEnum(asm_extra.items_end) - @intFromEnum(asm_extra.items_start) != 0 or
- becomesMultilineExpr(tree, lhs) or optBecomesMultilineExpr(tree, asm_extra.clobbers);
- },
- .array_type, .array_type_sentinel => {
- const array_type = tree.fullArrayType(node).?;
- const rbracket = tree.firstToken(array_type.ast.elem_type) - 1;
- return !isOneLineArrayType(tree, array_type, rbracket) or
- becomesMultilineExpr(tree, array_type.ast.elem_type);
- },
- .array_access => {
- const lhs, const rhs = tree.nodeData(node).node_and_node;
- const lbracket = tree.firstToken(rhs) - 1;
- const rbracket = tree.lastToken(rhs) + 1;
- return !tree.tokensOnSameLine(lbracket, rbracket) or
- becomesMultilineExpr(tree, lhs) or
- becomesMultilineExpr(tree, rhs);
- },
- .call_one,
- .call,
- .builtin_call_two,
- .builtin_call,
- .array_init_one,
- .array_init_dot_two,
- .array_init_dot,
- .array_init,
- .struct_init_one,
- .struct_init_dot_two,
- .struct_init_dot,
- .struct_init,
- => |tag| {
- var buf: [2]Ast.Node.Index = undefined;
- const opt_lhs: Ast.Node.OptionalIndex, const items = switch (tag) {
- .call_one, .call => blk: {
- const full = tree.fullCall(buf[0..1], node).?;
- break :blk .{ full.ast.fn_expr.toOptional(), full.ast.params };
- },
- .builtin_call_two, .builtin_call => .{ .none, tree.builtinCallParams(&buf, node).? },
- .array_init_one,
- .array_init_dot_two,
- .array_init_dot,
- .array_init,
- => blk: {
- const full = tree.fullArrayInit(&buf, node).?;
- break :blk .{ full.ast.type_expr, full.ast.elements };
- },
- .struct_init_one,
- .struct_init_dot_two,
- .struct_init_dot,
- .struct_init,
- => blk: {
- const full = tree.fullStructInit(&buf, node).?;
- break :blk .{ full.ast.type_expr, full.ast.fields };
- },
- else => unreachable,
- };
- if (opt_lhs.unwrap()) |lhs| {
- if (becomesMultilineExpr(tree, lhs))
- return true;
- }
- for (items) |expr| {
- if (becomesMultilineExpr(tree, expr))
- return true;
- }
- return false;
- },
- .assign_destructure => {
- const full = tree.assignDestructure(node);
- for (full.ast.variables) |expr| {
- if (becomesMultilineExpr(tree, expr))
- return true;
- }
- return becomesMultilineExpr(tree, full.ast.value_expr);
- },
- .ptr_type_aligned,
- .ptr_type_sentinel,
- .ptr_type,
- .ptr_type_bit_range,
- => {
- const full = tree.fullPtrType(node).?;
- return becomesMultilineExpr(tree, full.ast.child_type) or
- optBecomesMultilineExpr(tree, full.ast.sentinel) or
- optBecomesMultilineExpr(tree, full.ast.align_node) or
- optBecomesMultilineExpr(tree, full.ast.addrspace_node) or
- optBecomesMultilineExpr(tree, full.ast.bit_range_start) or
- optBecomesMultilineExpr(tree, full.ast.bit_range_end);
- },
- .slice_open,
- .slice,
- .slice_sentinel,
- => {
- const full = tree.fullSlice(node).?;
- return becomesMultilineExpr(tree, full.ast.sliced) or
- becomesMultilineExpr(tree, full.ast.start) or
- optBecomesMultilineExpr(tree, full.ast.end) or
- optBecomesMultilineExpr(tree, full.ast.sentinel);
- },
- .@"comptime",
- .@"nosuspend",
- .@"suspend",
- .@"resume",
- .bit_not,
- .bool_not,
- .negation,
- .negation_wrap,
- .optional_type,
- .address_of,
- .deref,
- .@"try",
- => return becomesMultilineExpr(tree, tree.nodeData(node).node),
- .@"return" => return optBecomesMultilineExpr(tree, tree.nodeData(node).opt_node),
- .field_access,
- .unwrap_optional,
- .grouped_expression,
- => return becomesMultilineExpr(tree, tree.nodeData(node).node_and_token[0]),
- .add,
- .add_wrap,
- .add_sat,
- .array_cat,
- .array_mult,
- .bang_equal,
- .bit_and,
- .bit_or,
- .shl,
- .shl_sat,
- .shr,
- .bit_xor,
- .bool_and,
- .bool_or,
- .div,
- .equal_equal,
- .greater_or_equal,
- .greater_than,
- .less_or_equal,
- .less_than,
- .merge_error_sets,
- .mod,
- .mul,
- .mul_wrap,
- .mul_sat,
- .sub,
- .sub_wrap,
- .sub_sat,
- .@"orelse",
- .@"catch",
- .error_union,
- .assign,
- .assign_bit_and,
- .assign_bit_or,
- .assign_shl,
- .assign_shl_sat,
- .assign_shr,
- .assign_bit_xor,
- .assign_div,
- .assign_sub,
- .assign_sub_wrap,
- .assign_sub_sat,
- .assign_mod,
- .assign_add,
- .assign_add_wrap,
- .assign_add_sat,
- .assign_mul,
- .assign_mul_wrap,
- .assign_mul_sat,
- => {
- const lhs, const rhs = tree.nodeData(node).node_and_node;
- return becomesMultilineExpr(tree, lhs) or becomesMultilineExpr(tree, rhs);
- },
- .@"break", .@"continue" => {
- const opt_expr = tree.nodeData(node).opt_token_and_opt_node[1];
- return optBecomesMultilineExpr(tree, opt_expr);
- },
- .anyframe_type => return becomesMultilineExpr(tree, tree.nodeData(node).token_and_node[1]),
- .@"errdefer",
- .@"defer",
- .for_range,
- .switch_range,
- .switch_case_one,
- .switch_case_inline_one,
- .switch_case,
- .switch_case_inline,
- .asm_output,
- .asm_input,
- .fn_decl,
- .container_field,
- .container_field_init,
- .container_field_align,
- .root,
- .global_var_decl,
- .local_var_decl,
- .simple_var_decl,
- .aligned_var_decl,
- .test_decl,
- => unreachable,
+ const pattern = data[data.len - 1];
+ if (splat != 0 and std.mem.indexOfScalar(u8, pattern, '\n') != null) {
+ return error.WriteFailed;
}
+ n += pattern.len * splat;
+
+ w.end = 0;
+ return n;
}
-fn isOneLineArrayType(
- tree: Ast,
- array_type: Ast.full.ArrayType,
- rbracket: Ast.TokenIndex,
-) bool {
- return tree.tokensOnSameLine(array_type.ast.lbracket, rbracket) and
- !becomesMultilineExpr(tree, array_type.ast.elem_count) and
- !optBecomesMultilineExpr(tree, array_type.ast.sentinel);
+fn rendersMultiline(r: *const Render, node: Ast.Node.Index) error{OutOfMemory}!bool {
+ var no_nl_buf: [64]u8 = undefined;
+ var no_nl_w: Writer = .{
+ .vtable = &.{ .drain = drainNoNewline },
+ .buffer = &no_nl_buf,
+ };
+
+ if (r.ais.disabled_offset != null) return true;
+ var sub_ais: AutoIndentingStream = .init(r.gpa, &no_nl_w, r.ais.indent_delta);
+ defer sub_ais.deinit();
+ // The following are needed to make sure isLineOverIndented is correct
+ sub_ais.indent_count = r.ais.indent_count;
+ sub_ais.applied_indent = r.ais.applied_indent;
+ sub_ais.current_line_empty = r.ais.current_line_empty;
+
+ var sub_r: Render = .{
+ .gpa = r.gpa,
+ .ais = &sub_ais,
+ .tree = r.tree,
+ .fixups = r.fixups,
+ };
+
+ renderExpression(&sub_r, node, .none) catch |e| return switch (e) {
+ error.OutOfMemory => return error.OutOfMemory,
+ error.WriteFailed => return true,
+ };
+ if (sub_ais.disabled_offset != null) return true;
+ if (std.mem.indexOfScalar(u8, no_nl_w.buffered(), '\n') != null) {
+ return true;
+ }
+
+ return false;
}
fn renderArrayType(
@@ -1335,7 +1018,9 @@ fn renderArrayType(
const tree = r.tree;
const ais = r.ais;
const rbracket = tree.firstToken(array_type.ast.elem_type) - 1;
- const one_line = isOneLineArrayType(tree, array_type, rbracket);
+ const one_line = tree.tokensOnSameLine(array_type.ast.lbracket, rbracket) and
+ !try rendersMultiline(r, array_type.ast.elem_count) and
+ (if (array_type.ast.sentinel.unwrap()) |s| !try rendersMultiline(r, s) else true);
const inner_space = if (one_line) Space.none else Space.newline;
try ais.pushIndent(.normal);
try renderToken(r, array_type.ast.lbracket, inner_space); // lbracket
@@ -2524,6 +2209,10 @@ fn renderArrayInit(
try renderSpace(&sub_r, after_expr, tokenSliceForRender(tree, after_expr).len, .none);
buf.clearRetainingCapacity();
+ // The following are needed to make sure isLineOverIndented is not influenced by
+ // the previous element.
+ sub_ais.indent_count = 0;
+ sub_ais.applied_indent = 0;
}
}
diff --git a/lib/std/zig/parser_test.zig b/lib/std/zig/parser_test.zig
@@ -6930,6 +6930,35 @@ test "zig fmt: cast builtins are not reordered with comments" {
);
}
+test "zig fmt: inner over-indented if expressions becoming multiline" {
+ try testTransform(
+ \\const a = (b or
+ \\c) and [if (d) {}]T; // If the if-statement is kept on the same line it becomes multiline
+ \\const a = (b or
+ \\c)[if (d) {}]; // If the if-statement is kept on the same line it becomes multiline
+ \\const a = .{a, b, (c or
+ \\d), if (d) {}, e, f, g,};
+ \\
+ ,
+ \\const a = (b or
+ \\ c) and [
+ \\ if (d) {}
+ \\]T; // If the if-statement is kept on the same line it becomes multiline
+ \\const a = (b or
+ \\ c)[
+ \\ if (d) {}
+ \\]; // If the if-statement is kept on the same line it becomes multiline
+ \\const a = .{
+ \\ a, b,
+ \\ (c or
+ \\ d),
+ \\ if (d) {}, e,
+ \\ f, g,
+ \\};
+ \\
+ );
+}
+
test "recovery: top level" {
try testError(
\\test "" {inline}
