x/tools/internal/refactor/inline: analyzer diffs that remove same import lead to (spurious) conflict

[lfolger]

Go version

gc-tip (should reproduce with all versions)

Output of go env in your module/workspace:

N/A

What did you do?

Running the inline analyzer on a package where it would inline multiple functions leads to duplicate suggested fixes that some tools cannot deal with (including the analysistest package). As far as I can tell it tries to remove the import twice. In general it seems to be an issue that it always tries to remove the import even if it is still needed (see example below).

Here is a self contained reproducer test:

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

			// inlineme
			func ToInline () {}

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

			import (
				"some/package/pkg"
			)

			func foo() {
				pkg.ToInline() // want "inline call of pkg.ToInline"
			}

			func bar() {
				pkg.ToInline() // want "inline call of pkg.ToInline"
				pkg.Bar() // ok
			}
		`,
		"b/c/foo.go.golden": `package c
			func foo() {
			}

			func 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?

The example fails with:

...analysistest.go:263: /tmp/analysistest3675113240/src/b/c/foo.go: error applying fixes: diff has overlapping edits (see possible explanations at RunWithSuggestedFixes)

What did you expect to see?

I expected the analyzer to not remove the import when it is still needed and if it is no longer needed to only remove the import once.

[Gruak]

I also need this for the same reason. It would also be nice to be able to set a baseUrl getter per client that would read the baseUrl from an external store on request or be able to set the baseUrl through a middleware.

[drwpow]

This is an easy add and wouldn’t disrupt anything. Would welcome a PR for this 🙂

[RobinClowers]

@Gruak I forgot about this, thanks for making that change! @drwpow can we get a release with the change soon?

[nikrooz]

Thank you for adding this feature. I was wondering if there are any plans to release it soon.

[drwpow]

Thank you for adding this feature. I was wondering if there are any plans to release it soon.

Please see the CHANGELOG. This has already been released!