go/printer, gofmt: improved comma placement

Not a Go 1 issue, but appeared to be fairly easy to fix.

- Note that a few existing test cases look slightly worse but
  those cases were not representative for real code. All real
  code looks better now.

- Manual move of the comment in go/scanner/example_test.go
  before applying gofmt.

- gofmt -w $GOROOT/src $GOROOT/misc

Fixes #3062.

R=rsc
CC=golang-dev
https://golang.org/cl/5674093

10 files changed, 106 insertions, 27 deletions

diff --git a/src/pkg/crypto/tls/handshake_client.go b/src/pkg/crypto/tls/handshake_client.go
index 687e5ef11b..0d7b806ff5 100644
--- a/src/pkg/crypto/tls/handshake_client.go
+++ b/src/pkg/crypto/tls/handshake_client.go
@@ -273,7 +273,7 @@ func (c *Conn) clientHandshake() error {
 	masterSecret, clientMAC, serverMAC, clientKey, serverKey, clientIV, serverIV :=
 		keysFromPreMasterSecret(c.vers, preMasterSecret, hello.random, serverHello.random, suite.macLen, suite.keyLen, suite.ivLen)
 
-	clientCipher := suite.cipher(clientKey, clientIV, false /* not for reading */ )
+	clientCipher := suite.cipher(clientKey, clientIV, false /* not for reading */)
 	clientHash := suite.mac(c.vers, clientMAC)
 	c.out.prepareCipherSpec(c.vers, clientCipher, clientHash)
 	c.writeRecord(recordTypeChangeCipherSpec, []byte{1})
@@ -294,7 +294,7 @@ func (c *Conn) clientHandshake() error {
 	finishedHash.Write(finished.marshal())
 	c.writeRecord(recordTypeHandshake, finished.marshal())
 
-	serverCipher := suite.cipher(serverKey, serverIV, true /* for reading */ )
+	serverCipher := suite.cipher(serverKey, serverIV, true /* for reading */)
 	serverHash := suite.mac(c.vers, serverMAC)
 	c.in.prepareCipherSpec(c.vers, serverCipher, serverHash)
 	c.readRecord(recordTypeChangeCipherSpec)
diff --git a/src/pkg/crypto/tls/handshake_server.go b/src/pkg/crypto/tls/handshake_server.go
index fb53767f3e..23ec558723 100644
--- a/src/pkg/crypto/tls/handshake_server.go
+++ b/src/pkg/crypto/tls/handshake_server.go
@@ -295,7 +295,7 @@ FindCipherSuite:
 	masterSecret, clientMAC, serverMAC, clientKey, serverKey, clientIV, serverIV :=
 		keysFromPreMasterSecret(c.vers, preMasterSecret, clientHello.random, hello.random, suite.macLen, suite.keyLen, suite.ivLen)
 
-	clientCipher := suite.cipher(clientKey, clientIV, true /* for reading */ )
+	clientCipher := suite.cipher(clientKey, clientIV, true /* for reading */)
 	clientHash := suite.mac(c.vers, clientMAC)
 	c.in.prepareCipherSpec(c.vers, clientCipher, clientHash)
 	c.readRecord(recordTypeChangeCipherSpec)
@@ -333,7 +333,7 @@ FindCipherSuite:
 
 	finishedHash.Write(clientFinished.marshal())
 
-	serverCipher := suite.cipher(serverKey, serverIV, false /* not for reading */ )
+	serverCipher := suite.cipher(serverKey, serverIV, false /* not for reading */)
 	serverHash := suite.mac(c.vers, serverMAC)
 	c.out.prepareCipherSpec(c.vers, serverCipher, serverHash)
 	c.writeRecord(recordTypeChangeCipherSpec, []byte{1})
diff --git a/src/pkg/crypto/x509/x509.go b/src/pkg/crypto/x509/x509.go
index 212662e21b..3116525d33 100644
--- a/src/pkg/crypto/x509/x509.go
+++ b/src/pkg/crypto/x509/x509.go
@@ -791,7 +791,7 @@ var (
 )
 
 func buildExtensions(template *Certificate) (ret []pkix.Extension, err error) {
-	ret = make([]pkix.Extension, 7 /* maximum number of elements. */ )
+	ret = make([]pkix.Extension, 7 /* maximum number of elements. */)
 	n := 0
 
 	if template.KeyUsage != 0 {
diff --git a/src/pkg/exp/proxy/socks5.go b/src/pkg/exp/proxy/socks5.go
index 466e135eb1..62fa5c9296 100644
--- a/src/pkg/exp/proxy/socks5.go
+++ b/src/pkg/exp/proxy/socks5.go
@@ -98,9 +98,9 @@ func (s *socks5) Dial(network, addr string) (net.Conn, error) {
 
 	buf = append(buf, socks5Version)
 	if len(s.user) > 0 && len(s.user) < 256 && len(s.password) < 256 {
-		buf = append(buf, 2, /* num auth methods */ socks5AuthNone, socks5AuthPassword)
+		buf = append(buf, 2 /* num auth methods */, socks5AuthNone, socks5AuthPassword)
 	} else {
-		buf = append(buf, 1, /* num auth methods */ socks5AuthNone)
+		buf = append(buf, 1 /* num auth methods */, socks5AuthNone)
 	}
 
 	if _, err = conn.Write(buf); err != nil {
@@ -139,7 +139,7 @@ func (s *socks5) Dial(network, addr string) (net.Conn, error) {
 	}
 
 	buf = buf[:0]
-	buf = append(buf, socks5Version, socks5Connect, 0 /* reserved */ )
+	buf = append(buf, socks5Version, socks5Connect, 0 /* reserved */)
 
 	if ip := net.ParseIP(host); ip != nil {
 		if len(ip) == 4 {
diff --git a/src/pkg/exp/terminal/terminal.go b/src/pkg/exp/terminal/terminal.go
index c3ba5bde2e..c1ed0c0c44 100644
--- a/src/pkg/exp/terminal/terminal.go
+++ b/src/pkg/exp/terminal/terminal.go
@@ -389,12 +389,12 @@ func (t *Terminal) Write(buf []byte) (n int, err error) {
 
 	// We have a prompt and possibly user input on the screen. We
 	// have to clear it first.
-	t.move(0, /* up */ 0, /* down */ t.cursorX, /* left */ 0 /* right */ )
+	t.move(0 /* up */, 0 /* down */, t.cursorX /* left */, 0 /* right */)
 	t.cursorX = 0
 	t.clearLineToRight()
 
 	for t.cursorY > 0 {
-		t.move(1, /* up */ 0, 0, 0)
+		t.move(1 /* up */, 0, 0, 0)
 		t.cursorY--
 		t.clearLineToRight()
 	}
diff --git a/src/pkg/go/printer/nodes.go b/src/pkg/go/printer/nodes.go
index 9ac678f5c1..cd5e075c16 100644
--- a/src/pkg/go/printer/nodes.go
+++ b/src/pkg/go/printer/nodes.go
@@ -132,7 +132,9 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 		for i, x := range list {
 			if i > 0 {
 				if mode&commaSep != 0 {
-					p.print(token.COMMA)
+					// use position of expression following the comma as
+					// comma position for correct comment placement
+					p.print(x.Pos(), token.COMMA)
 				}
 				p.print(blank)
 			}
@@ -212,11 +214,18 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 		}
 
 		if i > 0 {
+			needsLinebreak := prevLine < line && prevLine > 0 && line > 0
 			if mode&commaSep != 0 {
+				// use position of expression following the comma as
+				// comma position for correct comment placement, but
+				// only if the expression is on the same line
+				if !needsLinebreak {
+					p.print(x.Pos())
+				}
 				p.print(token.COMMA)
 			}
 			needsBlank := true
-			if prevLine < line && prevLine > 0 && line > 0 {
+			if needsLinebreak {
 				// lines are broken using newlines so comments remain aligned
 				// unless forceFF is set or there are multiple expressions on
 				// the same line in which case formfeed is used
@@ -283,11 +292,18 @@ func (p *printer) parameters(fields *ast.FieldList, multiLine *bool) {
 				parLineBeg = parLineEnd
 			}
 			// separating "," if needed
+			needsLinebreak := 0 < prevLine && prevLine < parLineBeg
 			if i > 0 {
+				// use position of parameter following the comma as
+				// comma position for correct comma placement, but
+				// only if the next parameter is on the same line
+				if !needsLinebreak {
+					p.print(par.Pos())
+				}
 				p.print(token.COMMA)
 			}
 			// separator if needed (linebreak or blank)
-			if 0 < prevLine && prevLine < parLineBeg && p.linebreak(parLineBeg, 0, ws, true) {
+			if needsLinebreak && p.linebreak(parLineBeg, 0, ws, true) {
 				// break line if the opening "(" or previous parameter ended on a different line
 				ws = ignore
 				*multiLine = true
@@ -312,7 +328,7 @@ func (p *printer) parameters(fields *ast.FieldList, multiLine *bool) {
 		// if the closing ")" is on a separate line from the last parameter,
 		// print an additional "," and line break
 		if closing := p.lineFor(fields.Closing); 0 < prevLine && prevLine < closing {
-			p.print(",")
+			p.print(token.COMMA)
 			p.linebreak(closing, 0, ignore, true)
 		}
 		// unindent if we indented
@@ -393,6 +409,7 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool)
 			f := list[0]
 			for i, x := range f.Names {
 				if i > 0 {
+					// no comments so no need for comma position
 					p.print(token.COMMA, blank)
 				}
 				p.expr(x, ignoreMultiLine)
@@ -1125,7 +1142,9 @@ func (p *printer) stmt(stmt ast.Stmt, nextIsRBrace bool, multiLine *bool) {
 		p.print(token.FOR, blank)
 		p.expr(s.Key, multiLine)
 		if s.Value != nil {
-			p.print(token.COMMA, blank)
+			// use position of value following the comma as
+			// comma position for correct comment placement
+			p.print(s.Value.Pos(), token.COMMA, blank)
 			p.expr(s.Value, multiLine)
 		}
 		p.print(blank, s.TokPos, s.Tok, blank, token.RANGE, blank)
diff --git a/src/pkg/go/printer/printer.go b/src/pkg/go/printer/printer.go
index c9949205e8..72f65a1d85 100644
--- a/src/pkg/go/printer/printer.go
+++ b/src/pkg/go/printer/printer.go
@@ -686,9 +686,11 @@ func (p *printer) intersperseComments(next token.Position, tok token.Token) (wro
 	}
 
 	if last != nil {
-		if last.Text[1] == '*' && p.lineFor(last.Pos()) == next.Line {
-			// the last comment is a /*-style comment and the next item
-			// follows on the same line: separate with an extra blank
+		// if the last comment is a /*-style comment and the next item
+		// follows on the same line but is not a comma or a "closing"
+		// token, add an extra blank for separation
+		if last.Text[1] == '*' && p.lineFor(last.Pos()) == next.Line && tok != token.COMMA &&
+			tok != token.RPAREN && tok != token.RBRACK && tok != token.RBRACE {
 			p.writeByte(' ', 1)
 		}
 		// ensure that there is a line break after a //-style comment,
diff --git a/src/pkg/go/printer/testdata/comments.golden b/src/pkg/go/printer/testdata/comments.golden
index e5826eecef..4c6f1ab827 100644
--- a/src/pkg/go/printer/testdata/comments.golden
+++ b/src/pkg/go/printer/testdata/comments.golden
@@ -405,16 +405,17 @@ func _() {
 }
 
 // Some interesting interspersed comments.
+// See below for more common cases.
 func _( /* this */ x /* is */ /* an */ int) {
 }
 
-func _( /* no params */ )	{}
+func _( /* no params */)	{}
 
 func _() {
-	f( /* no args */ )
+	f( /* no args */)
 }
 
-func ( /* comment1 */ T /* comment2 */ ) _()	{}
+func ( /* comment1 */ T /* comment2 */) _()	{}
 
 func _() { /* one-line functions with comments are formatted as multi-line functions */
 }
@@ -425,7 +426,7 @@ func _() {
 }
 
 func _() {
-	_ = []int{0, 1 /* don't introduce a newline after this comment - was issue 1365 */ }
+	_ = []int{0, 1 /* don't introduce a newline after this comment - was issue 1365 */}
 }
 
 // Test cases from issue 1542:
@@ -448,8 +449,9 @@ func _() {
 	_ = a
 }
 
-// Comments immediately adjacent to punctuation (for which the go/printer
-// may only have estimated position information) must remain after the punctuation.
+// Comments immediately adjacent to punctuation followed by a newline
+// remain after the punctuation (looks better and permits alignment of
+// comments).
 func _() {
 	_ = T{
 		1,	// comment after comma
@@ -479,6 +481,35 @@ func _() {
 	}
 }
 
+// If there is no newline following punctuation, commas move before the punctuation.
+// This way, commas interspersed in lists stay with the respective expression.
+func f(x /* comment */, y int, z int /* comment */, u, v, w int /* comment */) {
+	f(x /* comment */, y)
+	f(x,	/* comment */
+		y)
+	f(
+		x,	/* comment */
+	)
+}
+
+func g(
+	x int,	/* comment */
+) {
+}
+
+type _ struct {
+	a, b /* comment */, c int
+}
+
+type _ struct {
+	a, b /* comment */, c int
+}
+
+func _() {
+	for a /* comment */, b := range x {
+	}
+}
+
 // Print line directives correctly.
 
 // The following is a legal line directive.
diff --git a/src/pkg/go/printer/testdata/comments.input b/src/pkg/go/printer/testdata/comments.input
index 55f6b61f21..c0f8cca3a9 100644
--- a/src/pkg/go/printer/testdata/comments.input
+++ b/src/pkg/go/printer/testdata/comments.input
@@ -411,6 +411,7 @@ func _() {
 
 
 // Some interesting interspersed comments.
+// See below for more common cases.
 func _(/* this */x/* is *//* an */ int) {
 }
 
@@ -453,8 +454,9 @@ func _() {
 	_ = a
 }
 
-// Comments immediately adjacent to punctuation (for which the go/printer
-// may only have estimated position information) must remain after the punctuation.
+// Comments immediately adjacent to punctuation followed by a newline
+// remain after the punctuation (looks better and permits alignment of
+// comments).
 func _() {
 	_ = T{
 		1,    // comment after comma
@@ -486,6 +488,31 @@ func _() {
 	}
 }
 
+// If there is no newline following punctuation, commas move before the punctuation.
+// This way, commas interspersed in lists stay with the respective expression.
+func f(x/* comment */, y int, z int /* comment */, u, v, w int /* comment */) {
+	f(x /* comment */, y)
+	f(x /* comment */, 
+	y)
+	f(
+		x /* comment */,
+	)
+}
+
+func g(
+	x int /* comment */,
+) {}
+
+type _ struct {
+	a, b /* comment */, c int
+}
+
+type _ struct { a, b /* comment */, c int }
+
+func _() {
+	for a /* comment */, b := range x {
+	}
+}
 
 // Print line directives correctly.
 
diff --git a/src/pkg/go/scanner/example_test.go b/src/pkg/go/scanner/example_test.go
index 850e536060..9004a4ad33 100644
--- a/src/pkg/go/scanner/example_test.go
+++ b/src/pkg/go/scanner/example_test.go
@@ -18,7 +18,7 @@ func ExampleScanner_Scan() {
 	var s scanner.Scanner
 	fset := token.NewFileSet()                      // positions are relative to fset
 	file := fset.AddFile("", fset.Base(), len(src)) // register input "file"
-	s.Init(file, src, /* no error handler: */ nil, scanner.ScanComments)
+	s.Init(file, src, nil /* no error handler */, scanner.ScanComments)
 
 	// Repeated calls to Scan yield the token sequence found in the input.
 	for {