code review 5376061: gofmt: leave nil nodes of the AST unchanged.

gofmt: leave nil nodes of the AST unchanged.

Without this check, gofmt panics when trying to apply
the identity transformation on "item.field" expressions.
Fixes issue 2410.

[remyoudompheng, 2011-11-11 09:21:17]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
remy@archlinux.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-11-11 19:51:37]

Thanks for fixing this.

http://codereview.appspot.com/5376061/diff/3001/src/cmd/gofmt/rewrite.go
File src/cmd/gofmt/rewrite.go (right):

http://codereview.appspot.com/5376061/diff/3001/src/cmd/gofmt/rewrite.go#newc...
src/cmd/gofmt/rewrite.go:67: if (val.Kind() == reflect.Ptr || val.Kind() ==
reflect.Interface) && val.IsNil() {
I think this test would work better in apply, below.
It already has a switch on val.Kind.

[gri, 2011-11-11 19:54:11]

In addition to rsc' comments.
- gri

http://codereview.appspot.com/5376061/diff/3001/src/cmd/gofmt/testdata/rewrit...
File src/cmd/gofmt/testdata/rewrite3.input (right):

http://codereview.appspot.com/5376061/diff/3001/src/cmd/gofmt/testdata/rewrit...
src/cmd/gofmt/testdata/rewrite3.input:6: 
Can you please add a comment here explaining what this is testing and which
issue it is fixing? (see testdata/rewrite2.input for instance).

[remyoudompheng, 2011-11-11 21:56:23]

Hello rsc@golang.org, gri@golang.org (cc: golang-dev@googlegroups.com,
remy@archlinux.org),

Please take another look.

[rsc, 2011-11-11 21:58:20]

LGTM

Leaving for gri.

[gri, 2011-11-11 22:11:13]

LGTM.
Much better.
- gri

[gri, 2011-11-11 22:11:48]

*** Submitted as http://code.google.com/p/go/source/detail?r=b793c9ddf021 ***

gofmt: leave nil nodes of the AST unchanged.

Without this check, gofmt panics when trying to apply
the identity transformation on "item.field" expressions.
Fixes issue 2410.

R=rsc, gri
CC=golang-dev, remy
http://codereview.appspot.com/5376061

Committer: Robert Griesemer <gri@golang.org>