Skip to content

feat(extractor): recognise File.Open*/Create* factory acquires → handoff leak arm (recall 7/10→8/10)#54

Merged
PhysShell merged 1 commit into
mainfrom
claude/factory-acquire
Jun 20, 2026
Merged

feat(extractor): recognise File.Open*/Create* factory acquires → handoff leak arm (recall 7/10→8/10)#54
PhysShell merged 1 commit into
mainfrom
claude/factory-acquire

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 20, 2026

Copy link
Copy Markdown
Owner

What

Teaches the extractor that an owned disposable can be born from a factory, not just new — corpus recall 7/10 → 8/10.

The flow / candidate passes only ever treated new X() as acquiring an owned disposable. So a stream opened by a factory — var s = File.OpenRead(path) — was invisible, and the leak arm of ownership-handoff-consume (a stream neither disposed nor handed off → a real OWN001) scored a miss purely for that reason.

New IsOwningFactory recognises ownership-transferring factory calls off the resolved symbol against a curated System.IO.File set (OpenRead/OpenWrite/Open/Create/OpenText/CreateText/AppendText → a fresh FileStream/StreamReader/StreamWriter the caller owns exactly as if it had new'd one). It is wired into the same two acquire points as new/pool:

  • the flow LowerFlowStmt acquire, and
  • the --flow-locals candidate loop.

All the existing escape / using / dispose logic then applies unchanged, so a factory stream that is returned, passed to a consumer, using-guarded or disposed stays silent — only an actually-leaked one is flagged.

The proof (benchmark, CI)

ownership-handoff-consume's Leak (var s = File.OpenRead(path); …; // never disposed) now fires OWN001; the fix's using var s = … stays clean. The case flips missed → caught: recall 7/10 → 8/10, specificity stays 10/10, 0 FP, CI floor --min-recall 8.

Precision over recall, by construction:

  • curated + symbol-resolved — a borrowed/cached disposable handed back by some other API is never mistaken for an owned acquire;
  • the blast radius is exactly one fileFile.* factory calls appear nowhere else in the corpus or samples (verified by grep), so no after.cs and no dog-food scan can newly cry wolf;
  • System.IO.File resolves from the compilation's trusted-platform references (it lives in System.Private.CoreLib), so single-file benchmark scans bind it.

case.own is unchanged (test_corpus 6/6); the extractor change has no local .NET SDK, so it is validated by the corpus-benchmark + wpf-extractor CI jobs.

Honest scope

This catches the leak arm (OWN001). The use-after-handoff arm (RunOWN002 — the stream is touched after Archive(s) consumed it) is deliberately left as the next slice: it needs the inter-procedural consume contract (recognise that Archive disposes its by-value parameter, then model Archive(s) as a release of s, like Rust's move — the cut is the signature, no whole-program points-to). Documented in the case notes.md and P-012.

Files

  • frontend/roslyn/OwnSharp.Extractor/Program.csIsOwningFactory + the two acquire-point wirings.
  • .github/workflows/ci.yml — floor --min-recall 8.
  • docs/notes/corpus-benchmark.md (the → 8/10 ratchet) + docs/proposals/P-012-bug-corpus-mining.md.
  • corpus/real-world/ownership-handoff-consume/notes.md — extractor-status note (leak caught; handoff arm future).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED


Generated by Claude Code

Summary by CodeRabbit

  • Improvements

    • Enhanced detection for factory-based resource acquisition patterns
    • Benchmark recall improved from 7/10 to 8/10
  • Documentation

    • Updated corpus benchmark notes with improved detection metrics
    • Expanded documentation on resource acquisition patterns and remaining coverage gaps

… -> 8/10)

The flow/candidate passes only ever treated `new X()` as ACQUIRING an owned
disposable, so a stream opened by a factory -- `var s = File.OpenRead(path)` --
was invisible. The leak arm of ownership-handoff-consume (a stream neither
disposed nor handed off -> a real OWN001) scored a miss purely for that reason.

New IsOwningFactory recognises ownership-transferring factory calls off the
resolved SYMBOL against a curated System.IO.File set (Open*/Create*/*Text -> a
fresh FileStream/StreamReader/StreamWriter the caller owns exactly as if it had
`new`'d one). Wired into the same two acquire points as `new`/pool: the flow
LowerFlowStmt acquire and the --flow-locals candidate loop. All the existing
escape/using/dispose logic then applies unchanged, so a factory stream that is
returned, passed to a consumer, `using`-guarded or disposed stays silent.

Effect: ownership-handoff-consume's Leak fires OWN001 (the `using var` fix stays
clean), flipping the case to caught -- recall 7/10 -> 8/10, specificity 10/10,
0 FP, CI floor --min-recall 8. Curated + symbol-resolved = precision over recall;
the blast radius is exactly one file (nothing else scanned opens a File.* stream).

The use-after-handoff arm (Run -> OWN002) stays extractor-future: it needs the
inter-procedural consume contract (Archive disposes its by-value param -> the
call is a release of the argument). Documented in the case notes + P-012.

case.own unchanged (test_corpus 6/6); the extractor change is validated by the
corpus-benchmark + wpf-extractor CI jobs (no local .NET SDK).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b48ed1ab-d001-4898-ac8a-060a1b7106f1

📥 Commits

Reviewing files that changed from the base of the PR and between 9ede9b0 and 286e573.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • corpus/real-world/ownership-handoff-consume/notes.md
  • docs/notes/corpus-benchmark.md
  • docs/proposals/P-012-bug-corpus-mining.md
  • frontend/roslyn/OwnSharp.Extractor/Program.cs

📝 Walkthrough

Walkthrough

Adds IsOwningFactory to OwnSharp.Extractor/Program.cs to recognize System.IO.File.Open*/Create* factory calls as ownership acquire sites in --flow-locals lowering. The CI corpus-benchmark recall gate is raised from 7 to 8, and benchmark notes, the P-012 proposal, and corpus case documentation are updated to reflect the new 8/10 score.

Changes

Factory acquire recognition and benchmark ratchet

Layer / File(s) Summary
IsOwningFactory helper and flow-locals integration
frontend/roslyn/OwnSharp.Extractor/Program.cs
Introduces IsOwningFactory(ExpressionSyntax?, SemanticModel) that resolves the invoked method symbol and returns true for System.IO.File.Open*/Create*/AppendText calls. Wires it into LowerFlowStmt to emit acquire nodes and into the --flow-locals candidate selection loop to track those locals as owned IDisposables.
CI gate tightening and benchmark docs
.github/workflows/ci.yml, docs/notes/corpus-benchmark.md, docs/proposals/P-012-bug-corpus-mining.md, corpus/real-world/ownership-handoff-consume/notes.md
Raises --min-recall from 7 to 8 in the CI benchmark job. Updates the corpus-benchmark notes heading to "8/10 (four ratchets)", expands the factory-acquires section, and rewrites remaining-gaps text. Updates P-012 status from 7/10 to 8/10 and rewrites gaps. Expands ownership-handoff-consume notes with extractor status for the OWN001 (caught) and OWN002 (pending inter-procedural consume contract) arms.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PhysShell/Own.NET#15: Introduced the --flow-locals lowering mode that this PR extends with IsOwningFactory as a new acquire-site recognizer.
  • PhysShell/Own.NET#53: Also extended --flow-locals tracked-local acquire detection (via ArrayPool<T>.Rent semantic resolution), following the same pattern as IsOwningFactory here.
  • PhysShell/Own.NET#50: Introduced the corpus-benchmark CI job and --min-recall gate that this PR ratchets upward.

Poem

🐇 Hop! A factory opens a stream,
File.OpenRead joins the acquire team.
The ratchet clicks — eight out of ten!
OWN001 fires correctly, and then…
Two gaps remain, but the bunny takes note,
Four ratchets earned — time to celebrate and gloat! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: recognizing File factory methods as acquire sites, enabling the leak detection arm to improve recall from 7/10 to 8/10.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/factory-acquire

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants