Skip to content

fix: add scope-aware filtering to P1 effect detection (#121, #144, #145)#149

Merged
trevor-vaughan merged 1 commit into
unbound-force:mainfrom
jflowers:opsx/p1-local-var-false-positives
Jun 18, 2026
Merged

fix: add scope-aware filtering to P1 effect detection (#121, #144, #145)#149
trevor-vaughan merged 1 commit into
unbound-force:mainfrom
jflowers:opsx/p1-local-var-false-positives

Conversation

@jflowers

Copy link
Copy Markdown
Collaborator

Summary

Eliminates false positive P1 side effects on locally-created maps, slices,
and channels. The root cause: detectAssignEffects, detectSendEffects,
and detectP1CallEffects checked type (is this a map/slice/channel?) but
not scope (is this variable externally observable?).

Changes

  • Added isExternallyObservable helper using types.Object pointer
    identity to distinguish parameters/receivers/named-returns from
    body-local variables. Uses collectSignatureVars to build the
    signature-level variable set via info.Defs.
  • Gated MapMutation, SliceMutation, ChannelSend, and ChannelClose
    emission behind the scope check.
  • Threaded info *types.Info to detectSendEffects for scope resolution.
  • Added 6 test fixtures (LocalMapWrite, LocalSliceWrite, LocalChannelSend,
    LocalChannelClose, NamedReturnMapWrite, WriteToStructMap) and 6
    corresponding tests with full assertion depth (effect count, type,
    tier, description).

Known limitations

  • Slice aliasing (local slice sub-sliced and returned) — documented
  • Closure capture (local variable captured by returned closure) — documented
  • Channel passed to goroutine — documented

Testing

  • 15/15 P1 effects tests pass (9 existing + 6 new)
  • All 12 packages pass: go test -race -count=1 -short ./...
  • No new lint violations

Closes #121, #144, #145

, unbound-force#144, unbound-force#145)

- Add isExternallyObservable helper using types.Object pointer identity
  to distinguish parameters/receivers/named-returns from body-local
  variables
- Gate MapMutation, SliceMutation, ChannelSend, ChannelClose emission
  behind scope check — local variables no longer produce false positives
- Add collectSignatureVars to build signature-level variable set from
  info.Defs
- Thread info *types.Info to detectSendEffects for scope resolution
- Add 6 test fixtures and 6 tests covering local suppression, named
  return detection, and receiver field access

@yvonnedevlinrh yvonnedevlinrh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Only minor findings in relation to scope and design doc divergence. Updating the design doc can be handled in a separate PR #151

@trevor-vaughan trevor-vaughan merged commit e78f048 into unbound-force:main Jun 18, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

fix: P1 MapMutation false positives on local map variables

3 participants