golang/go

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

lfolger opened this issue · 6 comments

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.