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

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.

Comment From: lfolger

cc: @adonovan

Comment From: lfolger

Any update on this?

Comment From: 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.

Comment From: 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.

Comment From: gopherbot

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

Comment From: gopherbot

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