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

Add gofumpt #490

Closed
andrewrynhard opened this issue Apr 10, 2019 · 14 comments · Fixed by #1239, #1228 or #1177
Closed

Add gofumpt #490

andrewrynhard opened this issue Apr 10, 2019 · 14 comments · Fixed by #1239, #1228 or #1177
Assignees
Labels
help wanted Issue that needs help from a contributor linter: new Support new linter linter: update version Update version of linter

Comments

@andrewrynhard
Copy link

Include https://github.com/mvdan/gofumpt in set of linters.

@jirfag jirfag added the linter: new Support new linter label Apr 21, 2019
@jirfag
Copy link
Member

jirfag commented Apr 21, 2019

Hi, nice tool, thank you!
Pull requests are welcome

@dackroyd
Copy link

Given gofumpt/gofumports does everything that gofmt / goimports would do, what is the preference for applying this?

Enforce a stricter format than gofmt, while being backwards compatible. That is, gofumpt is happy with a subset of the formats that gofmt is happy with.
The tool is a modified fork of gofmt, so it can be used as a drop-in replacement. Running gofmt after gofumpt should be a no-op.

Would gofumpt (and gofumports) be added as an entirely new linter, or options for gofmt/goimports ?

@tpounds tpounds added the help wanted Issue that needs help from a contributor label Oct 8, 2019
@jirfag jirfag added this to To do in Support more linters Oct 15, 2019
@gonzaloserrano
Copy link

Hey there, I'm also interested in adding gofumpt/goimports.

Would gofumpt (and gofumports) be added as an entirely new linter, or options for gofmt/goimports ?

About new linter vs gofmt option is see pros and cons:

  • new linter pros: more clean to add/remove the linter if it gets deprecated
  • cons: does not make sense to run gofmt/goimports if gofumpt/gofumports are enabled since they are a superset of gofmt/goimports, so how to handle this?

@ernado
Copy link
Member

ernado commented Feb 10, 2020

Hey, sorry for long responses, we are currently trying to deal with bugs and preparing to go1.14, so very low of capacity.

I'm also not sure how to deal with linters that superset other linters, e.g. we can't just auto-ignore them.

@gonzaloserrano
Copy link

Maybe we can just start adding it as a new linter and disable gofmt/goimports manually.

@teivah teivah mentioned this issue Jun 5, 2020
@teivah
Copy link
Member

teivah commented Jun 5, 2020

I created a PR for it: #1177

@dnwe
Copy link

dnwe commented Jun 24, 2020

@teivah thanks for working on the PR, pleased to see that it was merged

A quick scan of the PR suggests it doesn't currently map the golangci-lint "Autofix" functionality to a gofumpt -s -w equivalent, like it does for the gofmt linter. It would be good if that could be added, as then we could switch our .golanci-lint.yml to disable the fmt linter and switch to gofumpt wholesale

cc @mvdan as he'd probably be interested in knowing that this work is taking place to add gofumpt to golangci-lint

@SVilgelm
Copy link
Member

Since gofump returns already formatted file instead of diff, so we need to use something like this https://github.com/sergi/go-diff library to create a diff then use extractIssuesFromPatch function

@SVilgelm SVilgelm moved this from To do to In progress in Support more linters Jul 13, 2020
@SVilgelm SVilgelm self-assigned this Jul 13, 2020
@SVilgelm
Copy link
Member

@dnwe Could you please test the #1239 ?

@invidian
Copy link
Contributor

This has been added in #1177.

@SVilgelm
Copy link
Member

@invidian you are right, the gofumpt linter was added in that PR, but it doesn't support autofixing, I'm adding it in the #1239

@Antonboom
Copy link
Contributor

@SVilgelm, hello!

Could you tell me something about this?
mvdan/gofumpt#79

@SVilgelm
Copy link
Member

@Antonboom latest master has an updated version of gofumpt and it works correctly for the example you provided.
The PR with the update: #1228

@Antonboom
Copy link
Contributor

Then I have to wait v1.28.4
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment