Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/cmd/goimports: make "-local" flag match exactly path without final "/" in addition to prefix #24368

Closed
nmiyake opened this issue Mar 13, 2018 · 3 comments

Comments

@nmiyake
Copy link

@nmiyake nmiyake commented Mar 13, 2018

Currently, it is not possible for goimports to group imports in a manner that encapsulates the operation "group all packages from a repository into the same group" using the -local flag due to the nature of prefix matches.

If a file imports github.com/org/project, github.com/org/project/pkg and github.com/org/project2/pkg, -local github.com/org/project will match the undesired package github.com/org/project2, while -local github.com/org/project/ will not match imports for the root package github.com/org/project.

Proposal

As a special case, if a prefix provided to -local ends in a slash (/) and the import being considered is exactly the prefix without the slash, treat it as a match. Although this modifies the current behavior, I believe that it semantically captures what such a prefix match would be trying to achieve (the fix is also very small and targeted). I believe that any other fix would require adding flags or options, which I know is not desirable.

Example repros

Groups github.com/org/project2/pkg with imports from github.com/org/project:

echo 'package foo\nimport (\n_ "fmt"\n_ "gopkg.in/yaml.v2"\n_ "github.com/org/project/pkg"\n_ "github.com/org/project"\n_ "github.com/org/project2/pkg"\n)' | cat | goimports -local github.com/org/project

package foo

import (
	_ "fmt"

	_ "gopkg.in/yaml.v2"

	_ "github.com/org/project"
	_ "github.com/org/project/pkg"
	_ "github.com/org/project2/pkg"
)

Does not group import for github.com/org/project with github.com/org/project/pkg:

echo 'package foo\nimport (\n_ "fmt"\n_ "gopkg.in/yaml.v2"\n_ "github.com/org/project/pkg"\n_ "github.com/org/project"\n_ "github.com/org/project2/pkg"\n)' | cat | goimports -local github.com/org/project/

package foo

import (
	_ "fmt"

	_ "github.com/org/project"
	_ "github.com/org/project2/pkg"
	_ "gopkg.in/yaml.v2"

	_ "github.com/org/project/pkg"
)

The proposed behavior would result in:

echo 'package foo\nimport (\n_ "fmt"\n_ "gopkg.in/yaml.v2"\n_ "github.com/org/project/pkg"\n_ "github.com/org/project"\n_ "github.com/org/project2/pkg"\n)' | cat | goimports -local github.com/org/project/

package foo

import (
	_ "fmt"

	_ "github.com/org/project2/pkg"
	_ "gopkg.in/yaml.v2"

	_ "github.com/org/project"
	_ "github.com/org/project/pkg"
)
@gopherbot gopherbot added this to the Unreleased milestone Mar 13, 2018
@andybons
Copy link
Member

@andybons andybons commented Mar 13, 2018

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Mar 13, 2018

Sure. If somebody wants to send a CL, that's fine, but it's not something that I will be working on myself.

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 13, 2018

Change https://golang.org/cl/100460 mentions this issue: x/tools/cmd/goimports: support match without trailing slash for -local flag

@golang golang locked and limited conversation to collaborators Mar 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.