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

Support revive #238

Closed
jirfag opened this issue Oct 3, 2018 · 16 comments · Fixed by #1729
Closed

Support revive #238

jirfag opened this issue Oct 3, 2018 · 16 comments · Fixed by #1729
Labels
linter: new Support new linter

Comments

@jirfag
Copy link
Member

jirfag commented Oct 3, 2018

https://github.com/mgechev/revive

@jirfag jirfag added the linter: new Support new linter label Oct 3, 2018
@jirfag
Copy link
Member Author

jirfag commented Nov 7, 2018

I've tested revive a bit, looks like all warnings are the same as in golint. Why do we need revive in golangci-lint if we already have golint?

@s3rj1k
Copy link

s3rj1k commented Nov 11, 2018

Revive provides more rules compared to golint.

@jirfag
Copy link
Member Author

jirfag commented Nov 12, 2018

ok, I will look at it, maybe it's rules are already implemented by other linters

jirfag added a commit that referenced this issue Feb 11, 2019
$ git cherry --abbrev -v 8afd9cbb6cfb 66fb7fc33547
+ 63b25c1 Fix typo in README (#235)
+ 419c929 G107 - SSRF (#236)
+ 145f1a0 Removed wrapping feature (#238)
+ ec32ce6 Support Go 1.11 (#239)
+ 762ff3a Allow quoted strings to be used to format SQL queries (#240)
+ 7f6509a Update README.md (#246)
+ 5f98926 Refactor Dockerfile (#245)
+ d3f1980 Fix false positives for SQL string concatenation with constants from another file (#247)
+ 64d58c2 Refactor the test code sample to support multiple files per sample
+ 1ecd47e bump Dockerfile golang from 1.10 to 1.11
+ 027dc2b This fixes the html template when using '-fmt=html'  - resolves HTML escaping issues within the template  - resolves reference issues to reportInfo struct i.e. issues -> Issues, metrics -> Stats
+ 8c09a83 Add install.sh script
+ 97bc137 Add CI Installation steps and correct markdown lint errors
+ 3116b07 Fix typos in comments and rulelist (#256)
+ 443f84f Fix golint link (#263)
+ 4180994 Make G201 ignore CallExpr with no args (#262)
+ 9b966a4 add test case for strings.Builder G104 whitelist inclusion
+ adb4222 whitelist strings.Builder method in rule G104
+ ae82798 Fix the WriteSring test by handling the error
+ 2695567 Build the code sample for string builder only fron Go 1.10 onwards
+ f14f17f Add a helper function which extracts the string parameters values of a call expression
+ 9b32fca Fix the bind rule to handle the case when the arguments of the net.Listen are returned by a function call
+ 24e3094 Extend the bind rule to handle the case when the net.Listen address in provided from a const
+ 72e95e8 Geneate and upload the test coverage report to codecove.io
+ 12400f9 Update README with the code coverage batch
+ 14ed63d Do not flag the unhandled errors which are explicitly ignored
+ f87af5f Detect the unhandled errors even though they are explicitly ignored if the 'audit: enabled' setting is defined in the global configuration (#274)
+ 5d33e6e Update the README with some details about the configuration file
+ b662615 Fix typo
+ a966ff7 Fix -conf example in README.md
+ 04ce7ba add a no-fail flag
+ e2752bc revert to default GOPATH if necessary (#279)
- c04360f make API
+ 66fb7fc Replace import paths
@AlekSi
Copy link
Contributor

AlekSi commented Apr 20, 2019

In my project I would really like to use this check: https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#empty-lines I don't think it is implemented anywhere else.

@jirfag
Copy link
Member Author

jirfag commented Apr 21, 2019

@AlekSi thank you. A similar check is in gofumpt, which is not included into golangci-lint too

@akyoto
Copy link

akyoto commented Apr 28, 2019

According to their README, revive is 2x faster running the same rules as golint.
So it could replace golint for a performance improvement?

I've used revive as a default linter so far without any problems.

@jirfag
Copy link
Member Author

jirfag commented Jun 9, 2019

As I understand It's faster just because of parallel execution. golangci-lint does the same parallel execution between linters (not inside golint only - but if anyone wants to run only golint in golangci-lint he can use revive/golint instead)

@johnrichardrinehart
Copy link

Will a PR be accepted which integrates revive into golangci?

@jirfag
Copy link
Member Author

jirfag commented Sep 24, 2019

@johnrichardrinehart yep, it will be accepted

@Dentrax
Copy link

Dentrax commented Mar 27, 2020

@jirfag any updates? :)

@jirfag
Copy link
Member Author

jirfag commented May 17, 2020

@Dentrax contributions are welcome. But, I hope revive uses go/analysis: we would like to include only go/analysis-based linters.

@johnrichardrinehart
Copy link

johnrichardrinehart commented May 19, 2020

@jirfag You mean this?

It seems that it's not based on x/analysis....

~ GO111MODULE=on go get github.com/mgechev/revive@3119f5881c5dd864f346426752eb745d3eddb17d
go: downloading github.com/mgechev/revive v1.0.3-0.20200517021941-3119f5881c5d
go: github.com/mgechev/revive 3119f5881c5dd864f346426752eb745d3eddb17d => v1.0.3-0.20200517021941-3119f5881c5d
go: downloading github.com/mgechev/dots v0.0.0-20190921121421-c36f7dcfbb81
go: downloading github.com/fatih/color v1.9.0
go: downloading golang.org/x/tools v0.0.0-20200515220128-d3bf790afa53
go: downloading github.com/olekukonko/tablewriter v0.0.4
go: downloading github.com/fatih/structtag v1.2.0
go: downloading github.com/mattn/go-isatty v0.0.11
go: downloading github.com/mattn/go-runewidth v0.0.7
go: downloading golang.org/x/sys v0.0.0-20191026070338-33540a1f6037

➜  ~ GO111MODULE=on go get github.com/mgechev/revive@master
go: github.com/mgechev/revive master => v1.0.3-0.20200517021941-3119f5881c5d

➜  ~ cd ~/go/pkg/mod/github.com/mgechev/revive@v1.0.3-0.20200517021941-3119f5881c5d

➜  revive@v1.0.3-0.20200517021941-3119f5881c5d go list -f '{{.Deps}}' ./... | grep analysis
➜  revive@v1.0.3-0.20200517021941-3119f5881c5d

I.e. no import "golang.org/x/tools/go/analysis" exists in any source file of github.com/mgechev/revive.

I know it's been some time, but coincidentally I have more time to pursue this, now. Please advise as to whether a PR would be considered in light of this new information.

@gonzaloserrano
Copy link

gonzaloserrano commented Jun 9, 2020

I think this is relevant to the discussion: "x/lint: freeze and deprecate" - golang/go#38968

@johnrichardrinehart
Copy link

@gonzaloserrano Thanks for referencing this.

@jirfag In light of the planned deprecation of x/lint, will you consider PRs for non-x/lint linters?

@wburningham
Copy link

Ignoring the speed differences, the biggest reason I use revive over golint is the support for "disabling a specific rule or the entire linter for a file or a range of lines". I currently have to run revive and golangci-lint (with golint disabled). Please add support for revive so I can just run one lint command.

@s3rj1k
Copy link

s3rj1k commented Dec 16, 2020

@wburningham And actually still feels that revive catches more errors than some of the similar linters in golangci-lint.

It was the case in 2018, now is probably difference of catching errors is lesser.

This all comes from my experience only :)

@ldez ldez removed the help wanted Issue that needs help from a contributor label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter: new Support new linter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants