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

Example of a comment to disable a check at one call-site? #248

Open
thockin opened this issue Jul 6, 2021 · 8 comments
Open

Example of a comment to disable a check at one call-site? #248

thockin opened this issue Jul 6, 2021 · 8 comments

Comments

@thockin
Copy link

thockin commented Jul 6, 2021

Many/most linters have some sort of way to disable a specific check at a specific call-site. Are there examples on how to do this in ruleguard?

@quasilyte
Copy link
Owner

quasilyte commented Jul 7, 2021

Hello!

ruleguard binary is not that well suited for the CI integration. It's mostly a tool that helps you to develop and debug the linting rules.

On the bright side, there is golangci-lint tool that embeds ruleguard (through gocritic linter). Here is an example of how you can enable it there: https://quasilyte.dev/blog/post/ruleguard/#using-from-the-golangci-lint

Note: golangci-lint will probably have some previous stable release available; so there is some update lag in there.

go-critic also does that: https://github.com/go-critic/go-critic/blob/master/.golangci.yml#L44

It could be possible to implement more user-oriented features in ruleguard itself, but if we look at the main file, it becomes apparent that we're trying to be as thin analysis-based tool as possible:

https://github.com/quasilyte/go-ruleguard/blob/master/cmd/ruleguard/main.go

There is a reason to do it this way.

ruleguard can be used as a library to build your own rule runners. For instance, go-critic embeds ruleguard as a library: https://github.com/go-critic/go-critic/blob/master/checkers/ruleguard_checker.go (without using the go-ruleguard/analyzer package).

If go/analysis provides some easy way to achieve that "ignore" behavior, maybe we can use that.

In case you're not using the golangci-lint or can't use it, I wonder if it's possible to use an external warnings filter.

@thockin
Copy link
Author

thockin commented Jul 7, 2021

Using it thru golangci-lint, I still don't see a way to //nolint a single ruleguard rule?

@mislav
Copy link

mislav commented Jul 28, 2021

Through golangci-lint I've found that you can do //nolint:gocritic to disable ruleguard for a specific line, but of course, this disables all ruleguard reporting for that line. I did not find a way to disable only a specific ruleguard rule by name.

Furthermore, the nolintlint golangci-linter (which reports unused //nolint directives) sometimes complains that specific //nolint:gocritic directives were not used by gocritic, even if said line would fail without the directive. Such false positives would go away after golangci-lint cache clean.

@quasilyte
Copy link
Owner

Does golangci-lint have any linters that can have a special comment like //nolint:lintername diagnostic-name?
If it does, it should be possible to add //nolint:gocritic ruleguard/something.

@mislav
Copy link

mislav commented Aug 16, 2021

@quasilyte I don't think so. golangci-lint only supports //nolint:<linter> // <description> statements, where description is meant to be human-readable and can't specify rules to disable, to my knowledge. golangci/golangci-lint#741

@quasilyte
Copy link
Owner

quasilyte commented Aug 16, 2021

Maybe we need to take a look at the specific rules then and try to make them smarter, so they have fewer false positives.

Although I agree that it's unfortunate that we can't get this behavior via nolint directives.

CC @ernado what do you think about additional nolint syntax? //nolint:<linter>:<diag>
It would be useful for multi-diagnostics linters like staticcheck and gocritic.

For ruleguard, I think it could be //nolint:gocritic:ruleguard/rule_name (using the same format, but with / to handle the namespacing). In theory, it's possible to register rules as first-class diagnostics, so it becomes //nolint:gocritic:rule_name. Note that we already do that with embedded ruleguard rules inside gocritic (we're registering them as normal checkers).

@thockin
Copy link
Author

thockin commented Aug 16, 2021 via email

@quasilyte
Copy link
Owner

I can see the point, but we'll still need something like //nolint:ruleguard:<rule_name> if we want to have a granularity of a single named rule.

@komuw komuw mentioned this issue Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants