x/tools/internal/refactor/inline: analyzer generates unnecessary import renames

[lfolger]

Go version

not relevant

Output of go env in your module/workspace:

not relevant

What did you do?

func TestNoUnnecessaryImportRename(t *testing.T) {
	files := map[string]string{
		"some/other/foo/foo.go": `
			package foo

			func Bar () {
			}
		`,
		"some/package/foo/foo.go": `
			package foo

			import "some/other/foo"

			// inlineme
			func Bar () {
				foo.Bar()
			}
		`,
		"b/c/foo.go": `
package c
import (
	"some/package/foo"
)

func f() {
	foo.Bar()
}
		`,
		"b/c/foo.go.golden": `
package c
import (
	"some/other/foo"
)

func f() {
	foo.Bar()
}
		`,
	}
	dir, cleanup, err := analysistest.WriteFiles(files)
	if err != nil {
		t.Fatal(err)
	}
	analysistest.RunWithSuggestedFixes(t, dir, analyzer.Analyzer, "b/c")
	cleanup()
}

What did you see happen?

=== RUN   TestNoUnnecessaryImportRename
    .../go/analysis/analysistest/analysistest.go:523: b/c/foo.go:8:2: unexpected diagnostic: inline call of foo.Bar
    .../go/analysis/analysistest/analysistest.go:263: suggested fixes failed for /tmp/analysistest3217487681/src/b/c/foo.go:
        --- /tmp/analysistest3217487681/src/b/c/foo.go.golden
        +++ actual
        @@ -1,9 +1,9 @@
         package c

         import (
        -       "some/other/foo"
        +       foo0 "some/other/foo"
         )

         func f() {
        -       foo.Bar()
        +       foo0.Bar()
         }
--- FAIL: TestNoUnnecessaryImportRename (0.04s)

What did you expect to see?

I would expect the analyzer to not rename the import if it removes the conflicting import the same suggested fix.

This pattern is common when you use the inline analyzer to facilitate package moves and it would be nice if the analyzer doesn't leave the code base with a bunch of unecessary import renames.

[lfolger]

cc: @adonovan

[lfolger]

Any update on this?

[adonovan]

I have a sketch of a fix but it was a little ugly and I haven't found time to improve it. Perhaps this week? I could also share it with @timothy-king since he's been keen to get his hands dirty in this package.

[lfolger]

I'm happy to take a stab at it myself if you don't have the capacity just wanted to make sure we are not duplicating work here.

[gopherbot]

Change https://go.dev/cl/592575 mentions this issue: internal/refactor/inline: avoid unnecessary import renames

[gopherbot]

Change https://go.dev/cl/593035 mentions this issue: internal/refactor/inline: avoid unnecessary import renames