x/tools/internal/imports: trailing comment with double-quote breaks imports grouping/sorting

[jhump]

A regexp is used to extract the import path from a line of code when inserting newlines to split a sequence of imports into groups. The regexp uses greedy matching, which means if there is an errant quote after the import path (such as when a trailing comment includes a quote), the regexp fails to find the line. By failing to find the line, it never inserts any newlines and thus fails to actually group and sort imports.

What version of Go are you using (go version)?

I'm using Go 1.16 and Go 1.18rc1. The issue is in the golang.org/x/tools module and happens with any version of Go.

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

I've tried this using amd64 and arm64 on windows, darwin, and linux. The issue is not platform-specific.

What did you do?

Here's an example input that tickles the problem:

// test.go
package boo

import (
	"context"
	baz "foo.bar/baz"
	buzz "foo.bar/buzz"
	fizz "foo.bar/fizz"
	bar "[github.com/foo/bar](http://github.com/foo/bar)" // this is a "typical" import
)

var (
	_ = context.Background()
	_ = fizz.F{}
	_ = buzz.B{}
	_ = bar.B{}
	_ = baz.B{}
)

We invoke goimports with the above input:

goimports -local foo.bar/ test.go

What did you expect to see?

Expected imports:

	"context"

	bar "github.com/foo/bar" // this is a "typical" import

	baz "foo.bar/baz"
	buzz "foo.bar/buzz"
	fizz "foo.bar/fizz"

What did you see instead?

Because the newlines are not inserted, to break up the imports into groups, the imports get re-sorted by the final "format" step, so
the actual imports end up like this instead:

	"context"
	baz "foo.bar/baz"
	buzz "foo.bar/buzz"
	fizz "foo.bar/fizz"
	bar "github.com/foo/bar" // this is a "typical" import

Upon stepping through the code, it is observed that at this line, the regexp match looks like github.com/foo/bar" // this is a "typical, instead of just github.com/foo/bar.

This is easily fixed by changing the relevant regexp to use a non-greedy match, so its matched text ends at the first observed double-quote instead of the very last one. See CL 386914 for the fix.

[ianlancetaylor]

You don't have to open an issue if you've already sent a fix.

[gopherbot]

Change https://go.dev/cl/386914 mentions this issue: internal/imports: use first quote when matching import path

[jhump]

@ianlancetaylor, sorry if this is abnormal. Creating a new issue and linking to it was recommended by @dmitshur in a comment in CL 386914.

[ianlancetaylor]

Ah, I see, to capture the information in the description. OK, sorry.