Skip to content

Commit

Permalink
cmd/gofmt, go/format, go/printer: move number normalization to printer
Browse files Browse the repository at this point in the history
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 #37476.
For #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>
  • Loading branch information
dmitshur committed May 1, 2020
1 parent cb00d93 commit 5c8715f
Show file tree
Hide file tree
Showing 10 changed files with 672 additions and 58 deletions.
58 changes: 2 additions & 56 deletions src/cmd/gofmt/gofmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
8 changes: 7 additions & 1 deletion src/go/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
38 changes: 38 additions & 0 deletions src/go/format/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package format

import (
"bytes"
"go/ast"
"go/parser"
"go/token"
"io/ioutil"
Expand Down Expand Up @@ -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 {
Expand Down
59 changes: 59 additions & 0 deletions src/go/printer/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/go/printer/performance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
1 change: 1 addition & 0 deletions src/go/printer/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions src/go/printer/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type checkMode uint
const (
export checkMode = 1 << iota
rawFormat
stdFormat
idempotent
)

Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 5c8715f

Please sign in to comment.