Skip to content

fix: track nil flows through function variables#388

Open
yd-beep wants to merge 1 commit intouber-go:mainfrom
yd-beep:fix-func-variable
Open

fix: track nil flows through function variables#388
yd-beep wants to merge 1 commit intouber-go:mainfrom
yd-beep:fix-func-variable

Conversation

@yd-beep
Copy link

@yd-beep yd-beep commented Jan 14, 2026

Previously, nilaway would skip nil analysis for calls made through function variables (e.g., var ProcessFunc = process; ProcessFunc(nil)), treating them as trusted non-nil. This caused false negatives where nil pointer dereferences were not detected.

This change adds support for resolving function variables to their underlying function declarations, enabling proper nil flow tracking through both return values and arguments.

Changes:

  • Add resolveFuncVariable() helper to resolve function variables to their underlying *types.Func for both package-level declarations (var F = f) and local assignments (f := someFunc)
  • Update ParseExprAsProducer() to use resolved functions for return value analysis when calling through function variables
  • Update AddComputation() to use resolved functions for argument consumption tracking when calling through function variables
  • Add getFuncReturnProducersForFunc() to support direct *types.Func usage without requiring an *ast.Ident
  • Add integration tests for function variable nil tracking

If resolution fails (dynamic assignment, closures, etc.), the code falls back to the previous trusted behavior to avoid false positives.

Fixes #387

Previously, nilaway would skip nil analysis for calls made through
function variables (e.g., `var ProcessFunc = process; ProcessFunc(nil)`),
treating them as trusted non-nil. This caused false negatives where nil
pointer dereferences were not detected.

This change adds support for resolving function variables to their
underlying function declarations, enabling proper nil flow tracking
through both return values and arguments.

Changes:
- Add resolveFuncVariable() helper to resolve function variables to
  their underlying *types.Func for both package-level declarations
  (var F = f) and local assignments (f := someFunc)
- Update ParseExprAsProducer() to use resolved functions for return
  value analysis when calling through function variables
- Update AddComputation() to use resolved functions for argument
  consumption tracking when calling through function variables
- Add getFuncReturnProducersForFunc() to support direct *types.Func
  usage without requiring an *ast.Ident
- Add integration tests for function variable nil tracking

If resolution fails (dynamic assignment, closures, etc.), the code
falls back to the previous trusted behavior to avoid false positives.

Fixes uber-go#387
@CLAassistant
Copy link

CLAassistant commented Jan 14, 2026

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.01%. Comparing base (89df5f7) to head (4befe05).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...tion/function/assertiontree/root_assertion_node.go 85.71% 2 Missing and 2 partials ⚠️
...tion/function/assertiontree/parse_expr_producer.go 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #388      +/-   ##
==========================================
- Coverage   87.01%   87.01%   -0.01%     
==========================================
  Files          73       73              
  Lines        8305     8340      +35     
==========================================
+ Hits         7227     7257      +30     
- Misses        885      888       +3     
- Partials      193      195       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

False negative: nil dereference not detected when function is called through a function variable

2 participants