tree d7cfeaa78c1d07658e3c35d67d62bd9147776a5c
parent 514964b7556a3790e5e2d5e83ef771cc469f99bc
author Rob Findley <rfindley@google.com> 1610743582 -0500
committer Robert Findley <rfindley@google.com> 1611669166 +0000

internal/lsp/source: make it an error to rename embedded fields

Field embedding links two objects (a TypeName and a Var) by name,
requiring special handling during renaming. In CL 282932, renaming of
types was made to propagate to uses of their embeddings. However, no
such propagation in the reverse direction was added, meaning that
renaming an embedded field would not rename the corresponding type, and
code could still be left in a non-compiling state.

It should be an invariant that renaming does not change program
behavior. To enforce with field embeddings this we'd need to also rename
the corresponding type, but this seems problematic. If I'm hovering over
the field selector x.T, and rename T, it is surprising that this would
end up renaming a type.

For lack of a better solution, make it an error to rename embedded
fields, but try to provide a helpful error message.

Also handle the blank identifier, for which renaming was giving a
message to "please file a bug".

Marker tests are added for the new errors in rename, but not for
prepareRename. The prepareRename tests were not set up for asserting on
errors -- perhaps that would be a good project for a later CL where we
clean up errors.

Fixes golang/go#43616

Change-Id: I66c2dd5e531dd102431d1edd443d553687d9ca7e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284312
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>