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

Idea: Allow linters to be "warn only" #708

Closed
Luzifer opened this issue Sep 17, 2019 · 15 comments
Closed

Idea: Allow linters to be "warn only" #708

Luzifer opened this issue Sep 17, 2019 · 15 comments
Labels
stale No recent correspondence or work activity won't fix This will not be worked on

Comments

@Luzifer
Copy link

Luzifer commented Sep 17, 2019

After godox linter was added I'm having an error which is not yet fixable in my code:

main.go:58  godox  main.go:58: Line contains TODO/BUG/FIXME: "TODO(kahlers): Replace with proper limit..."

Seeing this TODO comment in the lint is great because it reminds me I need to do something about that. Sadly the issue was prioritized as "lets do this later" as it is not a business critical issue.

Therefore having an option to set (for example) godox as a warn only linter which reminds me I need to do something but does not fail the linter (if there are only warn only errors left, exit with status 0 instead of 1) would be great.

Sure I also could just add a //nolint:godox but that would also make the output vanish…

@jirfag jirfag added the enhancement New feature or improvement label Sep 24, 2019
@snargleplax
Copy link

You could also perform two separate runs with separate configuration files, and treat the results however you want. That would allow you more flexibility, such as having both warning and error settings for a given linter. At the cost, of course, of having two configs to maintain.

@tpounds tpounds added area: config Related to .golangci.yml and/or cli options help wanted Issue that needs help from a contributor labels Oct 4, 2019
@tpounds
Copy link
Contributor

tpounds commented Oct 4, 2019

@Luzifer Thanks for the feature request! I agree that this sounds useful and could be exposed in a few ways such as a flag to --show-excluded-errors or as part of a verbose setting --verbose=N. Thoughts?

/cc @jirfag

@jirfag
Copy link
Member

jirfag commented Oct 8, 2019

I don't believe in warnings as a part of production process (CI, etc). It's a nice thing to see warnings sometimes. But a stable process can be based only on failure if there are any issues. I agree with @snargleplax that you can run golangci-lint just twice: godox should be very fast linter (as I remember it needs only AST) so two runs will be equal by time to one run.

@fho
Copy link

fho commented Oct 14, 2019

We have a similar need.
We have a monorepository with a lot of applications and libraries.
Sometimes it's necessary to deprecate a method but not possible to rework all code at once in the repository not use the deprecated code anymore.

Currently golangci-lint will fail because of the deprecation warnings. To allow deprecations and not fail CI, we currently can only disable the whole linter for the line (// nolint:staticcheck)
This is not a good solution because it also suppresses all other findings from the linter.

It would be great to be able to configure similar to the issues rules, issues that are only warnings and won't result in failed check.

Running golangci-lint twice is not a practical solution:

  • it's does not allow to only consider specific issues from a linter as non-critical (e.g. only use of deprecated code issues), it requires to write a script to process the output of golangci-lint and react as needed
  • Especial on big codebases it will increase the check time a lot

@snargleplax
Copy link

@fho Hushing specific issues within a linter seems like a separable thing, larger than what the OP was after here. My $0.02 would be not to lump it in here, related though it may be.

@Luzifer
Copy link
Author

Luzifer commented Oct 14, 2019

@snargleplax to me it sounds like the exact same use-case. We both want to have certain linter errors handled as "warnings" (displayed but not breaking the build). Sure, my current use-case is not deprecating methods but allowing soft-migrations but in the end that doesn't differ that much.

@jirfag
Copy link
Member

jirfag commented Oct 14, 2019

  1. if you would like to ignore (== don't fail CI build) only some errors from a linter use exclude or exclude-rules. I think you don't really need to display these excluded issues. But if you wish you can run golangci-lint with -v to see them. Or we can add an additional debug flag to only print them.
  2. if you see the case when running golangci-lint twice is slow please make an issue for that. It will improve incremental analysis.

@jirfag jirfag added won't fix This will not be worked on and removed area: config Related to .golangci.yml and/or cli options enhancement New feature or improvement help wanted Issue that needs help from a contributor labels Oct 14, 2019
@snargleplax
Copy link

@Luzifer: It's not identical based on the issue as written. Ignoring entire linters is different (and simpler) than adding the ability to ignore specific issues within a linter.

@stale
Copy link

stale bot commented Apr 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Apr 11, 2020
@stale stale bot closed this as completed May 11, 2020
@jufemaiz
Copy link

Can this be reopened or is there a more appropriate location to continue the discussion.

The use of levels (for example, error, warning) is used widely in other linting tools for good reasons (many outlined above):

  • rubocop
  • eslint
  • pylint

@bombsimon
Copy link
Member

Support to differentiate between error and warning was added in #1155 since this question was also discussed in #127. Is there something else you need @jufemaiz or did you just not know about this?

@jufemaiz
Copy link

Thanks for that @bombsimon ! I did trawl but evidently my mojo wasn't so good (neither tickets were previously mentioned here).

Will see how we roll with it! Thanks!

@jufemaiz
Copy link

@bombsimon QQ on follow up, we've configured info level for some linters, however, the CLI is still returning a non-0 value if they are hit. Obviously this has some issues for CICD.

golangci-lint has version 1.39.0 built from 9aea4aee on 2021-03-26T08:02:53Z

cc: @lawko95

@doron-cohen
Copy link

@jufemaiz is there any solution for this by now? I set some rules to warning severity but still fail in the CI on them. I really don't want to ignore them. How is this solved?

@jufemaiz
Copy link

@doron-cohen worthwhile joining in the discussion @ #1981

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale No recent correspondence or work activity won't fix This will not be worked on
Projects
None yet
Development

No branches or pull requests

8 participants