From 8653605b0a977bc76852f753928ee32e14a5d278 Mon Sep 17 00:00:00 2001 From: Enzo Date: Mon, 20 Jul 2020 16:48:33 +0200 Subject: [PATCH] fmt: allow for comments in call args and if expressions branches (#5871) --- vlib/v/ast/ast.v | 24 +++++----- vlib/v/fmt/fmt.v | 69 +++++++++++++++++++-------- vlib/v/fmt/tests/comments_expected.vv | 18 +++++++ vlib/v/fmt/tests/comments_input.vv | 12 +++++ vlib/v/fmt/tests/comments_keep.vv | 1 + vlib/v/parser/fn.v | 6 ++- vlib/v/parser/if.v | 13 +++-- vlib/v/parser/parser.v | 4 -- 8 files changed, 106 insertions(+), 41 deletions(-) diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 4383238783..ad362486cc 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -288,11 +288,12 @@ pub mut: pub struct CallArg { pub: - is_mut bool - share table.ShareType - expr Expr + is_mut bool + share table.ShareType + expr Expr + comments []Comment pub mut: - typ table.Type + typ table.Type } pub struct Return { @@ -445,14 +446,15 @@ pub mut: pub struct IfExpr { pub: - tok_kind token.Kind - left Expr // `a` in `a := if ...` - pos token.Position + tok_kind token.Kind + left Expr // `a` in `a := if ...` + pos token.Position + post_comments []Comment pub mut: - branches []IfBranch // includes all `else if` branches - is_expr bool - typ table.Type - has_else bool + branches []IfBranch // includes all `else if` branches + is_expr bool + typ table.Type + has_else bool } pub struct IfBranch { diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index fe881a8016..df7548c9a4 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -253,9 +253,7 @@ pub fn (mut f Fmt) stmt(node ast.Stmt) { } match node { ast.AssignStmt { - f.comments(node.comments, { - inline: false - }) + f.comments(node.comments, {}) for i, left in node.left { if left is ast.Ident { var_info := left.var_info() @@ -334,6 +332,7 @@ pub fn (mut f Fmt) stmt(node ast.Stmt) { name := it.name.after('.') f.writeln('enum $name {') f.comments(it.comments, { + inline: true level: .indent }) for field in it.fields { @@ -343,6 +342,7 @@ pub fn (mut f Fmt) stmt(node ast.Stmt) { f.expr(field.expr) } f.comments(field.comments, { + inline: true has_nl: false level: .indent }) @@ -351,9 +351,7 @@ pub fn (mut f Fmt) stmt(node ast.Stmt) { f.writeln('}\n') } ast.ExprStmt { - f.comments(it.comments, { - inline: false - }) + f.comments(it.comments, {}) f.expr(it.expr) if !f.single_line_if { f.writeln('') @@ -444,9 +442,7 @@ pub fn (mut f Fmt) stmt(node ast.Stmt) { f.mod(it) } ast.Return { - f.comments(it.comments, { - inline: false - }) + f.comments(it.comments, {}) f.write('return') if it.exprs.len > 1 { // multiple returns @@ -631,7 +627,9 @@ pub fn (mut f Fmt) struct_decl(node ast.StructDecl) { for j < comments.len && comments[j].pos.pos < field.pos.pos { f.indent++ f.empty_line = true - f.comment(comments[j], {}) + f.comment(comments[j], { + inline: true + }) f.writeln('') f.indent-- j++ @@ -665,7 +663,9 @@ pub fn (mut f Fmt) struct_decl(node ast.StructDecl) { for comment in node.end_comments { f.indent++ f.empty_line = true - f.comment(comment, {}) + f.comment(comment, { + inline: true + }) f.writeln('') f.indent-- } @@ -781,7 +781,9 @@ pub fn (mut f Fmt) expr(node ast.Expr) { f.write('`$node.val`') } ast.Comment { - f.comment(node, {}) + f.comment(node, { + inline: true + }) } ast.ComptimeCall { if node.is_vweb { @@ -1104,7 +1106,7 @@ enum CommentsLevel { struct CommentsOptions { has_nl bool = true - inline bool = true + inline bool level CommentsLevel = .keep } @@ -1280,19 +1282,29 @@ pub fn (mut f Fmt) if_expr(it ast.IfExpr) { (it.is_expr || f.is_assign) f.single_line_if = single_line for i, branch in it.branches { - if branch.comments.len > 0 { - f.comments(branch.comments, {}) - } if i == 0 { + f.comments(branch.comments, {}) f.write('if ') f.expr(branch.cond) f.write(' {') } else if i < it.branches.len - 1 || !it.has_else { - f.write('} else if ') + if branch.comments.len > 0 { + f.writeln('}') + f.comments(branch.comments, {}) + } else { + f.write('} ') + } + f.write('else if ') f.expr(branch.cond) f.write(' {') } else if i == it.branches.len - 1 && it.has_else { - f.write('} else {') + if branch.comments.len > 0 { + f.writeln('}') + f.comments(branch.comments, {}) + } else { + f.write('} ') + } + f.write('else {') } if single_line { f.write(' ') @@ -1306,6 +1318,12 @@ pub fn (mut f Fmt) if_expr(it ast.IfExpr) { } f.write('}') f.single_line_if = false + if it.post_comments.len > 0 { + f.writeln('') + f.comments(it.post_comments, { + has_nl: false + }) + } } pub fn (mut f Fmt) call_expr(node ast.CallExpr) { @@ -1318,6 +1336,9 @@ pub fn (mut f Fmt) call_expr(node ast.CallExpr) { // } } */ + for arg in node.args { + f.comments(arg.comments, {}) + } if node.is_method { /* // x.foo!() experiment @@ -1425,7 +1446,9 @@ pub fn (mut f Fmt) match_expr(it ast.MatchExpr) { } for branch in it.branches { if branch.comment.text != '' { - f.comment(branch.comment, {}) + f.comment(branch.comment, { + inline: true + }) f.writeln('') } if !branch.is_else { @@ -1457,7 +1480,9 @@ pub fn (mut f Fmt) match_expr(it ast.MatchExpr) { } } if branch.post_comments.len > 0 { - f.comments(branch.post_comments, {}) + f.comments(branch.post_comments, { + inline: true + }) } } f.indent-- @@ -1691,7 +1716,9 @@ pub fn (mut f Fmt) const_decl(it ast.ConstDecl) { comments := field.comments mut j := 0 for j < comments.len && comments[j].pos.pos < field.pos.pos { - f.comment(comments[j], {}) + f.comment(comments[j], { + inline: true + }) f.writeln('') j++ } diff --git a/vlib/v/fmt/tests/comments_expected.vv b/vlib/v/fmt/tests/comments_expected.vv index f8838e571d..509cc14d72 100644 --- a/vlib/v/fmt/tests/comments_expected.vv +++ b/vlib/v/fmt/tests/comments_expected.vv @@ -16,4 +16,22 @@ fn main() { // just to make it worse b, c := a, 2 d := c // and an extra one + // before arg comment + // after arg comment + println('this is a test') + // before if expr + // after if expr + if true { + println('if') + } + // before else if + // between else if + else if false { + println('else if') + } + // before else + // after else + else { + println('else') + } } diff --git a/vlib/v/fmt/tests/comments_input.vv b/vlib/v/fmt/tests/comments_input.vv index e7ab7d95c9..827d49429f 100644 --- a/vlib/v/fmt/tests/comments_input.vv +++ b/vlib/v/fmt/tests/comments_input.vv @@ -10,4 +10,16 @@ fn main() { a := /* this is a comment */ 1 b, c := /* and another comment */ a, /* just to make it worse */ 2 d := c // and an extra one + println(/* before arg comment */ 'this is a test' /* after arg comment */) + if /* before if expr */ true /* after if expr */ { + println('if') + } + // before else if + else /* between else if */ if false { + println('else if') + } + // before else + else /* after else */ { + println('else') + } } diff --git a/vlib/v/fmt/tests/comments_keep.vv b/vlib/v/fmt/tests/comments_keep.vv index 95eb19de0b..8727096392 100644 --- a/vlib/v/fmt/tests/comments_keep.vv +++ b/vlib/v/fmt/tests/comments_keep.vv @@ -10,6 +10,7 @@ fn main() { } if true { } + // some comment after an if without else n := sizeof(User) // else // else { diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 033b3f6229..6523afebfa 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -88,7 +88,7 @@ pub fn (mut p Parser) call_expr(language table.Language, mod string) ast.CallExp fn_name = registered.name } } - node := ast.CallExpr{ + return ast.CallExpr{ name: fn_name args: args mod: fn_mod @@ -101,7 +101,6 @@ pub fn (mut p Parser) call_expr(language table.Language, mod string) ast.CallExp } generic_type: generic_type } - return node } pub fn (mut p Parser) call_args() []ast.CallArg { @@ -113,11 +112,14 @@ pub fn (mut p Parser) call_args() []ast.CallArg { if is_mut { p.next() } + mut comments := p.eat_comments() e := p.expr(0) + comments << p.eat_comments() args << ast.CallArg{ is_mut: is_mut share: table.sharetype_from_flags(is_shared, is_atomic) expr: e + comments: comments } if p.tok.kind != .rpar { p.check(.comma) diff --git a/vlib/v/parser/if.v b/vlib/v/parser/if.v index 8fd9715ba3..7613100a79 100644 --- a/vlib/v/parser/if.v +++ b/vlib/v/parser/if.v @@ -24,8 +24,9 @@ fn (mut p Parser) if_expr() ast.IfExpr { if p.tok.kind == .key_if { p.next() } else { - comments = p.eat_comments() + comments << p.eat_comments() p.check(.key_else) + comments << p.eat_comments() if p.tok.kind == .key_if { p.next() } else { @@ -65,13 +66,16 @@ fn (mut p Parser) if_expr() ast.IfExpr { } mut cond := ast.Expr{} mut is_guard := false + comments << p.eat_comments() // `if x := opt() {` if p.peek_tok.kind == .decl_assign { p.open_scope() is_guard = true var_pos := p.tok.position() var_name := p.check_name() + comments << p.eat_comments() p.check(.decl_assign) + comments << p.eat_comments() expr := p.expr(0) p.scope.register(var_name, ast.Var{ name: var_name @@ -87,6 +91,7 @@ fn (mut p Parser) if_expr() ast.IfExpr { prev_guard = false cond = p.expr(0) } + comments << p.eat_comments() mut left_as_name := '' if cond is ast.InfixExpr as infix { // if sum is T @@ -117,13 +122,14 @@ fn (mut p Parser) if_expr() ast.IfExpr { comments: comments left_as_name: left_as_name } - comments = [] + comments = p.eat_comments() if p.tok.kind != .key_else { break } } return ast.IfExpr{ branches: branches + post_comments: comments pos: pos has_else: has_else } @@ -210,7 +216,8 @@ fn (mut p Parser) match_expr() ast.MatchExpr { expr := p.expr(0) p.inside_match_case = false if p.tok.kind == .dotdot { - p.error_with_pos('match only supports inclusive (`...`) ranges, not exclusive (`..`)', p.tok.position()) + p.error_with_pos('match only supports inclusive (`...`) ranges, not exclusive (`..`)', + p.tok.position()) } else if p.tok.kind == .ellipsis { p.next() expr2 := p.expr(0) diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index 0aec310d57..123c296f19 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -626,10 +626,6 @@ pub fn (mut p Parser) stmt(is_top_level bool) ast.Stmt { expr := p.expr(0) // mut call_expr := &ast.CallExpr(0) // TODO // { call_expr = it } - match expr { - ast.CallExpr {} - else {} - } return ast.GoStmt{ call_expr: expr }