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

@@ -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"},

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 == "_" {
+		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)
 }

src/cmd/gofmt/testdata/slices1.golden

+// 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
+}

src/cmd/gofmt/testdata/slices1.input

+// 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
+}

src/cmd/gofmt/testdata/slices2.golden

+// 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
+}

src/cmd/gofmt/testdata/slices2.input

+// 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
+}