From 0cbca268d8aba5d7ffb24be58ea1ea92fcd5f6f5 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 31 Oct 2012 11:48:55 -0700 Subject: [PATCH] gofmt: simplify slices of the form s[a : len(s)] to s[a:] Fixes #4314. R=r, rsc CC=golang-dev https://golang.org/cl/6822059 --- src/cmd/gofmt/gofmt_test.go | 2 + src/cmd/gofmt/simplify.go | 54 ++++++++++++++++++++---- src/cmd/gofmt/testdata/slices1.golden | 58 +++++++++++++++++++++++++ src/cmd/gofmt/testdata/slices1.input | 58 +++++++++++++++++++++++++ src/cmd/gofmt/testdata/slices2.golden | 61 +++++++++++++++++++++++++++ src/cmd/gofmt/testdata/slices2.input | 61 +++++++++++++++++++++++++++ 6 files changed, 285 insertions(+), 9 deletions(-) create mode 100644 src/cmd/gofmt/testdata/slices1.golden create mode 100644 src/cmd/gofmt/testdata/slices1.input create mode 100644 src/cmd/gofmt/testdata/slices2.golden create mode 100644 src/cmd/gofmt/testdata/slices2.input diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index e4d5796f3c..1f19d64eee 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -72,6 +72,8 @@ var tests = []struct { {"gofmt.go", ""}, {"gofmt_test.go", ""}, {"testdata/composites.input", "-s"}, + {"testdata/slices1.input", "-s"}, + {"testdata/slices2.input", "-s"}, {"testdata/old.input", ""}, {"testdata/rewrite1.input", "-r=Foo->Bar"}, {"testdata/rewrite2.input", "-r=int->bool"}, diff --git a/src/cmd/gofmt/simplify.go b/src/cmd/gofmt/simplify.go index 470c00625b..e9a67a73ac 100644 --- a/src/cmd/gofmt/simplify.go +++ b/src/cmd/gofmt/simplify.go @@ -10,7 +10,9 @@ import ( "reflect" ) -type simplifier struct{} +type simplifier struct { + hasDotImport bool // package file contains: import . "some/import/path" +} func (s *simplifier) Visit(node ast.Node) ast.Visitor { switch n := node.(type) { @@ -34,7 +36,7 @@ func (s *simplifier) Visit(node ast.Node) ast.Visitor { x = t.Value px = &t.Value } - simplify(x) + ast.Walk(s, x) // simplify x // if the element is a composite literal and its literal type // matches the outer literal's element type exactly, the inner // literal type may be omitted @@ -62,20 +64,54 @@ func (s *simplifier) Visit(node ast.Node) ast.Visitor { return nil } + case *ast.SliceExpr: + // a slice expression of the form: s[a:len(s)] + // can be simplified to: s[a:] + // if s is "simple enough" (for now we only accept identifiers) + if s.hasDotImport { + // if dot imports are present, we cannot be certain that an + // unresolved "len" identifier refers to the predefined len() + break + } + if s, _ := n.X.(*ast.Ident); s != nil && s.Obj != nil { + // the array/slice object is a single, resolved identifier + if call, _ := n.High.(*ast.CallExpr); call != nil && len(call.Args) == 1 && !call.Ellipsis.IsValid() { + // the high expression is a function call with a single argument + if fun, _ := call.Fun.(*ast.Ident); fun != nil && fun.Name == "len" && fun.Obj == nil { + // the function called is "len" and it is not locally defined; and + // because we don't have dot imports, it must be the predefined len() + if arg, _ := call.Args[0].(*ast.Ident); arg != nil && arg.Obj == s.Obj { + // the len argument is the array/slice object + n.High = nil + } + } + } + } + // Note: We could also simplify slice expressions of the form s[0:b] to s[:b] + // but we leave them as is since sometimes we want to be very explicit + // about the lower bound. + case *ast.RangeStmt: - // range of the form: for x, _ = range v {...} + // a range of the form: for x, _ = range v {...} // can be simplified to: for x = range v {...} - if n.Value != nil { - if ident, ok := n.Value.(*ast.Ident); ok && ident.Name == "_" { - n.Value = nil - } + if ident, _ := n.Value.(*ast.Ident); ident != nil && ident.Name == "_" { + n.Value = nil } } return s } -func simplify(node ast.Node) { +func simplify(f *ast.File) { var s simplifier - ast.Walk(&s, node) + + // determine if f contains dot imports + for _, imp := range f.Imports { + if imp.Name != nil && imp.Name.Name == "." { + s.hasDotImport = true + break + } + } + + ast.Walk(&s, f) } diff --git a/src/cmd/gofmt/testdata/slices1.golden b/src/cmd/gofmt/testdata/slices1.golden new file mode 100644 index 0000000000..61e074f68a --- /dev/null +++ b/src/cmd/gofmt/testdata/slices1.golden @@ -0,0 +1,58 @@ +// Test cases for slice expression simplification. +package p + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:] + _ = a[3:(len(a))] + _ = a[len(a) : len(a)-1] + _ = a[0:len(b)] + + _ = a[:] + _ = a[:10] + _ = a[:] + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(b)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:] + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + + _ = s[:] + _ = s[:10] + _ = s[:] + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] +) + +func _() { + s := s[0:] + _ = s +} diff --git a/src/cmd/gofmt/testdata/slices1.input b/src/cmd/gofmt/testdata/slices1.input new file mode 100644 index 0000000000..4d2cbfff40 --- /dev/null +++ b/src/cmd/gofmt/testdata/slices1.input @@ -0,0 +1,58 @@ +// Test cases for slice expression simplification. +package p + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:len(a)] + _ = a[3:(len(a))] + _ = a[len(a) : len(a)-1] + _ = a[0:len(b)] + + _ = a[:] + _ = a[:10] + _ = a[:len(a)] + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(b)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:len(s)] + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + + _ = s[:] + _ = s[:10] + _ = s[:len(s)] + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] +) + +func _() { + s := s[0:len(s)] + _ = s +} diff --git a/src/cmd/gofmt/testdata/slices2.golden b/src/cmd/gofmt/testdata/slices2.golden new file mode 100644 index 0000000000..433788e1ee --- /dev/null +++ b/src/cmd/gofmt/testdata/slices2.golden @@ -0,0 +1,61 @@ +// Test cases for slice expression simplification. +// Because of a dot import, these slices must remain untouched. +package p + +import . "math" + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:len(a)] + _ = a[3:(len(a))] + _ = a[len(a) : len(a)-1] + _ = a[0:len(b)] + + _ = a[:] + _ = a[:10] + _ = a[:len(a)] + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(b)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:len(s)] + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + + _ = s[:] + _ = s[:10] + _ = s[:len(s)] + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] +) + +func _() { + s := s[0:len(s)] + _ = s +} diff --git a/src/cmd/gofmt/testdata/slices2.input b/src/cmd/gofmt/testdata/slices2.input new file mode 100644 index 0000000000..433788e1ee --- /dev/null +++ b/src/cmd/gofmt/testdata/slices2.input @@ -0,0 +1,61 @@ +// Test cases for slice expression simplification. +// Because of a dot import, these slices must remain untouched. +package p + +import . "math" + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:len(a)] + _ = a[3:(len(a))] + _ = a[len(a) : len(a)-1] + _ = a[0:len(b)] + + _ = a[:] + _ = a[:10] + _ = a[:len(a)] + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(b)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:len(s)] + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + + _ = s[:] + _ = s[:10] + _ = s[:len(s)] + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] +) + +func _() { + s := s[0:len(s)] + _ = s +} -- 2.50.0