From 6586a9a657f057f698a948aa3cf1a78b4d6651dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Uwe=20Kr=C3=BCger?=
 <45282134+UweKrueger@users.noreply.github.com>
Date: Thu, 23 Jul 2020 00:32:34 +0200
Subject: [PATCH] fmt: give braced sub expressions a higher priority (#5941)

---
 vlib/v/checker/checker.v                 | 38 +++++++++++-------------
 vlib/v/fmt/fmt.v                         | 26 ++++++++++++----
 vlib/v/fmt/tests/expressions_expected.vv |  6 ++--
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v
index 6780a67a1b..752f811454 100644
--- a/vlib/v/checker/checker.v
+++ b/vlib/v/checker/checker.v
@@ -788,12 +788,12 @@ fn (mut c Checker) check_map_and_filter(is_map bool, elem_typ table.Type, call_e
 		ast.AnonFn {
 			if arg_expr.decl.args.len > 1 {
 				c.error('function needs exactly 1 argument', call_expr.pos)
-			} else if is_map && (arg_expr.decl.return_type != elem_typ ||
-				arg_expr.decl.args[0].typ != elem_typ) {
+			} else if is_map &&
+				(arg_expr.decl.return_type != elem_typ || arg_expr.decl.args[0].typ != elem_typ) {
 				c.error('type mismatch, should use `fn(a $elem_sym.name) $elem_sym.name {...}`',
 					call_expr.pos)
-			} else if !is_map && (arg_expr.decl.return_type != table.bool_type ||
-				arg_expr.decl.args[0].typ != elem_typ) {
+			} else if !is_map &&
+				(arg_expr.decl.return_type != table.bool_type || arg_expr.decl.args[0].typ != elem_typ) {
 				c.error('type mismatch, should use `fn(a $elem_sym.name) bool {...}`',
 					call_expr.pos)
 			}
@@ -809,8 +809,8 @@ fn (mut c Checker) check_map_and_filter(is_map bool, elem_typ table.Type, call_e
 				} else if is_map && (func.return_type != elem_typ || func.args[0].typ != elem_typ) {
 					c.error('type mismatch, should use `fn(a $elem_sym.name) $elem_sym.name {...}`',
 						call_expr.pos)
-				} else if !is_map && (func.return_type != table.bool_type ||
-					func.args[0].typ != elem_typ) {
+				} else if !is_map &&
+					(func.return_type != table.bool_type || func.args[0].typ != elem_typ) {
 					c.error('type mismatch, should use `fn(a $elem_sym.name) bool {...}`',
 						call_expr.pos)
 				}
@@ -904,8 +904,8 @@ pub fn (mut c Checker) call_method(mut call_expr ast.CallExpr) table.Type {
 		}
 	}
 	if method := c.table.type_find_method(left_type_sym, method_name) {
-		if !method.is_pub && !c.is_builtin_mod && !c.pref.is_test &&
-			left_type_sym.mod != c.mod && left_type_sym.mod != '' { // method.mod != c.mod {
+		if !method.is_pub && !c.is_builtin_mod && !c.pref.is_test && left_type_sym.mod != c.mod &&
+			left_type_sym.mod != '' { // method.mod != c.mod {
 			// If a private method is called outside of the module
 			// its receiver type is defined in, show an error.
 			// println('warn $method_name lef.mod=$left_type_sym.mod c.mod=$c.mod')
@@ -1131,8 +1131,7 @@ pub fn (mut c Checker) call_fn(mut call_expr ast.CallExpr) table.Type {
 	if f.is_deprecated {
 		c.warn('function `$f.name` has been deprecated', call_expr.pos)
 	}
-	if f.is_unsafe && !c.inside_unsafe &&
-		f.language == .c && f.name[2] in [`m`, `s`] &&
+	if f.is_unsafe && !c.inside_unsafe && f.language == .c && f.name[2] in [`m`, `s`] &&
 		f.mod == 'builtin' {
 		// builtin C.m*, C.s* only - temp
 		c.warn('function `$f.name` must be called from an `unsafe` block', call_expr.pos)
@@ -1491,8 +1490,8 @@ pub fn (mut c Checker) return_stmt(mut return_stmt ast.Return) {
 	}
 	for i, exp_type in expected_types {
 		got_typ := c.unwrap_generic(got_types[i])
-		if got_typ.has_flag(.optional) && (!exp_type.has_flag(.optional) || c.table.type_to_str(got_typ) !=
-			c.table.type_to_str(exp_type)) {
+		if got_typ.has_flag(.optional) &&
+			(!exp_type.has_flag(.optional) || c.table.type_to_str(got_typ) != c.table.type_to_str(exp_type)) {
 			pos := return_stmt.exprs[i].position()
 			c.error('cannot use `${c.table.type_to_str(got_typ)}` as type `${c.table.type_to_str(exp_type)}` in return argument',
 				pos)
@@ -1674,8 +1673,7 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) {
 				if !left_sym.is_number() && left_type != table.string_type && !left_sym.is_pointer() {
 					c.error('operator += not defined on left operand type `$left_sym.name`',
 						left.position())
-				} else if !right_sym.is_number() && right_type != table.string_type &&
-					!right_sym.is_pointer() {
+				} else if !right_sym.is_number() && right_type != table.string_type && !right_sym.is_pointer() {
 					c.error('operator += not defined on right operand type `$right_sym.name`',
 						right.position())
 				}
@@ -1823,8 +1821,7 @@ pub fn (mut c Checker) array_init(mut array_init ast.ArrayInit) table.Type {
 			array_init.typ = table.new_type(idx)
 		}
 		array_init.elem_type = elem_type
-	} else if array_init.is_fixed && array_init.exprs.len == 1 &&
-		array_init.elem_type != table.void_type {
+	} else if array_init.is_fixed && array_init.exprs.len == 1 && array_init.elem_type != table.void_type {
 		// [50]byte
 		mut fixed_size := 1
 		match array_init.exprs[0] as init_expr {
@@ -2843,8 +2840,7 @@ pub fn (mut c Checker) if_expr(mut node ast.IfExpr) table.Type {
 		if branch.cond is ast.InfixExpr {
 			infix := branch.cond as ast.InfixExpr
 			if infix.op == .key_is &&
-				(infix.left is ast.Ident || infix.left is ast.SelectorExpr) &&
-				infix.right is ast.Type {
+				(infix.left is ast.Ident || infix.left is ast.SelectorExpr) && infix.right is ast.Type {
 				right_expr := infix.right as ast.Type
 				is_variable := if infix.left is ast.Ident { (infix.left as ast.Ident).kind ==
 						.variable } else { true }
@@ -3252,7 +3248,8 @@ fn (mut c Checker) sql_stmt(mut node ast.SqlStmt) table.Type {
 }
 
 fn (c &Checker) fetch_and_verify_orm_fields(info table.Struct, pos token.Position, table_name string) []table.Field {
-	fields := info.fields.filter(it.typ in [table.string_type, table.int_type, table.bool_type] &&
+	fields := info.fields.filter(it.typ in
+		[table.string_type, table.int_type, table.bool_type] &&
 		'skip' !in it.attrs)
 	if fields.len == 0 {
 		c.error('V orm: select: empty fields in `$table_name`', pos)
@@ -3341,8 +3338,7 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) {
 	}
 	c.stmts(node.stmts)
 	returns := c.returns || has_top_return(node.stmts)
-	if node.language == .v && !node.no_body &&
-		node.return_type != table.void_type && !returns &&
+	if node.language == .v && !node.no_body && node.return_type != table.void_type && !returns &&
 		node.name !in ['panic', 'exit'] {
 		c.error('missing return at end of function `$node.name`', node.pos)
 	}
diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v
index 8cecdbd43c..edeea98dad 100644
--- a/vlib/v/fmt/fmt.v
+++ b/vlib/v/fmt/fmt.v
@@ -183,8 +183,8 @@ fn (mut f Fmt) adjust_complete_line() {
 			}
 		}
 		// emergency fallback: decrease penalty in front of long unbreakable parts
-		if i > 0 && buf.len > max_len[3] - max_len[1] && f.penalties[i - 1] > 0 {
-			f.penalties[i - 1] = if buf.len >= max_len[2] { 0 } else { 1 }
+		if i > 0 && buf.len > 55 && f.penalties[i - 1] > 0 {
+			f.penalties[i - 1] = if buf.len >= 72 { 0 } else { 1 }
 		}
 	}
 }
@@ -1246,11 +1246,25 @@ pub fn (mut f Fmt) infix_expr(node ast.InfixExpr) {
 		}
 		f.expr_bufs << f.out.str()
 		mut penalty := 3
-		if node.left is ast.InfixExpr || node.left is ast.ParExpr {
-			penalty--
+		match node.left as left {
+			ast.InfixExpr {
+				if int(token.precedences[left.op]) > int(token.precedences[node.op]) {
+					penalty--
+				}
+			}
+			ast.ParExpr {
+				penalty = 1
+			}
+			else {}
 		}
-		if node.right is ast.InfixExpr || node.right is ast.ParExpr {
-			penalty--
+		match node.right as right {
+			ast.InfixExpr {
+				penalty--
+			}
+			ast.ParExpr {
+				penalty = 1
+			}
+			else {}
 		}
 		f.penalties << penalty
 		// combine parentheses level with operator precedence to form effective precedence
diff --git a/vlib/v/fmt/tests/expressions_expected.vv b/vlib/v/fmt/tests/expressions_expected.vv
index ead5e6e78e..a84294ead4 100644
--- a/vlib/v/fmt/tests/expressions_expected.vv
+++ b/vlib/v/fmt/tests/expressions_expected.vv
@@ -11,7 +11,8 @@ fn string_inter_lit(mut c checker.Checker, mut node ast.StringInterLiteral) tabl
 		typ := c.table.unalias_num_type(ftyp)
 		mut fmt := node.fmts[i]
 		// analyze and validate format specifier
-		if fmt !in [`E`, `F`, `G`, `e`, `f`, `g`, `d`, `u`, `x`, `X`, `o`, `c`, `s`, `p`, `_`] {
+		if fmt !in
+			[`E`, `F`, `G`, `e`, `f`, `g`, `d`, `u`, `x`, `X`, `o`, `c`, `s`, `p`, `_`] {
 			c.error('unknown format specifier `${fmt:c}`', node.fmt_poss[i])
 		}
 		if node.precisions[i] != 0 && !typ.is_float() {
@@ -37,8 +38,7 @@ fn string_inter_lit(mut c checker.Checker, mut node ast.StringInterLiteral) tabl
 
 fn get_some_val(a_test, b_test, c_test, d_test, e_test, f_test f64) f64 {
 	return a_test * b_test * c_test * d_test +
-		e_test * f_test * a_test * d_test +
-		a_test * b_test * c_test
+		e_test * f_test * a_test * d_test + a_test * b_test * c_test
 }
 
 fn main() {