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: add --format-only flag #31745

Closed
natefinch opened this issue Apr 29, 2019 · 5 comments
Closed

x/tools/cmd/goimports: add --format-only flag #31745

natefinch opened this issue Apr 29, 2019 · 5 comments
Milestone

Comments

@natefinch
Copy link
Contributor

@natefinch natefinch commented Apr 29, 2019

Proposal - allow goimports to "just format"

Add a flag or some such that skips all the time-intensive adding/removing of imports, and just formats the file(s).

Use Cases

A Compromise Between gofmt and goimports

Some people like goimports, some people don't. But when the two meet... everyone is sad, because goimports and gofmt don't format the same, causing spurious changes in files when goimports "fixes" files that gofmt thinks are fine.... i.e. sorting stdlib imports into their own section.

If goimports had a way to just format, then a compromise could be reached where a project standardizes on goimports' format, but then wouldn't require people to actually use goimports import add/ remove logic.

Formatting Everything

Additionally, right now there's no way to ensure that an entire codebase is properly formatted using goimports' style. Running goimports -w . on any non-trivial codebase is just amazingly slow (for good reason - it's doing a ton of work). In contrast, go fmt ./... is quite fast. Running go fmt on our 100,000-ish LOC project at work completes in less than a second. Running goimports on the same project... well, I stopped it after a half hour, so I don't know how long it actually takes.

I presume if we skipped all the import checking, that the goimports formatting would be approximately as fast as go fmt.

@gopherbot gopherbot added this to the Unreleased milestone Apr 29, 2019
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Apr 29, 2019

I hate to add more flags, but sure.

Maybe that'd prevent more bugs like #26846 and #27365.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 29, 2019

Change https://golang.org/cl/174326 mentions this issue: cmd/goimports: add -format-only flag

@heschik
Copy link
Contributor

@heschik heschik commented Apr 29, 2019

I don't understand why goimports would take any significant amount of time at all on a codebase that was already mostly goimports-clean.

@cespare
Copy link
Contributor

@cespare cespare commented Apr 29, 2019

goimports speed issues probably belong in a different issue, but I don't see anything resembling this dramatic goimports/gofmt difference. On our 325,000 LOC Go codebase gofmt -l takes 2.5s to run and goimports -l takes 14s to run.

@muirdm
Copy link

@muirdm muirdm commented May 23, 2019

FYI goimports slowness might be from #22139. Code has changed since I filed that, but issue might be present. You can try passing absolute paths to goimports -w and see if that speeds it up.

@golang golang locked and limited conversation to collaborators May 22, 2020
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
6 participants
You can’t perform that action at this time.