Adding gofumpt #1177

teivah · 2020-06-05T12:01:28Z

Hey!
I was also interested in #490 (adding gofumpt).
Here is a PR for it.

boring-cyborg · 2020-06-05T12:01:31Z

Hey, thank you for opening your first Pull Request !

CLAassistant · 2020-06-05T12:01:33Z

All committers have signed the CLA.

tetafro · 2020-06-05T14:12:18Z

pkg/golinters/gofumpt.go

+					issues = append(issues, goanalysis.NewIssue(&result.Issue{
+						FromLinter: gofumptName,
+						Text:       "File is not `gofumpt`-ed",
+						Pos: token.Position{


There is a Replacement parameter to automatically fix issues, which is very useful for gofumpt.

If I'm not mistaken, gofumpt provides only as a public function a way to get the gofumpt-ed version of the file (as a []byte).
With Replacement you mean I should include the gofumpt-ed version of the whole file in the issue?

@tetafro Am I correct?

gofumpt provides only as a public function a way to get the gofumpt-ed version of the file

Yeah, sorry, I didn't think of it.

With Replacement you mean I should include the gofumpt-ed version of the whole file in the issue?

I'm not sure if it's a right thing to do. Someone more experienced with golangci-lint architecture should answer.

Anyway, if there's no easy way to do this, just skip my first comment.

tetafro · 2020-06-09T07:37:01Z

Can you please add test to tests/testdata?

tetafro · 2020-06-09T07:19:15Z

pkg/golinters/gofumpt.go

+					issues = append(issues, goanalysis.NewIssue(&result.Issue{
+						FromLinter: gofumptName,
+						Text:       "File is not `gofumpt`-ed",
+						Pos: token.Position{


gofumpt provides only as a public function a way to get the gofumpt-ed version of the file

Yeah, sorry, I didn't think of it.

With Replacement you mean I should include the gofumpt-ed version of the whole file in the issue?

I'm not sure if it's a right thing to do. Someone more experienced with golangci-lint architecture should answer.

Anyway, if there's no easy way to do this, just skip my first comment.

tetafro · 2020-06-09T07:20:13Z

pkg/lint/lintersdb/manager.go

@@ -191,6 +191,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithPresets(linter.PresetFormatting).
 			WithAutoFix().
 			WithURL("https://golang.org/cmd/gofmt/"),
+		linter.NewConfig(golinters.NewGofumpt()).
+			WithPresets(linter.PresetFormatting).
+			WithAutoFix().


If there is no autofix, you should remove this line

# Conflicts: # go.sum

teivah · 2020-06-09T08:19:11Z

Can you please add test to tests/testdata?

@tetafro I've just added a test. Just something to mention, previously I was setting the error line to 0. I wanted to change it to line 1 but in the test file, I can't do this:

// ERROR "File is not `gofumpt`-ed"
//args: -Egofumpt
package testdata

Hence, I set it to the last line number and it's working correctly.

tetafro · 2020-06-12T19:23:08Z

Hence, I set it to the last line number and it's working correctly.

I've looked how gofmt linter is implemented. Seems like the right thing to do here is to re-use diff code from gofmt linter.

But I think using the last line is ok for the first iteration. Maybe just add TODO in the code.

tetafro · 2020-06-12T18:49:34Z

pkg/golinters/gofumpt.go

+					return nil, fmt.Errorf("error while running gofumpt: %w", err)
+				}
+				if !bytes.Equal(input, output) {
+					reader, err := os.Open(f)


Is it really necessary? You've already read the file into input. Just use strings.Count.

tetafro

Thanks! This looks good to me.

teivah · 2020-06-15T09:38:53Z

Thanks! This looks good to me.

@tetafro Any chance to get it merged then? :)

tetafro · 2020-06-16T10:05:04Z

Any chance to get it merged then? :)

Yeah, sorry for delay

golangci-automator · 2020-06-16T10:05:51Z

Hey, @teivah — we just merged your PR to golangci-lint! 🔥🚀

golangci-lint is built by awesome people like you. Let us say “thanks”: we just invited you to join the GolangCI organization on GitHub.
This will add you to our team of maintainers. Accept the invite by visiting this link.

By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.
More information about contributing is here.

Thanks again!

teivah · 2020-06-16T10:08:59Z

Thanks @tetafro

gonzaloserrano · 2020-06-16T13:04:34Z

Hi, how's the doc updated once a new linter is merged?

teivah · 2020-06-22T07:47:18Z

@tetafro Just out of curiosity, when do you plan roughly to make a new release?
Thanks

tetafro · 2020-06-22T09:11:22Z

@teivah this is a question for guys from core team. I just help with pr reviews from time to time.

Adding gofumpt

0368987

teivah mentioned this pull request Jun 5, 2020

Add gofumpt #490

Closed

teivah added 2 commits June 5, 2020 14:13

Updating changelog

b44db2b

Bytes comparison simplification

d16d013

tetafro reviewed Jun 5, 2020

View reviewed changes

tetafro reviewed Jun 9, 2020

View reviewed changes

teivah added 3 commits June 9, 2020 10:14

Adding test

dffb7b4

Merge branch 'master' into gofumpt

d77761d

# Conflicts: # go.sum

Updating go.sum

a5fcffd

teivah requested a review from tetafro June 9, 2020 08:19

Fixing magic number

2fa559f

tetafro reviewed Jun 12, 2020

View reviewed changes

teivah added 2 commits June 12, 2020 21:44

Using strings.count

badf526

Fixing linting issue

108fb8c

teivah requested a review from tetafro June 12, 2020 19:47

tetafro approved these changes Jun 13, 2020

View reviewed changes

tetafro merged commit 38d298c into golangci:master Jun 16, 2020

gonzaloserrano mentioned this pull request Jun 16, 2020

consider golangci-lint integration? mvdan/gofumpt#69

Closed

teivah deleted the gofumpt branch June 22, 2020 07:47

SVilgelm linked an issue Jul 13, 2020 that may be closed by this pull request

Add gofumpt #490

Closed

ldez added the linter: new Support new linter label Dec 7, 2020

ldez added this to the v1.28 milestone Mar 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding gofumpt #1177

Adding gofumpt #1177

teivah commented Jun 5, 2020

boring-cyborg bot commented Jun 5, 2020

CLAassistant commented Jun 5, 2020 •

edited

Loading

tetafro Jun 5, 2020

teivah Jun 5, 2020

teivah Jun 8, 2020

tetafro Jun 9, 2020

tetafro commented Jun 9, 2020

tetafro Jun 9, 2020

tetafro Jun 9, 2020

teivah commented Jun 9, 2020

tetafro commented Jun 12, 2020 •

edited

Loading

tetafro Jun 12, 2020

tetafro left a comment

teivah commented Jun 15, 2020

tetafro commented Jun 16, 2020

golangci-automator bot commented Jun 16, 2020

teivah commented Jun 16, 2020

gonzaloserrano commented Jun 16, 2020

teivah commented Jun 22, 2020

tetafro commented Jun 22, 2020

Adding gofumpt #1177

Adding gofumpt #1177

Conversation

teivah commented Jun 5, 2020

boring-cyborg bot commented Jun 5, 2020

CLAassistant commented Jun 5, 2020 • edited Loading

tetafro Jun 5, 2020

Choose a reason for hiding this comment

teivah Jun 5, 2020

Choose a reason for hiding this comment

teivah Jun 8, 2020

Choose a reason for hiding this comment

tetafro Jun 9, 2020

Choose a reason for hiding this comment

tetafro commented Jun 9, 2020

tetafro Jun 9, 2020

Choose a reason for hiding this comment

tetafro Jun 9, 2020

Choose a reason for hiding this comment

teivah commented Jun 9, 2020

tetafro commented Jun 12, 2020 • edited Loading

tetafro Jun 12, 2020

Choose a reason for hiding this comment

tetafro left a comment

Choose a reason for hiding this comment

teivah commented Jun 15, 2020

tetafro commented Jun 16, 2020

golangci-automator bot commented Jun 16, 2020

teivah commented Jun 16, 2020

gonzaloserrano commented Jun 16, 2020

teivah commented Jun 22, 2020

tetafro commented Jun 22, 2020

CLAassistant commented Jun 5, 2020 •

edited

Loading

tetafro commented Jun 12, 2020 •

edited

Loading