go/printer: inconsistently requires ParenExpr

[dominikh]

go/printer is inconsistent in handling the absence of ParenExpr. First, let me show an example that automatically inserts required parentheses, despite no ParenExpr existing in the AST:

// (- a (- b c))
&ast.BinaryExpr{
	X:  &ast.Ident{Name: "a"},
	Op: token.SUB,
	Y: &ast.BinaryExpr{
		X:  &ast.Ident{Name: "b"},
		Op: token.SUB,
		Y:  &ast.Ident{Name: "c"},
	},
}

prints as

a - (b - c)

despite the lack of a ParenExpr. I posit that this is correct behavior.

Now, consider this example:

&ast.IfStmt{
	Cond: &ast.BinaryExpr{
		X:  &ast.Ident{Name: "a"},
		Op: token.EQL,
		Y: &ast.CompositeLit{
			Type: &ast.Ident{Name: "b"},
		},
	},
	Body: &ast.BlockStmt{},
}

prints as

if a == b{} {
}

that is, it doesn't insert the necessary parentheses to make the printed form syntactically valid.

I am of the opinion that the second example should produce correct output, inserting parentheses as necessary. go/printer already has code that strips an existing ParenExpr if it is not required (i.e. it turns if (b) into if b), but leaves it intact otherwise. It should similarly be possible to insert missing parentheses, based on the same logic.

Users of gofmt can also run into this issue: the rewrite rule (a) -> a, which is documented as stripping unnecessary parentheses, incorrectly strips parentheses of the if (x == T{}) form.

IMHO, an AST should not require explicit parentheses to be valid, and go/printer should take care of turning a valid AST into valid code. I'd like to go over go/printer and fix all instances of this problem, thus making ParenExpr optional.

/cc @griesemer for his expert opinion

[griesemer]

I agree that go/printer should insert parentheses where needed to make the output valid.