From 1517d1a3ba600175b642a4ae8106f0484516d3d9 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Fri, 11 Aug 2023 19:35:44 -0400 Subject: [PATCH] gopls/internal/lsp/source: fix renaming instantiated fields Correctly associate instantiated functions and fields with their origin during renaming. Fixes golang/go#61640 Change-Id: I819ffe303a2b1c35810d5b3c2d71fa5f4231a0c4 Reviewed-on: https://go-review.googlesource.com/c/tools/+/518897 TryBot-Result: Gopher Robot Run-TryBot: Robert Findley Reviewed-by: Hyang-Ah Hana Kim gopls-CI: kokoro --- gopls/internal/lsp/source/origin.go | 26 ++++++++++ gopls/internal/lsp/source/origin_119.go | 33 +++++++++++++ gopls/internal/lsp/source/references.go | 13 +---- gopls/internal/lsp/source/rename.go | 8 +--- .../marker/testdata/rename/issue61640.txt | 47 +++++++++++++++++++ 5 files changed, 109 insertions(+), 18 deletions(-) create mode 100644 gopls/internal/lsp/source/origin.go create mode 100644 gopls/internal/lsp/source/origin_119.go create mode 100644 gopls/internal/regtest/marker/testdata/rename/issue61640.txt diff --git a/gopls/internal/lsp/source/origin.go b/gopls/internal/lsp/source/origin.go new file mode 100644 index 00000000000..8ee467e844e --- /dev/null +++ b/gopls/internal/lsp/source/origin.go @@ -0,0 +1,26 @@ +// Copyright 2023 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. + +//go:build !go1.19 +// +build !go1.19 + +package source + +import "go/types" + +// containsOrigin reports whether the provided object set contains an object +// with the same origin as the provided obj (which may be a synthetic object +// created during instantiation). +func containsOrigin(objSet map[types.Object]bool, obj types.Object) bool { + if obj == nil { + return objSet[obj] + } + // In Go 1.18, we can't use the types.Var.Origin and types.Func.Origin methods. + for target := range objSet { + if target.Pkg() == obj.Pkg() && target.Pos() == obj.Pos() && target.Name() == obj.Name() { + return true + } + } + return false +} diff --git a/gopls/internal/lsp/source/origin_119.go b/gopls/internal/lsp/source/origin_119.go new file mode 100644 index 00000000000..a249ce4b1c5 --- /dev/null +++ b/gopls/internal/lsp/source/origin_119.go @@ -0,0 +1,33 @@ +// Copyright 2023 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. + +//go:build go1.19 +// +build go1.19 + +package source + +import "go/types" + +// containsOrigin reports whether the provided object set contains an object +// with the same origin as the provided obj (which may be a synthetic object +// created during instantiation). +func containsOrigin(objSet map[types.Object]bool, obj types.Object) bool { + objOrigin := origin(obj) + for target := range objSet { + if origin(target) == objOrigin { + return true + } + } + return false +} + +func origin(obj types.Object) types.Object { + switch obj := obj.(type) { + case *types.Var: + return obj.Origin() + case *types.Func: + return obj.Origin() + } + return obj +} diff --git a/gopls/internal/lsp/source/references.go b/gopls/internal/lsp/source/references.go index 3d923e44702..46459dcbec4 100644 --- a/gopls/internal/lsp/source/references.go +++ b/gopls/internal/lsp/source/references.go @@ -580,10 +580,8 @@ func localReferences(pkg Package, targets map[types.Object]bool, correspond bool // matches reports whether obj either is or corresponds to a target. // (Correspondence is defined as usual for interface methods.) matches := func(obj types.Object) bool { - for target := range targets { - if equalOrigin(obj, target) { - return true - } + if containsOrigin(targets, obj) { + return true } if methodRecvs != nil && obj.Name() == methodName { if orecv := effectiveReceiver(obj); orecv != nil { @@ -611,13 +609,6 @@ func localReferences(pkg Package, targets map[types.Object]bool, correspond bool return nil } -// equalOrigin reports whether obj1 and obj2 have equivalent origin object. -// This may be the case even if obj1 != obj2, if one or both of them is -// instantiated. -func equalOrigin(obj1, obj2 types.Object) bool { - return obj1.Pkg() == obj2.Pkg() && obj1.Pos() == obj2.Pos() && obj1.Name() == obj2.Name() -} - // effectiveReceiver returns the effective receiver type for method-set // comparisons for obj, if it is a method, or nil otherwise. func effectiveReceiver(obj types.Object) types.Type { diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go index c1db0e5fd5d..eb1fdce622f 100644 --- a/gopls/internal/lsp/source/rename.go +++ b/gopls/internal/lsp/source/rename.go @@ -1054,13 +1054,7 @@ func (r *renamer) update() (map[span.URI][]diff.Edit, error) { // shouldUpdate reports whether obj is one of (or an // instantiation of one of) the target objects. shouldUpdate := func(obj types.Object) bool { - if r.objsToUpdate[obj] { - return true - } - if fn, ok := obj.(*types.Func); ok && r.objsToUpdate[funcOrigin(fn)] { - return true - } - return false + return containsOrigin(r.objsToUpdate, obj) } // Find all identifiers in the package that define or use a diff --git a/gopls/internal/regtest/marker/testdata/rename/issue61640.txt b/gopls/internal/regtest/marker/testdata/rename/issue61640.txt new file mode 100644 index 00000000000..91c2b76933d --- /dev/null +++ b/gopls/internal/regtest/marker/testdata/rename/issue61640.txt @@ -0,0 +1,47 @@ +This test verifies that gopls can rename instantiated fields. + +-- flags -- +-min_go=go1.18 + +-- a.go -- +package a + +// This file is adapted from the example in the issue. + +type builder[S ~[]int] struct { + elements S //@rename("elements", elements2, OneToTwo) +} + +type BuilderImpl[S ~[]int] struct{ builder[S] } + +func NewBuilderImpl[S ~[]int](name string) *BuilderImpl[S] { + impl := &BuilderImpl[S]{ + builder[S]{ + elements: S{}, + }, + } + + _ = impl.elements + return impl +} +-- @OneToTwo/a.go -- +package a + +// This file is adapted from the example in the issue. + +type builder[S ~[]int] struct { + elements2 S //@rename("elements", elements2, OneToTwo) +} + +type BuilderImpl[S ~[]int] struct{ builder[S] } + +func NewBuilderImpl[S ~[]int](name string) *BuilderImpl[S] { + impl := &BuilderImpl[S]{ + builder[S]{ + elements2: S{}, + }, + } + + _ = impl.elements2 + return impl +}