feat: add SuggestedFixes for --fix support#19
Conversation
Attach SuggestedFixes to diagnostics so golangci-lint --fix can automatically convert value receivers to pointer receivers. Switch tests to RunWithSuggestedFixes and add golden files.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@analyzer.go`:
- Around line 105-112: The call pass.Pkg.Scope().Lookup(recv) can return nil and
calling .Pos() would panic; update the diagnostic creation in the block that
calls pass.Report so you first capture the result of
pass.Pkg.Scope().Lookup(recv) into a variable, check for nil, and use a safe
fallback position (for example token.NoPos or the position of the first related
method) when lookup is nil before passing Pos into analysis.Diagnostic; keep the
same Message and SuggestedFixes logic but supply the guarded position to avoid
nil dereference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 833209dd-ae10-4d4a-9520-d22f79cd0e2e
📒 Files selected for processing (9)
analyzer.goanalyzer_test.gotestdata/src/basic/rpc.go.goldentestdata/src/disablebuiltin/binary.go.goldentestdata/src/disablebuiltin/gob.go.goldentestdata/src/disablebuiltin/json.go.goldentestdata/src/disablebuiltin/text.go.goldentestdata/src/disablebuiltin/xml.go.goldentestdata/src/disablebuiltin/yaml.go.golden
|
@ldez it should be the final enhancement for the next release, then I’ll open a PR to upgrade the linter version in golangci-lint. |
|
I can see 2 problems:
Before, the goal was to have consistent receiver types, after, the goal is to force the usage of pointer receiver when there is at least one pointer receiver. |
|
@ldez thanks for the review. I want to clarify two points. About the report message: it didn’t change. the methods of %q use pointer receiver and non-pointer receiver.The only added field is About the fix direction: yes, the suggested fix chooses pointer receivers. I chose that direction because it is the safer automatic edit. Changing pointer receivers to value receivers can change mutation/copy/nil semantics, while changing value receivers to pointer receivers is the smaller local fix when this linter reports mixed receivers. This also follows Go’s receiver guidance: “Don’t mix receiver types” and “Finally, when in doubt, use a pointer receiver”. That said, I understand this can be too opinionated for If you still think this should not be included, I’ll leave this PR open, release the current version first, and update recvcheck in golangci-lint. If this direction looks acceptable, I’ll include this PR in the next recvcheck release. |
|
I think this is too opinionated, I understand the goal, but I also know that using pointer receiver is not always the right solution. A suggested fix is not a simple report message, it will change the code, the impact is important. |
FYI, we use dependabot to update the linters, you should not open a PR to update the linter. |
|
@ldez makes sense. I’ll keep this out of the next recvcheck release and leave it open for now. Thanks for the golangci-lint note too — I’ll let Dependabot handle the update. |
2 participants
Why
golangci-lint supports
--fixto auto-fix linter issues, but recvcheck only reports diagnostics without suggested fixes. Users have to manually change value receivers to pointer receivers.What
Attach
analysis.SuggestedFixwithTextEdits that insert*before value receiver type names. Whengolangci-lint --fixis used, value receivers are automatically converted to pointer receivers.Changes
pass.Reportf→pass.Report(analysis.Diagnostic{...})withSuggestedFixes*ast.Identpositions during AST walkanalysistest.RunWithSuggestedFixeswith.go.goldenfilesSummary by CodeRabbit
New Features
Tests