Skip to content

Support --fix for gofumpt #1239

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
Jul 19, 2020
Merged

Support --fix for gofumpt #1239

merged 1 commit into from
Jul 19, 2020

Conversation

SVilgelm
Copy link
Member

@SVilgelm SVilgelm commented Jul 13, 2020 •
edited
Loading

Add autofix functionality (-w)
Add fix and linter tests.
Closes: #490

Since the gofumpt/format.Source returns already formatted body of a file, but golangci-lint expects a diff to support auto-fixing, so I use github.com/shazow/go-diff/difflib library to make a diff of input and output. I expect, that it can be slower than gofmt linter.

@SVilgelm SVilgelm changed the title Support --fix for gofumpt WIP Support --fix for gofumpt Jul 13, 2020
@SVilgelm SVilgelm mentioned this pull request Jul 13, 2020
Closed
@SVilgelm SVilgelm force-pushed the gofump-formatter branch 2 times, most recently from 457ad6a to db4a9e6 Compare July 13, 2020 02:37
@andrewrynhard
Copy link

gofumpt also has gofumports. Any way we can sneak that in here?

@SVilgelm SVilgelm mentioned this pull request Jul 13, 2020
Closed
@SVilgelm
Copy link
Member Author

gofumpt also has gofumports. Any way we can sneak that in here?

I created an issue to add it as a new linter

@SVilgelm SVilgelm added the linter: update version Update version of linter label Jul 13, 2020
@SVilgelm SVilgelm force-pushed the gofump-formatter branch 3 times, most recently from 44e4831 to 2db7582 Compare July 13, 2020 21:53
@SVilgelm SVilgelm changed the title WIP Support --fix for gofumpt Support --fix for gofumpt Jul 13, 2020
@SVilgelm SVilgelm requested a review from a team July 13, 2020 22:03
@SVilgelm SVilgelm force-pushed the gofump-formatter branch 2 times, most recently from 617c693 to 7972626 Compare July 15, 2020 19:13
@SVilgelm
Copy link
Member Author

@golangci/team Please review this

@daixiang0
Copy link
Contributor

/lgtm

@SVilgelm SVilgelm requested a review from a team July 16, 2020 03:55
@SVilgelm
Copy link
Member Author

@golangci/team Could anyone please review it?

@SVilgelm
Copy link
Member Author

@golangci/team Could anyone review it? Approve?
@sayboras @tetafro @daixiang0 ???

bombsimon
bombsimon approved these changes Jul 18, 2020
View reviewed changes
Copy link
Member

@bombsimon bombsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SVilgelm SVilgelm merged commit c8bd954 into golangci:master Jul 19, 2020
@SVilgelm SVilgelm deleted the gofump-formatter branch July 19, 2020 02:33
@arvenil
Copy link

arvenil commented Aug 4, 2020

This new linter broke goimports sorting with -local and --fix. I first thought it's gci linter as it seems to do similar thing as goimports, but in fact gci doesn't sort local-prefixes at all (broken?). I've disabled gofumpt and now goimports works correctly. I think it's a general issue that there is no single well working import sorter and some linters support partial import sorting and then they fight with other linters for which one has the final word on the order. I've noticed that every run I got different order of packages. Just FYI.

@daixiang0
Copy link
Contributor

gci foucs on import group by setting local-prefixes, it is expected to be run after goimports.

@ldez ldez added this to the v1.29 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daixiang0 daixiang0 daixiang0 left review comments

@bombsimon bombsimon bombsimon approved these changes

Assignees
No one assigned
Labels
linter: update version Update version of linter
Projects
None yet
Milestone
v1.29
Development

Successfully merging this pull request may close these issues.

Add gofumpt
6 participants
@SVilgelm @andrewrynhard @daixiang0 @arvenil @bombsimon @ldez