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

Jump to bottom

x/tools/cmd/goimports: put std, cmd packages in separate blocks #38735

Closed
jayconrod opened this issue Apr 28, 2020 · 4 comments
Closed

x/tools/cmd/goimports: put std, cmd packages in separate blocks #38735

jayconrod opened this issue Apr 28, 2020 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@jayconrod
Copy link
Contributor

This came up in CL 228378 and many others. If we delete all the imports from src/cmd/go/internal/modload/modfile.go then run goimports, we should see cmd imports in a separate group from std imports:

import (
	"fmt"
	"io/ioutil"
	"path/filepath"

	"cmd/go/internal/base"
	"cmd/go/internal/cfg"
	"cmd/go/internal/modfetch"

	"golang.org/x/mod/modfile"
	"golang.org/x/mod/module"
	"golang.org/x/mod/semver"
)

Instead, all the std and cmd imports are grouped together.

import (
	"cmd/go/internal/base"
	"cmd/go/internal/cfg"
	"cmd/go/internal/modfetch"
	"fmt"
	"io/ioutil"
	"path/filepath"

	"golang.org/x/mod/modfile"
	"golang.org/x/mod/module"
	"golang.org/x/mod/semver"
)

goimports does the right thing if at least one cmd import is already in a separate group. It combines the groups when adding the first std or cmd import though.
@jayconrod jayconrod added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository. labels Apr 28, 2020
@jayconrod jayconrod added this to the Unreleased milestone Apr 28, 2020
@heschi
Copy link
Contributor

heschi commented Apr 29, 2020

To be clear, do you really want it in three sections, or do you just want cmd/ grouped with golang.org/x? Because the latter seems like a bug that should be fixed. The former would be more of a job for -local.

@bcmills
Copy link
Contributor

bcmills commented Apr 29, 2020

The existing files in cmd/go mostly have it in three sections, so we would like to retain that.

But it looks like the existing files in cmd/compile make no such distinction. Perhaps we should reformat cmd to eliminate it too, and collapse down to only two sections.

@heschi
Copy link
Contributor

heschi commented Apr 29, 2020

Not sure what you'd like me to do here. I can change it so that cmd/ is grouped with golang.org, but -local is the only way to get three groups, and in general goimports will never merge groups, so if you want to collapse down to two you're going to have to give it some help.

@jayconrod
Copy link
Contributor Author

After some discussion, we're leaning toward just combining std and cmd sections.

I was not at all aware of the -local flag, so it looks like that's there if we decide otherwise.

I'll close this. Looks like there's nothing to do on the goimports side.

@golang golang locked and limited conversation to collaborators Apr 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@jayconrod @bcmills @gopherbot @heschi