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

revive:disable-next-line isn't working #540

Closed
kevincantu opened this issue Jul 1, 2021 · 2 comments · Fixed by #543
Closed

revive:disable-next-line isn't working #540

kevincantu opened this issue Jul 1, 2021 · 2 comments · Fixed by #543
Assignees
Labels

Comments

@kevincantu
Copy link

kevincantu commented Jul 1, 2021

Using macOS and golangci-lint v1.41.1 (a207480, 2021-06-19), I expected this to work, and not give me a linter failure:

//revive:disable-next-line:var-naming
func (source Source) BaseApiURL() string {

But actually I get this error when linting:

source.go:99:22: var-naming: method BaseApiURL should be BaseAPIURL (revive)
func (source Source) BaseApiURL() string {
                     ^
make: *** [lint] Error 1

To work around, I can use this:

func (source Source) BaseApiURL() string { //revive:disable-line:var-naming

But from how disable-next-line is described in the README, I think I've seen a bug.

@chavacava
Copy link
Collaborator

Hi @kevincantu, thanks for filling the bug description. I'll check it ASAP

@chavacava chavacava self-assigned this Jul 2, 2021
@mgechev mgechev added the bug label Jul 2, 2021
chavacava added a commit that referenced this issue Jul 10, 2021
@chavacava
Copy link
Collaborator

chavacava commented Jul 10, 2021

Hi @kevincantu, can you please check if the PR #543 fixes the problem?

I've detected the following bug: when the disable annotation was not the first line in the comment block, the annotation was ignored. Example:

// BaseApiURL yields ...
//revive:disable-next-line:var-naming   <- this annotation was ignored
func (source Source) BaseApiURL() string {}

The example you provided in the issue is something that should work without this fix, i.e. I was not able to reproduce the bug with the exact code example you provided.

mgechev pushed a commit that referenced this issue Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants