Fix issue #277: proper rich check effect support for function variables#401
Open
nickita-khylkouski wants to merge 4 commits intouber-go:mainfrom
Open
Fix issue #277: proper rich check effect support for function variables#401nickita-khylkouski wants to merge 4 commits intouber-go:mainfrom
nickita-khylkouski wants to merge 4 commits intouber-go:mainfrom
Conversation
… effect processing For issue uber-go#277, function pointers with error-returning or ok-returning signatures should trigger rich check effects, but the existing code suppressed all function variable calls as TrustedFuncNonnil. This change checks the function signature and only applies the non-nil workaround for non-error/non-ok-returning function variables, allowing error/ok-returning function variables to generate proper rich check effects. Fixes uber-go#277
…ction variables
This commit partially fixes the false positives for function variables with
error/ok return signatures by distinguishing them from other function variables.
Changes:
- Modified parse_expr_producer.go to check if function variables are
error-returning or ok-returning using typeshelper.GetFuncSignature
- For error/ok-returning function variables, generate simple producers
using TrustedFuncNilable/TrustedFuncNonnil
- For other function variables, keep the TrustedFuncNonnil{:never} workaround
Limitations:
- Rich check effects are not properly implemented for function variables yet
- True negatives (unchecked error/ok returns) are still not detected
- This is a temporary workaround until proper function variable support is added
This fixes some false positives but doesn't fully resolve issue uber-go#277.
TODO: Implement proper rich check effect support for function variables so that
checked cases (if err != nil) don't warn and unchecked cases do warn.
…turning function variables - Modified parse_expr_producer.go to distinguish between error/ok-returning and other function variables - For non-error/ok function variables: continue using TrustedFuncNonnil to suppress false positives - For error/ok function variables: generate specialized producers (TrustedFuncNilable/Tautology) to enable tracking - Updated both *ast.Ident and *ast.SelectorExpr cases to handle f() and s.f() patterns - Added comprehensive TODO comments explaining limitation: proper FuncReturn producers require creating synthetic *types.Func or modifying annotation key system - Test status: basic producer generation works, but requires further work for full integration with rich check effects
…variables
This commit fixes the false positive where function variables (e.g., s.f()
where f is a function field) were incorrectly flagged as nil when the
error was properly checked.
Changes:
- Add FuncVarRetAnnotationKey: new annotation key type for function variable
returns that doesn't require a *types.Func declaration
- Add getFuncVarReturnProducers helper: generates proper FuncReturn producers
with NeedsGuard and IsFromRichCheckEffectFunc flags
- Update FuncReturn.Prestring: handle FuncVarRetAnnotationKey
- Update primitivizer.site: handle keys with nil Object()
The fix ensures:
1. When error is checked (if err != nil { return }), no warning is reported
2. When error is ignored (v, _ := f()), a warning is correctly reported
Fixes uber-go#277
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes the false positive where function variables (e.g.,
s.f()wherefis a function field) were incorrectly flagged as nil when the error was properly checked.Before:
v, err := s.f(); if err != nil { return }; _ = *v→ false positive warningAfter: No warning (error is checked)
Changes
FuncVarRetAnnotationKey: New annotation key type for function variable returns that doesn't require a*types.FuncdeclarationgetFuncVarReturnProducershelper: Generates properFuncReturnproducers withNeedsGuardandIsFromRichCheckEffectFuncflags for rich check effect integrationFuncReturn.Prestring: Handle the newFuncVarRetAnnotationKeytypeprimitivizer.site: Handle keys with nilObject()for local-only annotationsTest plan
issue277_function_pointer.gocovers:Fixes #277
🤖 Generated with Claude Code