gofmt: don't crash when rewriting nil interfaces in AST.

The new reflection API makes it an error to call value.Set(x)
if x is invalid. Guard for it.

Added corresponding test case.

Fixes #1696.

R=rsc, r
CC=golang-dev
https://golang.org/cl/4398047

src/cmd/gofmt/gofmt_test.go

@@ -71,6 +71,7 @@ var tests = []struct {
 	{".", "gofmt_test.go", "gofmt_test.go", ""},
 	{"testdata", "composites.input", "composites.golden", "-s"},
 	{"testdata", "rewrite1.input", "rewrite1.golden", "-r=Foo->Bar"},
+	{"testdata", "rewrite2.input", "rewrite2.golden", "-r=int->bool"},
 }
 
 

src/cmd/gofmt/rewrite.go

@@ -63,6 +63,10 @@ func rewriteFile(pattern, replace ast.Expr, p *ast.File) *ast.File {
 	repl := reflect.NewValue(replace)
 	var f func(val reflect.Value) reflect.Value // f is recursive
 	f = func(val reflect.Value) reflect.Value {
+		// don't bother if val is invalid to start with
+		if !val.IsValid() {
+			return reflect.Value{}
+		}
 		for k := range m {
 			m[k] = reflect.Value{}, false
 		}
@@ -79,6 +83,10 @@ func rewriteFile(pattern, replace ast.Expr, p *ast.File) *ast.File {
 // setValue is a wrapper for x.SetValue(y); it protects
 // the caller from panics if x cannot be changed to y.
 func setValue(x, y reflect.Value) {
+	// don't bother if y is invalid to start with
+	if !y.IsValid() {
+		return
+	}
 	defer func() {
 		if x := recover(); x != nil {
 			if s, ok := x.(string); ok && strings.HasPrefix(s, "type mismatch") {
@@ -95,10 +103,12 @@ func setValue(x, y reflect.Value) {
 // Values/types for special cases.
 var (
 	objectPtrNil = reflect.NewValue((*ast.Object)(nil))
+	scopePtrNil  = reflect.NewValue((*ast.Scope)(nil))
 
 	identType     = reflect.Typeof((*ast.Ident)(nil))
 	objectPtrType = reflect.Typeof((*ast.Object)(nil))
 	positionType  = reflect.Typeof(token.NoPos)
+	scopePtrType  = reflect.Typeof((*ast.Scope)(nil))
 )
 
 
@@ -115,6 +125,12 @@ func apply(f func(reflect.Value) reflect.Value, val reflect.Value) reflect.Value
 		return objectPtrNil
 	}
 
+	// similarly for scopes: they are likely incorrect after a rewrite;
+	// replace them with nil
+	if val.Type() == scopePtrType {
+		return scopePtrNil
+	}
+
 	switch v := reflect.Indirect(val); v.Kind() {
 	case reflect.Slice:
 		for i := 0; i < v.Len(); i++ {

src/cmd/gofmt/testdata/rewrite1.golden

@@ -1,3 +1,7 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
 package main
 
 type Bar int

src/cmd/gofmt/testdata/rewrite1.input

@@ -1,3 +1,7 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
 package main
 
 type Foo int

src/cmd/gofmt/testdata/rewrite2.golden

@@ -0,0 +1,10 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+// Slices have nil Len values in the corresponding ast.ArrayType
+// node and reflect.NewValue(slice.Len) is an invalid reflect.Value.
+// The rewriter must not crash in that case. Was issue 1696.
+func f() []bool {}

src/cmd/gofmt/testdata/rewrite2.input

@@ -0,0 +1,10 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+// Slices have nil Len values in the corresponding ast.ArrayType
+// node and reflect.NewValue(slice.Len) is an invalid reflect.Value.
+// The rewriter must not crash in that case. Was issue 1696.
+func f() []int {}