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.