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
Labels
FeatureRequest FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@nmiyake
Copy link
Contributor

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 commented Mar 13, 2018

/cc @bradfitz @ianlancetaylor

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 13, 2018
@bradfitz
Copy link
Contributor

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

@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. FeatureRequest labels Mar 13, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 13, 2018
@gopherbot
Copy link

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.
Labels
FeatureRequest FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants