From 8a0b5bad9482f01428ac72640c6b61c42a90adcb Mon Sep 17 00:00:00 2001 From: Lukas Neubert Date: Wed, 3 Mar 2021 08:23:11 +0100 Subject: [PATCH] fmt: keep empty lines in const blocks (#9071) --- vlib/v/checker/checker.v | 6 ++- vlib/v/fmt/fmt.v | 76 ++++++++++++++++------------ vlib/v/fmt/tests/empty_lines_keep.vv | 23 +++++++++ vlib/v/parser/parser.v | 12 +---- 4 files changed, 75 insertions(+), 42 deletions(-) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index ed49a4415b..bc0e008a68 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -2448,7 +2448,11 @@ pub fn (mut c Checker) const_decl(mut node ast.ConstDecl) { for i, field in node.fields { // TODO Check const name once the syntax is decided if field.name in c.const_names { - c.error('duplicate const `$field.name`', field.pos) + name_pos := token.Position{ + ...field.pos + len: util.no_cur_mod(field.name, c.mod).len + } + c.error('duplicate const `$field.name`', name_pos) } c.const_names << field.name field_names << field.name diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index 30b3a05ff6..327f469ad2 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -310,40 +310,49 @@ pub fn (f Fmt) imp_stmt_str(imp ast.Import) string { return '$imp.mod$imp_alias_suffix' } -fn (f Fmt) should_insert_newline_before_stmt(stmt ast.Stmt, prev_stmt ast.Stmt) bool { - prev_line_nr := prev_stmt.position().last_line +fn (f Fmt) should_insert_newline_before_node(node ast.Node, prev_node ast.Node) bool { // No need to insert a newline if there is already one if f.out.last_n(2) == '\n\n' { return false } - // Force a newline after a block of HashStmts - if prev_stmt is ast.HashStmt && stmt !is ast.HashStmt && stmt !is ast.ExprStmt { - return true - } - // Force a newline after a block of function declarations - if prev_stmt is ast.FnDecl { - if prev_stmt.no_body && stmt !is ast.FnDecl { + prev_line_nr := prev_node.position().last_line + // The nodes are Stmts + if node is ast.Stmt && prev_node is ast.Stmt { + stmt := node as ast.Stmt + prev_stmt := prev_node as ast.Stmt + // Force a newline after a block of HashStmts + if prev_stmt is ast.HashStmt && stmt !is ast.HashStmt && stmt !is ast.ExprStmt { return true } - } - // The stmt shouldn't have a newline before - if stmt.position().line_nr - prev_line_nr <= 1 { - return false - } - // Imports are handled special hence they are ignored here - if stmt is ast.Import || prev_stmt is ast.Import { - return false - } - // Attributes are not respected in the stmts position, so this requires a manual check - if stmt is ast.StructDecl { - if stmt.attrs.len > 0 && stmt.attrs[0].pos.line_nr - prev_line_nr <= 1 { + // Force a newline after a block of function declarations + if prev_stmt is ast.FnDecl { + if prev_stmt.no_body && stmt !is ast.FnDecl { + return true + } + } + // The stmt shouldn't have a newline before + // if stmt.position().line_nr - prev_line_nr <= 1 { + // return false + // } + // Imports are handled special hence they are ignored here + if stmt is ast.Import || prev_stmt is ast.Import { return false } - } - if stmt is ast.FnDecl { - if stmt.attrs.len > 0 && stmt.attrs[0].pos.line_nr - prev_line_nr <= 1 { - return false + // Attributes are not respected in the stmts position, so this requires a manual check + if stmt is ast.StructDecl { + if stmt.attrs.len > 0 && stmt.attrs[0].pos.line_nr - prev_line_nr <= 1 { + return false + } } + if stmt is ast.FnDecl { + if stmt.attrs.len > 0 && stmt.attrs[0].pos.line_nr - prev_line_nr <= 1 { + return false + } + } + } + // The node shouldn't have a newline before + if node.position().line_nr - prev_line_nr <= 1 { + return false } return true } @@ -352,7 +361,7 @@ pub fn (mut f Fmt) stmts(stmts []ast.Stmt) { mut prev_stmt := if stmts.len > 0 { stmts[0] } else { ast.Stmt{} } f.indent++ for stmt in stmts { - if !f.pref.building_v && f.should_insert_newline_before_stmt(stmt, prev_stmt) { + if !f.pref.building_v && f.should_insert_newline_before_node(stmt, prev_stmt) { f.out.writeln('') } f.stmt(stmt) @@ -2241,13 +2250,17 @@ pub fn (mut f Fmt) const_decl(it ast.ConstDecl) { } } f.indent++ + mut prev_field := if it.fields.len > 0 { ast.Node(it.fields[0]) } else { ast.Node{} } for field in it.fields { - comments := field.comments - mut j := 0 - for j < comments.len && comments[j].pos.pos < field.pos.pos { - f.comment(comments[j], inline: true) + if field.comments.len > 0 { + if f.should_insert_newline_before_node(ast.Expr(field.comments[0]), prev_field) { + f.writeln('') + } + f.comments(field.comments, inline: true) + prev_field = ast.Expr(field.comments.last()) + } + if f.should_insert_newline_before_node(field, prev_field) { f.writeln('') - j++ } name := field.name.after('.') f.write('$name ') @@ -2255,6 +2268,7 @@ pub fn (mut f Fmt) const_decl(it ast.ConstDecl) { f.write('= ') f.expr(field.expr) f.writeln('') + prev_field = field } f.comments_after_last_field(it.end_comments) f.indent-- diff --git a/vlib/v/fmt/tests/empty_lines_keep.vv b/vlib/v/fmt/tests/empty_lines_keep.vv index ce916657b3..1321ee3f19 100644 --- a/vlib/v/fmt/tests/empty_lines_keep.vv +++ b/vlib/v/fmt/tests/empty_lines_keep.vv @@ -1,3 +1,26 @@ +// Keep empty lines in const blocks +const ( + _ = SomeStruct{ + val: 'Multiline field exprs should cause no problems' + } + _ = 1 + + _ = 'Keep this empty line before' + // Comment before a field + _ = 2 + + // The empty line above should stay too + _ = 3 + + // This comment doesn't really belong anywhere + + _ = 'A multiline string with a StringInterLiteral... + $foo + ...and the ending brace on a newline had a wrong pos.last_line. + ' + _ = 4 +) + fn keep_single_empty_line() { println('a') println('b') diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index 72e0218f47..131b8b980b 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -1772,7 +1772,7 @@ fn (mut p Parser) string_expr() ast.Expr { fills: fills fmts: fmts fmt_poss: fposs - pos: pos + pos: pos.extend(p.prev_tok.position()) } // need_fmts: prelimery - until checker finds out if really needed p.inside_str_interp = false @@ -2012,12 +2012,6 @@ fn (mut p Parser) const_decl() ast.ConstDecl { const_pos := p.tok.position() p.check(.key_const) is_block := p.tok.kind == .lpar - /* - if p.tok.kind != .lpar { - p.error_with_pos('const declaration is missing parentheses `( ... )`', const_pos) - return ast.ConstDecl{} - } - */ if is_block { p.next() // ( } @@ -2039,8 +2033,6 @@ fn (mut p Parser) const_decl() ast.ConstDecl { pos) } full_name := p.prepend_mod(name) - // name := p.check_name() - // println('!!const: $name') p.check(.assign) if p.tok.kind == .key_fn { p.error('const initializer fn literal is not a constant') @@ -2052,7 +2044,7 @@ fn (mut p Parser) const_decl() ast.ConstDecl { mod: p.mod is_pub: is_pub expr: expr - pos: pos + pos: pos.extend(expr.position()) comments: comments } fields << field