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

golangci-lint: support autofix #2679

Closed
seaneagan opened this issue May 22, 2023 · 16 comments · Fixed by #2700
Closed

golangci-lint: support autofix #2679

seaneagan opened this issue May 22, 2023 · 16 comments · Fixed by #2700
Labels
enhancement New feature or request

Comments

@seaneagan
Copy link
Contributor

Is your feature request related to a problem? Please describe.
golangci-lint supports a --fix option, but it's not supported by megalinter:
https://golangci-lint.run/usage/configuration/#command-line-options

Describe the solution you'd like
APPLY_FIXES against GO_GOLANGCI_LINT results in golangci-lint --fix ...

Describe alternatives you've considered

Running golangci-lint manually outside megalinter

@seaneagan seaneagan added the enhancement New feature or request label May 22, 2023
@Kurt-von-Laven
Copy link
Collaborator

Sounds like a good idea. Feel free to open a pull request!

@echoix
Copy link
Collaborator

echoix commented May 22, 2023

Since golang-ci is a Golang linter aggregator (compiled into one binary), and on their help page, they mention that the --fix option will work if it is supported by the linter, then for the tests file, we should find an error that is fixable by one of the linters to test that the option works when provided by MegaLinter.

Ie: it's possible that our test files for Go won't trigger a fix as is.

@nvuillam
Copy link
Member

I share @Kurt-von-Laven & @echoix 's opinion, that would be a good improvement :)

@seaneagan do you have an example of error that could be autofixed in this file ? https://github.com/oxsecurity/megalinter/blob/main/.automation/test/golang/golang_good_01.go

@seaneagan
Copy link
Contributor Author

@nvuillam the unused variable check is enabled by default, and autofixing will remove the line where the variable is defined, so that might be a good option

@nvuillam
Copy link
Member

Well, we'll see what CI jobs say in #2683 :)

@nvuillam
Copy link
Member

@seaneagan not working yet...

image

Maybe you could provide an example of file + example call ?
Is the fix just applied on the go file with --fix, or maybe it requires extra arguments ?

@Kurt-von-Laven
Copy link
Collaborator

The call itself looks right to me based on the docs, so I'm guessing we simply need a different example.

@seaneagan
Copy link
Contributor Author

@nvuillam do the tests use one of these .golangci.yml's?

https://github.com/search?q=repo%3Aoxsecurity%2Fmegalinter%20path%3A*.golangci.yml&type=code

those don't match the golangci-lint's default config, which has the unused enabled: https://golangci-lint.run/usage/linters/#enabled-by-default:

it seems the git history of those files got lost in the superlinter > megalinter migration, but updating them to match the current default might make sense:

linters:
  disable-all: true
  enable:
    - gocritic
    - gosimple
    - govet
    - ineffassign
    - staticcheck
    - typecheck
    - unused

@seaneagan
Copy link
Contributor Author

^ would also get rid of the deprecated linter usage (maligned, golint)

@nvuillam
Copy link
Member

@seaneagan thanks, I try that :)

@nvuillam
Copy link
Member

@seaneagan Still no fix with this update :/

https://github.com/oxsecurity/megalinter/actions/runs/5088248526/jobs/9144546351?pr=2683

Maybe you could try locally and provide the exact call and test file ?

@seaneagan
Copy link
Contributor Author

sorry, i thought unused supported auto-fix but it doesn't, nor do any of the enabled by default linters. gofmt does though, so i'd suggest to add that to the list of linters in .golangci.yml.

Probably the most clear formatting mistake, which will get fixed by the above, will be to under-indent a line:

// This is a package comment
package main

import "fmt"

func nicolas() {
fmt.Println("hello world")
}

@nvuillam
Copy link
Member

Thanks I try that :)

@nvuillam
Copy link
Member

@seaneagan still KO :/

image

@seaneagan
Copy link
Contributor Author

@nvuillam attempted to fix here: #2700

looks like it needs your approval to kick off the tests

@seaneagan
Copy link
Contributor Author

@nvuillam #2700 looks good to go, it's based on #2683 so updating that one is fine too, either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants