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

fix: filter invalid issues before other processors #4552

Merged
merged 4 commits into from Mar 20, 2024

Conversation

ldez
Copy link
Member

@ldez ldez commented Mar 20, 2024

Before:

$ golangci-lint run 
WARN [runner] Can't process result by autogenerated_exclude processor: can't filter issue result.Issue{FromLinter:"contextcheck", Text:"Function `NewWriteLock$1` should pass the context parameter", Severity:"", SourceLines:[]string(nil), Replacement:(*result.Replacement)(nil), Pkg:(*packages.Package)(0xc00012c480), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"", Offset:0, Line:0, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""}: no file path for issue 
WARN [runner] Can't process result by nolint processor: can't filter issue result.Issue{FromLinter:"contextcheck", Text:"Function `NewWriteLock$1` should pass the context parameter", Severity:"", SourceLines:[]string(nil), Replacement:(*result.Replacement)(nil), Pkg:(*packages.Package)(0xc00012c480), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"", Offset:0, Line:0, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""}: no file path for issue 
WARN [runner/source_code] Failed to get line 0 for file : failed to get file  lines cache: can't get file  bytes from cache: can't read file : open : no such file or directory 
:0: Function `NewWriteLock$1` should pass the context parameter (contextcheck)

With this PR:

$ ./golangci-lint run
WARN [runner/invalid_issue] no file path for the issue: probably a bug inside the linter "contextcheck": &result.Issue{FromLinter:"contextcheck", Text:"Function `NewWriteLock$1` should pass the context parameter", Severity:"", SourceLines:[]string(nil), Replacement:(*result.Replacement)(nil), Pkg:(*packages.Package)(0xc000be4480), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"", Offset:0, Line:0, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""}

The previous behavior was causing a side effect on the Nolint processor.

EDIT: the invalid issues reported by contextcheck will not produce a warning because it's a known bug.

Fixes #4550


Sponsoring is a good way to sustain open source maintainers: sponsor me

@ldez ldez added bug Something isn't working area: processors labels Mar 20, 2024
@ldez ldez added this to the next milestone Mar 20, 2024
@ldez
Copy link
Member Author

ldez commented Mar 20, 2024

For the details:
Previously inside the AutogeneratedExclude processor, this issue.FilePath() == "" was evaluated after this filepath.Ext(issue.FilePath()) == ".go" but it's impossible for a file path to be xx.go and empty at the same time.

When I added the new option for the AutogeneratedExclude processor, I fixed the behavior.

The problem is that the AutogeneratedExclude processor was excluding reports not related to generated files.

And so welcome to side effects.

@ldez ldez merged commit cd890db into golangci:master Mar 20, 2024
13 checks passed
@ldez ldez deleted the fix/report-without-filepath branch March 20, 2024 16:25
@ldez ldez modified the milestones: next, v1.57 Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: processors bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgraded to 1.57.0, all my "nolint" comments are not working (contextcheck)
2 participants