x/tools/gopls: renaming an imported package name does not rename the imported package itself

[Fumbibi]

Type: Bug

step 1, have a very simple application file app/mapper.go and a simple test file in the same package, but a different directory app_test/mapper_test.go
both have first line 'package app'

step 2, start fixing your file structure, create a new directory named 'mapper' and put mapper.go and mapper_test.go both in there.

step 3, change the package for mapper to 'package mapper' and mapper_test to 'package mapper_test'

step 4, right click the 'app' in rooms := &app.Generate()

step 5, left-click Rename Symbol

step 6, type in 'mapper' to match the package name properly

step 7, ctrl + s saves the file

Import list before step 7

import (
"data-algo/app"
"fmt"
"math/rand"
"testing"
"time"
)

after step 7

import (
mapper "data-algo/app"
"fmt"
"math/rand"
"testing"
"time"
)

I have many folders I'm fixing like this and I repeated this bug consistently

Extension version: 0.41.4
VS Code version: Code 1.90.1 (611f9bfce64f25108829dd295f54a6894e87339d, 2024-06-11T21:02:43.666Z)
OS version: Linux x64 6.6.25-01713-g2f237acc8e50
Modes:

System Info

Item	Value
CPUs	Intel(R) Celeron(R) N4500 @ 1.10GHz (2 x 0)
GPU Status	2d_canvas: enabled canvas_oop_rasterization: disabled_off direct_rendering_display_compositor: disabled_off_ok gpu_compositing: enabled multiple_raster_threads: disabled_off opengl: enabled_on rasterization: enabled raw_draw: disabled_off_ok skia_graphite: disabled_off video_decode: enabled video_encode: disabled_software vulkan: disabled_off webgl: enabled webgl2: enabled webgpu: disabled_off
Load (avg)	0, 1, 1
Memory (System)	2.68GB (1.94GB free)
Process Argv	. --crash-reporter-id 3a1bcf46-b7c7-4601-b24a-48d4ebbe4fd8
Screen Reader	no
VM	100%
DESKTOP_SESSION	undefined
XDG_CURRENT_DESKTOP	X-Generic
XDG_SESSION_DESKTOP	undefined
XDG_SESSION_TYPE	wayland

A/B Experiments

vsliv368:30146709
vspor879:30202332
vspor708:30202333
vspor363:30204092
vscorecescf:30445987
vscod805:30301674
binariesv615:30325510
vsaa593:30376534
py29gd2263:31024239
c4g48928:30535728
azure-dev_surveyone:30548225
a9j8j154:30646983
962ge761:30959799
pythongtdpath:30769146
welcomedialogc:30910334
pythonidxpt:30866567
pythonnoceb:30805159
asynctok:30898717
pythontestfixt:30902429
pythonregdiag2:30936856
pythonmypyd1:30879173
2e7ec940:31000449
pythontbext0:30879054
accentitlementst:30995554
dsvsc016:30899300
dsvsc017:30899301
dsvsc018:30899302
cppperfnew:31000557
dsvsc020:30976470
pythonait:31006305
jchc7451:31067544
chatpanelt:31048053
dsvsc021:30996838
g316j359:31013175
pythoncenvpt:31062603
a69g1124:31058053
dvdeprecation:31068756
pythonprt:31056678
dwnewjupyter:31046869
newcmakeconfigv2:31071590

[Fumbibi]

another odd behavior along the same line was, I updated world := &bonus.World{} in main to world := &worldroller.World{} by typing it manually, and ctrl +s removed the import line for the old bonus package and and didn't add the one for the worldroller package. But then hit undo, selected bonus, started typing, saw predicted text "world roller" hit tab, and that updated the import perfectly correctly even before I saved.

When doing this in a file with more than one instance, it kept the old 'bad' import and added the new one on a new line, which makes sense because it was still referenced in the code.

update, further testing showed that when you right click the directory name in the import list and you click "rename symbol" it will populate the box so like
import "data/ops" displays "ops" in the box, and you can type "dllist" to replace it, and it will replace all the references except it just aliases the import which doesn't fix update the line to import "data/dllist" like one might expect.

further investigation reveals that if you try to delete the import to prevent the mistake, then you can't use "rename symbol" on the code because it no longer recognizes it.

[adonovan]

There are three issues here. Let's call this one issue 1.

step 4, right click the 'app' in rooms := &app.Generate()

The symbol app is an imported package name, whose scope is just the current file. When you rename it, you are changing the name by which the package is imported into the current file, not the name of the package itself, so the rename tool is working as intended when it introduces a renaming import: import mapper "data-algo/app".

To rename a package, invoke the Rename operation while selecting the package app identifier at the top of the file.

[adonovan]

Issue 2:

another odd behavior along the same line

I tried to reproduce this using the encoding/xml and encoding/json packages, which both define NewEncoder:

• add var _ = xml.NewEncoder and save file. import "encoding/xml" appears.
• replace "xml." with "json." and save file. The import changes to "encoding/json".
• add var _ = xml. and start typing. Completions within xml package are observed, even though it is not imported.
• hit tab to accept completion. import "encoding/xml" appears. (We now have both imports.) This is normal.
• If instead of the previous two steps, we replace the existing var _ = json... with var _ = xml., then start typing NewEnc, accepting the completion adds an xml import but doesn't remove the json import that is no longer needed. This too is normal; redundant imports are not removed until the file is saved.

If you see something different, could you please give instructions for reproducing it with these standard library symbols? Ideally this should be a separate GitHub issue, since it is unrelated to the other two items.

[adonovan]

Issue 3:

update, further testing showed that when you right click the directory name...

Correct, renaming an import declaration such as "fmt" is equivalent to renaming the local package name such as fmt.Println. It changes only the current file, not the imported package. To rename a package, open a source file in that package and rename the package declaration on the first line. This issue is a different manifestation of issue 1. The tool is working as intended.

further investigation reveals that if you try to delete the import to prevent the mistake, then you can't use "rename symbol" on the code because it no longer recognizes it.

Indeed; to undo this mistake you should rename again to the original name.

[adonovan]

@findleyr wonders whether issue 2 may have been affected by https://go.dev/cl/590377.
Could you try again after installing today's v0.16 release of gopls?
go install golang.org/x/tools/gopls@latest

[Fumbibi]

Issue 2:

another odd behavior along the same line

I tried to reproduce this using the encoding/xml and encoding/json packages, which both define NewEncoder:

• add var _ = xml.NewEncoder and save file. import "encoding/xml" appears.
• replace "xml." with "json." and save file. The import changes to "encoding/json".
• add var _ = xml. and start typing. Completions within xml package are observed, even though it is not imported.
• hit tab to accept completion. import "encoding/xml" appears. (We now have both imports.) This is normal.
• If instead of the previous two steps, we replace the existing var _ = json... with var _ = xml., then start typing NewEnc, accepting the completion adds an xml import but doesn't remove the json import that is no longer needed. This too is normal; redundant imports are not removed until the file is saved.

If you see something different, could you please give instructions for reproducing it with these standard library symbols? Ideally this should be a separate GitHub issue, since it is unrelated to the other two items.

I installed the latest gopls as per @findleyr and followed the steps for issue 2 and everything behaved as you described as normal with the redundant imports not being removed until saving the file and the xml completions showing before the package is imported on-save.