Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: rename: handle generic methods more precisely #58462

Open
adonovan opened this issue Feb 10, 2023 · 1 comment
Open

x/tools/gopls: rename: handle generic methods more precisely #58462

adonovan opened this issue Feb 10, 2023 · 1 comment
Labels
gopls/generics Issues related to gopls' support for generics gopls Issues related to the Go language server, gopls. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

gopls's renaming is tricky in the presence of generic methods, and our support for them is rudimentary. The challenge is in trying to decide which interfaces and methods correspond (and must be renamed together) when type parameters are involved. Recent changes to the renaming implementation for incrementalized type checking have not made things worse but have made us more aware of the problems and the paucity of testing generally. We should try to come up with a more comprehensive theory of how to do it. This isn't a regression so it needn't happen for v0.12.0.

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Feb 10, 2023
@gopherbot gopherbot added this to the Unreleased milestone Feb 10, 2023
@gopherbot
Copy link

Change https://go.dev/cl/464902 mentions this issue: gopls/internal/lsp/source/rename: incremental renaming

gopherbot pushed a commit to golang/tools that referenced this issue Feb 14, 2023
This change is a reimplementation of the renaming operation so
that it no longer mixes types.Objects from different packages.
The basic approach is as follows.

First, the referenced object is found and classified. If it is
nonexported (e.g. lowercase, or inherently local such as an
import or label, or within a function body), the operation is
local to that package and is carried out essentially as before.

However, if it is exported, then the scope of the global search
is deduced (direct for package-level var/func/const/type,
transitive for fields and methods). The object is converted to an
objectpath, and all the reverse dependencies are analyzed, using
the objectpath to identify the target in each package.

The renameObject function (the entry point for the fiddly renamer
algorithm) is now called once per package, and the results of all
calls are combined.

Because we process variants, duplicate edits are likely. We sort
and de-dup at the very end under the assumption that edits are
well behaved. The "seen edit" tracking in package renaming is no
longer needed.

Also:
- Package.importMap maps PackagePath to Package for all dependencies,
  so that we can resolve targets identified by (PackagePath,
  objectpath) to their objects in a different types.Importer "realm".
  It is materialized as a DAG of k/v pairs and exposed as
  Package.DependencyTypes.
- The rename_check algorithm (renamer) is now applied once to each
  package instead of once to all packages.
- The set of references to update is derived from types.Info, not the
  references operation.

Still to do in follow-ups:
- Method renaming needs to expand the set of renamed types (based on
  'satisfy') and recompute the dependencies of their declarations,
  until a fixed point is reached. (Not supporting this case is a
  functional regression since v0.11.0, but we plan to submit this
  change to unblock foundational progress and then fix it before the
  next release. See golang/go#58461)
- Lots of generics cases to consider (see golang/go#58462).
- Lots more tests required. For golang/go#57987.

Change-Id: I5fd8538ab35d61744d765d8bd101cd4efa41bd33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/464902
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@jamalc jamalc modified the milestones: Unreleased, gopls/v0.13.0 Feb 16, 2023
@adonovan adonovan added Refactoring Issues related to refactoring tools gopls/generics Issues related to gopls' support for generics labels Aug 31, 2023
@adonovan adonovan modified the milestones: gopls/v0.14.0, gopls/v0.15.0 Aug 31, 2023
@findleyr findleyr modified the milestones: gopls/v0.15.0, gopls/v0.16.0 Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/generics Issues related to gopls' support for generics gopls Issues related to the Go language server, gopls. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants