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

diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go
index e4d5796f3c8f459ba7ac0594248498783f588895..1f19d64eeef0122ccf61f0a66049ef3a4038a289 100644 (file)
--- 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 470c00625bbdbe1a99101638c1927460ff640da0..e9a67a73ac7ae4673b4880185f0b068f936b1d7a 100644 (file)
--- 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 (file)
index 0000000..61e074f
--- /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 (file)
index 0000000..4d2cbff
--- /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 (file)
index 0000000..433788e
--- /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 (file)
index 0000000..433788e
--- /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
+}