cmd/gofmt, go/format, go/printer: move number normalization to printer

Normalization of number prefixes and exponents was added in CL 160184
directly in cmd/gofmt. The same behavior change needs to be applied in
the go/format package. This is done by moving the normalization code
into go/printer, behind a new StdFormat mode, which is then re-used
by both cmd/gofmt and go/format.

Note that formatting of Go source code changes over time, so the exact
byte output produced by go/printer may change between versions of Go
when using StdFormat mode. What is guaranteed is that the new formatting
is equivalent Go code.

Clients looking to format Go code with standard formatting consistent
with cmd/gofmt and go/format would need to start using this flag, but
a better alternative is to use the go/format package instead.

Benchstat numbers on go test go/printer -bench=BenchmarkPrint:

	name     old time/op    new time/op    delta
	Print-8    4.56ms ± 1%    4.57ms ± 0%   ~     (p=0.700 n=3+3)

	name     old alloc/op   new alloc/op   delta
	Print-8     467kB ± 0%     467kB ± 0%   ~     (p=1.000 n=3+3)

	name     old allocs/op  new allocs/op  delta
	Print-8     17.2k ± 0%     17.2k ± 0%   ~     (all equal)

That benchmark data doesn't contain any numbers that need to be
normalized. More work needs to be performed when formatting Go code
with numbers, but it is unavoidable to produce standard formatting.

Fixes golang#37476.
For golang#37453.

Change-Id: If50bde4035c3ee6e6ff0ece5691f6d3566ffe8d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/231461
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

src/cmd/gofmt/gofmt.go

@@ -37,9 +37,10 @@ var (
 	cpuprofile = flag.String("cpuprofile", "", "write cpu profile to this file")
 )
 
+// Keep these in sync with go/format/format.go.
 const (
 	tabWidth    = 8
-	printerMode = printer.UseSpaces | printer.TabIndent
+	printerMode = printer.UseSpaces | printer.TabIndent | printer.StdFormat
 )
 
 var (
@@ -113,8 +114,6 @@ func processFile(filename string, in io.Reader, out io.Writer, stdin bool) error
 		simplify(file)
 	}
 
-	ast.Inspect(file, normalizeNumbers)
-
 	res, err := format(fileSet, file, sourceAdj, indentAdj, src, printer.Config{Mode: printerMode, Tabwidth: tabWidth})
 	if err != nil {
 		return err
@@ -294,56 +293,3 @@ func backupFile(filename string, data []byte, perm os.FileMode) (string, error)
 
 	return bakname, err
 }
-
-// normalizeNumbers rewrites base prefixes and exponents to
-// use lower-case letters, and removes leading 0's from
-// integer imaginary literals. It leaves hexadecimal digits
-// alone.
-func normalizeNumbers(n ast.Node) bool {
-	lit, _ := n.(*ast.BasicLit)
-	if lit == nil || (lit.Kind != token.INT && lit.Kind != token.FLOAT && lit.Kind != token.IMAG) {
-		return true
-	}
-	if len(lit.Value) < 2 {
-		return false // only one digit (common case) - nothing to do
-	}
-	// len(lit.Value) >= 2
-
-	// We ignore lit.Kind because for lit.Kind == token.IMAG the literal may be an integer
-	// or floating-point value, decimal or not. Instead, just consider the literal pattern.
-	x := lit.Value
-	switch x[:2] {
-	default:
-		// 0-prefix octal, decimal int, or float (possibly with 'i' suffix)
-		if i := strings.LastIndexByte(x, 'E'); i >= 0 {
-			x = x[:i] + "e" + x[i+1:]
-			break
-		}
-		// remove leading 0's from integer (but not floating-point) imaginary literals
-		if x[len(x)-1] == 'i' && strings.IndexByte(x, '.') < 0 && strings.IndexByte(x, 'e') < 0 {
-			x = strings.TrimLeft(x, "0_")
-			if x == "i" {
-				x = "0i"
-			}
-		}
-	case "0X":
-		x = "0x" + x[2:]
-		fallthrough
-	case "0x":
-		// possibly a hexadecimal float
-		if i := strings.LastIndexByte(x, 'P'); i >= 0 {
-			x = x[:i] + "p" + x[i+1:]
-		}
-	case "0O":
-		x = "0o" + x[2:]
-	case "0o":
-		// nothing to do
-	case "0B":
-		x = "0b" + x[2:]
-	case "0b":
-		// nothing to do
-	}
-
-	lit.Value = x
-	return false
-}

src/go/format/format.go

@@ -24,7 +24,13 @@ import (
 	"io"
 )
 
-var config = printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 8}
+// Keep these in sync with cmd/gofmt/gofmt.go.
+const (
+	tabWidth    = 8
+	printerMode = printer.UseSpaces | printer.TabIndent | printer.StdFormat
+)
+
+var config = printer.Config{Mode: printerMode, Tabwidth: tabWidth}
 
 const parserMode = parser.ParseComments
 

src/go/format/format_test.go

@@ -6,6 +6,7 @@ package format
 
 import (
 	"bytes"
+	"go/ast"
 	"go/parser"
 	"go/token"
 	"io/ioutil"
@@ -57,6 +58,43 @@ func TestNode(t *testing.T) {
 	diff(t, buf.Bytes(), src)
 }
 
+// Node is documented to not modify the AST. Test that it is so, even when
+// formatting changes are applied due to printer.StdFormat mode being used.
+func TestNodeNoModify(t *testing.T) {
+	const (
+		src    = "package p\n\nconst _ = 0000000123i\n"
+		golden = "package p\n\nconst _ = 123i\n"
+	)
+
+	fset := token.NewFileSet()
+	file, err := parser.ParseFile(fset, "", src, parser.ParseComments)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Capture original address and value of a BasicLit node
+	// which will undergo formatting changes during printing.
+	wantLit := file.Decls[0].(*ast.GenDecl).Specs[0].(*ast.ValueSpec).Values[0].(*ast.BasicLit)
+	wantVal := wantLit.Value
+
+	var buf bytes.Buffer
+	if err = Node(&buf, fset, file); err != nil {
+		t.Fatal("Node failed:", err)
+	}
+	diff(t, buf.Bytes(), []byte(golden))
+
+	// Check if anything changed after Node returned.
+	gotLit := file.Decls[0].(*ast.GenDecl).Specs[0].(*ast.ValueSpec).Values[0].(*ast.BasicLit)
+	gotVal := gotLit.Value
+
+	if gotLit != wantLit {
+		t.Errorf("got *ast.BasicLit address %p, want %p", gotLit, wantLit)
+	}
+	if gotVal != wantVal {
+		t.Errorf("got *ast.BasicLit value %q, want %q", gotVal, wantVal)
+	}
+}
+
 func TestSource(t *testing.T) {
 	src, err := ioutil.ReadFile(testfile)
 	if err != nil {

src/go/printer/nodes.go

@@ -791,6 +791,9 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) {
 		}
 
 	case *ast.BasicLit:
+		if p.Config.Mode&StdFormat != 0 {
+			x = normalizeNumbers(x)
+		}
 		p.print(x)
 
 	case *ast.FuncLit:
@@ -971,6 +974,62 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) {
 	}
 }
 
+// normalizeNumbers rewrites base prefixes and exponents to
+// use lower-case letters, and removes leading 0's from
+// integer imaginary literals. It leaves hexadecimal digits
+// alone.
+func normalizeNumbers(lit *ast.BasicLit) *ast.BasicLit {
+	if lit.Kind != token.INT && lit.Kind != token.FLOAT && lit.Kind != token.IMAG {
+		return lit // not a number - nothing to do
+	}
+	if len(lit.Value) < 2 {
+		return lit // only one digit (common case) - nothing to do
+	}
+	// len(lit.Value) >= 2
+
+	// We ignore lit.Kind because for lit.Kind == token.IMAG the literal may be an integer
+	// or floating-point value, decimal or not. Instead, just consider the literal pattern.
+	x := lit.Value
+	switch x[:2] {
+	default:
+		// 0-prefix octal, decimal int, or float (possibly with 'i' suffix)
+		if i := strings.LastIndexByte(x, 'E'); i >= 0 {
+			x = x[:i] + "e" + x[i+1:]
+			break
+		}
+		// remove leading 0's from integer (but not floating-point) imaginary literals
+		if x[len(x)-1] == 'i' && strings.IndexByte(x, '.') < 0 && strings.IndexByte(x, 'e') < 0 {
+			x = strings.TrimLeft(x, "0_")
+			if x == "i" {
+				x = "0i"
+			}
+		}
+	case "0X":
+		x = "0x" + x[2:]
+		// possibly a hexadecimal float
+		if i := strings.LastIndexByte(x, 'P'); i >= 0 {
+			x = x[:i] + "p" + x[i+1:]
+		}
+	case "0x":
+		// possibly a hexadecimal float
+		i := strings.LastIndexByte(x, 'P')
+		if i == -1 {
+			return lit // nothing to do
+		}
+		x = x[:i] + "p" + x[i+1:]
+	case "0O":
+		x = "0o" + x[2:]
+	case "0o":
+		return lit // nothing to do
+	case "0B":
+		x = "0b" + x[2:]
+	case "0b":
+		return lit // nothing to do
+	}
+
+	return &ast.BasicLit{ValuePos: lit.ValuePos, Kind: lit.Kind, Value: x}
+}
+
 func (p *printer) possibleSelectorExpr(expr ast.Expr, prec1, depth int) bool {
 	if x, ok := expr.(*ast.SelectorExpr); ok {
 		return p.selectorExpr(x, depth, true)

src/go/printer/performance_test.go

@@ -20,7 +20,7 @@ import (
 var testfile *ast.File
 
 func testprint(out io.Writer, file *ast.File) {
-	if err := (&Config{TabIndent | UseSpaces, 8, 0}).Fprint(out, fset, file); err != nil {
+	if err := (&Config{TabIndent | UseSpaces | StdFormat, 8, 0}).Fprint(out, fset, file); err != nil {
 		log.Fatalf("print error: %s", err)
 	}
 }

src/go/printer/printer.go

@@ -1276,6 +1276,7 @@ const (
 	TabIndent                  // use tabs for indentation independent of UseSpaces
 	UseSpaces                  // use spaces instead of tabs for alignment
 	SourcePos                  // emit //line directives to preserve original source positions
+	StdFormat                  // apply standard formatting changes (exact byte output may change between versions of Go)
 )
 
 // A Config node controls the output of Fprint.

src/go/printer/printer_test.go

@@ -33,6 +33,7 @@ type checkMode uint
 const (
 	export checkMode = 1 << iota
 	rawFormat
+	stdFormat
 	idempotent
 )
 
@@ -57,6 +58,9 @@ func format(src []byte, mode checkMode) ([]byte, error) {
 	if mode&rawFormat != 0 {
 		cfg.Mode |= RawFormat
 	}
+	if mode&stdFormat != 0 {
+		cfg.Mode |= StdFormat
+	}
 
 	// print AST
 	var buf bytes.Buffer
@@ -200,6 +204,8 @@ var data = []entry{
 	{"statements.input", "statements.golden", 0},
 	{"slow.input", "slow.golden", idempotent},
 	{"complit.input", "complit.x", export},
+	{"go2numbers.input", "go2numbers.golden", idempotent},
+	{"go2numbers.input", "go2numbers.stdfmt", stdFormat | idempotent},
 }
 
 func TestFiles(t *testing.T) {