go/printer, gofmt: fix alignment of "=" in const/var declarations

gofmt -w src misc

Fixes #1414.

R=rsc, r
CC=golang-dev
https://golang.org/cl/4456054

misc/cgo/test/env.go

@@ -30,5 +30,3 @@ func testSetEnv(t *testing.T) {
 		t.Fatalf("getenv() = %q; want %q", vs, val)
 	}
 }
-
-

src/pkg/go/printer/nodes.go

@@ -1189,6 +1189,97 @@ func (p *printer) stmt(stmt ast.Stmt, nextIsRBrace bool, multiLine *bool) {
 // ----------------------------------------------------------------------------
 // Declarations
 
+// The keepTypeColumn function determines if the type column of a series of
+// consecutive const or var declarations must be kept, or if initialization
+// values (V) can be placed in the type column (T) instead. The i'th entry
+// in the result slice is true if the type column in spec[i] must be kept.
+//
+// For example, the declaration:
+//
+//	const (
+//		foobar int = 42 // comment
+//		x          = 7  // comment
+//		foo
+//              bar = 991
+//	)
+//
+// leads to the type/values matrix below. A run of value columns (V) can
+// be moved into the type column if there is no type for any of the values
+// in that column (we only move entire columns so that they align properly).
+//
+//	matrix        formatted     result
+//                    matrix
+//	T  V    ->    T  V     ->   true      there is a T and so the type
+//	-  V          -  V          true      column must be kept
+//	-  -          -  -          false
+//	-  V          V  -          false     V is moved into T column
+//
+func keepTypeColumn(specs []ast.Spec) []bool {
+	m := make([]bool, len(specs))
+
+	populate := func(i, j int, keepType bool) {
+		if keepType {
+			for ; i < j; i++ {
+				m[i] = true
+			}
+		}
+	}
+
+	i0 := -1 // if i0 >= 0 we are in a run and i0 is the start of the run
+	var keepType bool
+	for i, s := range specs {
+		t := s.(*ast.ValueSpec)
+		if t.Values != nil {
+			if i0 < 0 {
+				// start of a run of ValueSpecs with non-nil Values
+				i0 = i
+				keepType = false
+			}
+		} else {
+			if i0 >= 0 {
+				// end of a run
+				populate(i0, i, keepType)
+				i0 = -1
+			}
+		}
+		if t.Type != nil {
+			keepType = true
+		}
+	}
+	if i0 >= 0 {
+		// end of a run
+		populate(i0, len(specs), keepType)
+	}
+
+	return m
+}
+
+
+func (p *printer) valueSpec(s *ast.ValueSpec, keepType, doIndent bool, multiLine *bool) {
+	p.setComment(s.Doc)
+	p.identList(s.Names, doIndent, multiLine) // always present
+	extraTabs := 3
+	if s.Type != nil || keepType {
+		p.print(vtab)
+		extraTabs--
+	}
+	if s.Type != nil {
+		p.expr(s.Type, multiLine)
+	}
+	if s.Values != nil {
+		p.print(vtab, token.ASSIGN)
+		p.exprList(token.NoPos, s.Values, 1, blankStart|commaSep, multiLine, token.NoPos)
+		extraTabs--
+	}
+	if s.Comment != nil {
+		for ; extraTabs > 0; extraTabs-- {
+			p.print(vtab)
+		}
+		p.setComment(s.Comment)
+	}
+}
+
+
 // The parameter n is the number of specs in the group. If doIndent is set,
 // multi-line identifier lists in the spec are indented when the first
 // linebreak is encountered.
@@ -1206,9 +1297,11 @@ func (p *printer) spec(spec ast.Spec, n int, doIndent bool, multiLine *bool) {
 		p.setComment(s.Comment)
 
 	case *ast.ValueSpec:
+		if n != 1 {
+			p.internalError("expected n = 1; got", n)
+		}
 		p.setComment(s.Doc)
 		p.identList(s.Names, doIndent, multiLine) // always present
-		if n == 1 {
 		if s.Type != nil {
 			p.print(blank)
 			p.expr(s.Type, multiLine)
@@ -1219,26 +1312,6 @@ func (p *printer) spec(spec ast.Spec, n int, doIndent bool, multiLine *bool) {
 		}
 		p.setComment(s.Comment)
 
-		} else {
-			extraTabs := 3
-			if s.Type != nil {
-				p.print(vtab)
-				p.expr(s.Type, multiLine)
-				extraTabs--
-			}
-			if s.Values != nil {
-				p.print(vtab, token.ASSIGN)
-				p.exprList(token.NoPos, s.Values, 1, blankStart|commaSep, multiLine, token.NoPos)
-				extraTabs--
-			}
-			if s.Comment != nil {
-				for ; extraTabs > 0; extraTabs-- {
-					p.print(vtab)
-				}
-				p.setComment(s.Comment)
-			}
-		}
-
 	case *ast.TypeSpec:
 		p.setComment(s.Doc)
 		p.expr(s.Name, multiLine)
@@ -1264,15 +1337,29 @@ func (p *printer) genDecl(d *ast.GenDecl, multiLine *bool) {
 	if d.Lparen.IsValid() {
 		// group of parenthesized declarations
 		p.print(d.Lparen, token.LPAREN)
-		if len(d.Specs) > 0 {
+		if n := len(d.Specs); n > 0 {
 			p.print(indent, formfeed)
+			if n > 1 && (d.Tok == token.CONST || d.Tok == token.VAR) {
+				// two or more grouped const/var declarations:
+				// determine if the type column must be kept
+				keepType := keepTypeColumn(d.Specs)
 				var ml bool
 				for i, s := range d.Specs {
 					if i > 0 {
 						p.linebreak(p.fset.Position(s.Pos()).Line, 1, ignore, ml)
 					}
 					ml = false
-				p.spec(s, len(d.Specs), false, &ml)
+					p.valueSpec(s.(*ast.ValueSpec), keepType[i], false, &ml)
+				}
+			} else {
+				var ml bool
+				for i, s := range d.Specs {
+					if i > 0 {
+						p.linebreak(p.fset.Position(s.Pos()).Line, 1, ignore, ml)
+					}
+					ml = false
+					p.spec(s, n, false, &ml)
+				}
 			}
 			p.print(unindent, formfeed)
 			*multiLine = true

src/pkg/go/printer/testdata/declarations.golden

@@ -160,7 +160,6 @@ bar`
 
 
 func _() {
-	// the following decls need a semicolon at the end
 	type _ int
 	type _ *int
 	type _ []int
@@ -175,7 +174,6 @@ func _() {
 	var _ chan int
 	var _ func() int
 
-	// the following decls don't need a semicolon at the end
 	type _ struct{}
 	type _ *struct{}
 	type _ []struct{}
@@ -401,6 +399,33 @@ func _() {
 	)
 }
 
+// alignment of "=" in consecutive lines (extended example from issue 1414)
+const (
+	umax	uint	= ^uint(0)		// maximum value for a uint
+	bpu		= 1 << (5 + umax>>63)	// bits per uint
+	foo
+	bar	= -1
+)
+
+// typical enum
+const (
+	a	MyType	= iota
+	abcd
+	b
+	c
+	def
+)
+
+// excerpt from godoc.go
+var (
+	goroot		= flag.String("goroot", runtime.GOROOT(), "Go root directory")
+	testDir		= flag.String("testdir", "", "Go root subdirectory - for testing only (faster startups)")
+	pkgPath		= flag.String("path", "", "additional package directories (colon-separated)")
+	filter		= flag.String("filter", "", "filter file containing permitted package directory paths")
+	filterMin	= flag.Int("filter_minutes", 0, "filter file update interval in minutes; disabled if <= 0")
+	filterDelay	delayTime	// actual filter update interval in minutes; usually filterDelay == filterMin, but filterDelay may back off exponentially
+)
+
 
 // formatting of structs
 type _ struct{}

src/pkg/go/printer/testdata/declarations.input

@@ -159,7 +159,6 @@ bar`
 
 
 func _() {
-	// the following decls need a semicolon at the end
 	type _ int
 	type _ *int
 	type _ []int
@@ -174,7 +173,6 @@ func _() {
 	var _ chan int
 	var _ func() int
 
-	// the following decls don't need a semicolon at the end
 	type _ struct{}
 	type _ *struct{}
 	type _ []struct{}
@@ -400,6 +398,33 @@ func _() {
 	)
 }
 
+// alignment of "=" in consecutive lines (extended example from issue 1414)
+const (
+	umax uint                  = ^uint(0) // maximum value for a uint
+	bpu  = 1 << (5 + umax>>63)            // bits per uint
+	foo
+	bar  = -1
+)
+
+// typical enum
+const (
+	a MyType = iota
+	abcd
+	b
+	c
+	def
+)
+
+// excerpt from godoc.go
+var (
+	goroot = flag.String("goroot", runtime.GOROOT(), "Go root directory")
+	testDir = flag.String("testdir", "", "Go root subdirectory - for testing only (faster startups)")
+	pkgPath = flag.String("path", "", "additional package directories (colon-separated)")
+	filter = flag.String("filter", "", "filter file containing permitted package directory paths")
+	filterMin = flag.Int("filter_minutes", 0, "filter file update interval in minutes; disabled if <= 0")
+	filterDelay delayTime // actual filter update interval in minutes; usually filterDelay == filterMin, but filterDelay may back off exponentially
+)
+
 
 // formatting of structs
 type _ struct{}