Commit b4db03d8bb29f0cded857300df460b4247e85670 | zig

author	Lewis Gaul <legaul@cisco.com> 2021-02-28 23:46:35+00:00
committer	Isaac Freund <mail@isaacfreund.com> 2021-03-15 14:55:34+01:00
log	b4db03d8bb29f0cded857300df460b4247e85670
tree	57d801b9efa60deaa615c16a1c3a6db0604e46bd
parent	a502c160cd51ce3de80b3be945245b7a91967a85

zig fmt: fix extra newline before if nested in for

Add failing testcase to reproduce issue 8088

Tidy up renderWhile(), factoring out renderWhilePayload()

Ensure correct newline is used before 'then' token in while/for/if

Handle indents for 'if' inside 'for' or 'while'

Stop special-casing 'if' compared to 'for' and 'while'

2 files changed, 76 insertions(+), 75 deletions(-)

lib
- std
  - zig
    parser_test.zig
    render.zig

lib/std/zig/parser_test.zig+25

...	@@ -3184,6 +3184,31 @@ test "zig fmt: for" {	...	@@ -3184,6 +3184,31 @@ test "zig fmt: for" {
3184	);	3184	);
3185	}	3185	}
3186		3186
		3187	test "zig fmt: for if" {
		3188	try testCanonical(
		3189	\\test "for if" {
		3190	\\ for (a) \|x\| if (x) f(x);
		3191	\\
		3192	\\ for (a) \|x\| if (x)
		3193	\\ f(x);
		3194	\\
		3195	\\ for (a) \|x\| if (x) {
		3196	\\ f(x);
		3197	\\ };
		3198	\\
		3199	\\ for (a) \|x\|
		3200	\\ if (x)
		3201	\\ f(x);
		3202	\\
		3203	\\ for (a) \|x\|
		3204	\\ if (x) {
		3205	\\ f(x);
		3206	\\ };
		3207	\\}
		3208	\\
		3209	);
		3210	}
		3211
3187	test "zig fmt: if" {	3212	test "zig fmt: if" {
3188	try testCanonical(	3213	try testCanonical(
3189	\\test "if" {	3214	\\test "if" {

lib/std/zig/render.zig+51-75

 @@ -1025,31 +1025,11 @@ fn renderWhile(gpa: *Allocator, ais: *Ais, tree: ast.Tree, while_node: ast.full.
     const then_tag = node_tags[while_node.ast.then_expr];
     if (nodeIsBlock(then_tag) and !nodeIsIf(then_tag)) {
         if (while_node.payload_token) |payload_token| {
-            try renderToken(ais, tree, payload_token - 2, .space); // rparen
-            try renderToken(ais, tree, payload_token - 1, .none); // |
-            const ident = blk: {
-                if (token_tags[payload_token] == .asterisk) {
-                    try renderToken(ais, tree, payload_token, .none); // *
-                    break :blk payload_token + 1;
-                } else {
-                    break :blk payload_token;
-                }
-            };
-            try renderToken(ais, tree, ident, .none); // identifier
-            const pipe = blk: {
-                if (token_tags[ident + 1] == .comma) {
-                    try renderToken(ais, tree, ident + 1, .space); // ,
-                    try renderToken(ais, tree, ident + 2, .none); // index
-                    break :blk ident + 3;
-                } else {
-                    break :blk ident + 1;
-                }
-            };
             const brace_space = if (while_node.ast.cont_expr == 0 and ais.isLineOverIndented())
                 Space.newline
             else
                 Space.space;
-            try renderToken(ais, tree, pipe, brace_space); // |
+            try renderWhilePayload(gpa, ais, tree, payload_token, brace_space);
         } else {
             const rparen = tree.lastToken(while_node.ast.cond_expr) + 1;
             const brace_space = if (while_node.ast.cont_expr == 0 and ais.isLineOverIndented())
 @@ -1082,38 +1062,23 @@ fn renderWhile(gpa: *Allocator, ais: *Ais, tree: ast.Tree, while_node: ast.full.
     }
     const rparen = tree.lastToken(while_node.ast.cond_expr) + 1;
+    const first_then_token = tree.firstToken(while_node.ast.then_expr);
     const last_then_token = tree.lastToken(while_node.ast.then_expr);
     const src_has_newline = !tree.tokensOnSameLine(rparen, last_then_token);
     if (src_has_newline) {
+        const newline_before_then_token = !tree.tokensOnSameLine(rparen, first_then_token);
+        const space_before_then_token: Space = if (newline_before_then_token) .newline else .space;
+        const indent_expression = !nodeIsIf(then_tag) or newline_before_then_token;
         if (while_node.payload_token) |payload_token| {
-            try renderToken(ais, tree, payload_token - 2, .space); // rparen
+            const after_space: Space = if (while_node.ast.cont_expr != 0) .space else space_before_then_token;
-            try renderToken(ais, tree, payload_token - 1, .none); // |
+            try renderWhilePayload(gpa, ais, tree, payload_token, after_space);
-            const ident = blk: {
-                if (token_tags[payload_token] == .asterisk) {
-                    try renderToken(ais, tree, payload_token, .none); // *
-                    break :blk payload_token + 1;
-                } else {
-                    break :blk payload_token;
-                }
-            };
-            try renderToken(ais, tree, ident, .none); // identifier
-            const pipe = blk: {
-                if (token_tags[ident + 1] == .comma) {
-                    try renderToken(ais, tree, ident + 1, .space); // ,
-                    try renderToken(ais, tree, ident + 2, .none); // index
-                    break :blk ident + 3;
-                } else {
-                    break :blk ident + 1;
-                }
-            };
-            const after_space: Space = if (while_node.ast.cont_expr != 0) .space else .newline;
-            try renderToken(ais, tree, pipe, after_space); // |
         } else {
-            ais.pushIndent();
+            if (indent_expression) ais.pushIndent();
-            const after_space: Space = if (while_node.ast.cont_expr != 0) .space else .newline;
+            const after_space: Space = if (while_node.ast.cont_expr != 0) .space else space_before_then_token;
             try renderToken(ais, tree, rparen, after_space); // rparen
-            ais.popIndent();
+            if (indent_expression) ais.popIndent();
         }
         if (while_node.ast.cont_expr != 0) {
             const cont_rparen = tree.lastToken(while_node.ast.cont_expr) + 1;
 @@ -1121,12 +1086,12 @@ fn renderWhile(gpa: *Allocator, ais: *Ais, tree: ast.Tree, while_node: ast.full.
             try renderToken(ais, tree, cont_lparen - 1, .space); // :
             try renderToken(ais, tree, cont_lparen, .none); // lparen
             try renderExpression(gpa, ais, tree, while_node.ast.cont_expr, .none);
-            try renderToken(ais, tree, cont_rparen, .newline); // rparen
+            try renderToken(ais, tree, cont_rparen, space_before_then_token); // rparen
         }
         if (while_node.ast.else_expr != 0) {
-            ais.pushIndent();
+            if (indent_expression) ais.pushIndent();
-            try renderExpression(gpa, ais, tree, while_node.ast.then_expr, Space.newline);
+            try renderExpression(gpa, ais, tree, while_node.ast.then_expr, .newline);
-            ais.popIndent();
+            if (indent_expression) ais.popIndent();
             const else_is_block = nodeIsBlock(node_tags[while_node.ast.else_expr]);
             if (else_is_block) {
                 try renderToken(ais, tree, while_node.else_token, .space); // else
 @@ -1145,12 +1110,18 @@ fn renderWhile(gpa: *Allocator, ais: *Ais, tree: ast.Tree, while_node: ast.full.
                 } else {
                     try renderToken(ais, tree, while_node.else_token, .newline); // else
                 }
-                try renderExpressionIndented(gpa, ais, tree, while_node.ast.else_expr, space);
+                if (indent_expression) {
-                return;
+                    return renderExpressionIndented(gpa, ais, tree, while_node.ast.else_expr, space);
+                } else {
+                    return renderExpression(gpa, ais, tree, while_node.ast.else_expr, space);
+                }
             }
         } else {
-            try renderExpressionIndented(gpa, ais, tree, while_node.ast.then_expr, space);
+            if (indent_expression) {
-            return;
+                return renderExpressionIndented(gpa, ais, tree, while_node.ast.then_expr, space);
+            } else {
+                return renderExpression(gpa, ais, tree, while_node.ast.then_expr, space);
+            }
         }
     }
 @@ -1158,27 +1129,7 @@ fn renderWhile(gpa: *Allocator, ais: *Ais, tree: ast.Tree, while_node: ast.full.
     if (while_node.payload_token) |payload_token| {
         assert(payload_token - 2 == rparen);
-        try renderToken(ais, tree, payload_token - 2, .space); // )
+        try renderWhilePayload(gpa, ais, tree, payload_token, .space);
-        try renderToken(ais, tree, payload_token - 1, .none); // |
-        const ident = blk: {
-            if (token_tags[payload_token] == .asterisk) {
-                try renderToken(ais, tree, payload_token, .none); // *
-                break :blk payload_token + 1;
-            } else {
-                break :blk payload_token;
-            }
-        };
-        try renderToken(ais, tree, ident, .none); // identifier
-        const pipe = blk: {
-            if (token_tags[ident + 1] == .comma) {
-                try renderToken(ais, tree, ident + 1, .space); // ,
-                try renderToken(ais, tree, ident + 2, .none); // index
-                break :blk ident + 3;
-            } else {
-                break :blk ident + 1;
-            }
-        };
-        try renderToken(ais, tree, pipe, .space); // |
     } else {
         try renderToken(ais, tree, rparen, .space); // )
     }
 @@ -1208,6 +1159,31 @@ fn renderWhile(gpa: *Allocator, ais: *Ais, tree: ast.Tree, while_node: ast.full.
     }
 }
+fn renderWhilePayload(gpa: *Allocator, ais: *Ais, tree: ast.Tree, payload_token: ast.TokenIndex, space: Space) Error!void {
+    const token_tags = tree.tokens.items(.tag);
+    try renderToken(ais, tree, payload_token - 2, .space); // rparen
+    try renderToken(ais, tree, payload_token - 1, .none); // |
+    const ident = blk: {
+        if (token_tags[payload_token] == .asterisk) {
+            try renderToken(ais, tree, payload_token, .none); // *
+            break :blk payload_token + 1;
+        } else {
+            break :blk payload_token;
+        }
+    };
+    try renderToken(ais, tree, ident, .none); // identifier
+    const pipe = blk: {
+        if (token_tags[ident + 1] == .comma) {
+            try renderToken(ais, tree, ident + 1, .space); // ,
+            try renderToken(ais, tree, ident + 2, .none); // index
+            break :blk ident + 3;
+        } else {
+            break :blk ident + 1;
+        }
+    };
+    try renderToken(ais, tree, pipe, space); // |
+}
 fn renderContainerField(
     gpa: *Allocator,
     ais: *Ais,