feat(suppression): implement [OwnIgnore("reason")] end to end (P-004) — Closes #209#215
Conversation
Ships the per-site suppression attribute the FP-policy page marked
"designed, not implemented". A `[OwnIgnore("reason")]` on an IDisposable field
now suppresses its OWN001 — but with VISIBILITY OVER SILENCE: the finding is
still minted, kept out of the exit code and the human findings stream, yet
COUNTED (a run-summary tally) and carried in SARIF `suppressions`
(kind "inSource", the reason as justification). A suppressed finding never
fails the run.
The reason is mandatory by design (a suppression is a documented decision):
a reason-less `[OwnIgnore]` or an empty `[OwnIgnore("")]` does NOT suppress —
never a silent accept.
Consumed core-side (P-013 "one checker"): the extractor emits the finding fact
with an additive-optional `ignore_reason` marker; the core is the sole authority
on the verdict. No OWNIR_VERSION bump (additive optional, mirrors
source_provenance).
- Extractor: OwnIgnoreReason() reads `[OwnIgnore]` (matched by simple name, so a
project may declare its own attribute) on IDisposable field declarations via the
SemanticModel's constant folding; stamps `ignore_reason` only for a non-empty reason.
- Core: Finding.ignore_reason + `suppressed` property; check_facts stamps it;
cmd_ownir excludes suppressed from the exit code and prints a tally; SARIF emits
the `suppressions` array; load() validates the field type; dedup key updated.
- Sample OwnIgnoreSample.cs: unsuppressed / suppressed / reason-less / empty-reason
contrast, wired into the C# leak-extractor CI job with a SARIF suppressions assertion.
- Pinned in tests/test_ownir.py; documented in spec/OwnIR.md §4 + ownir.schema.json;
docs/suppression-and-fp-policy.md flipped to shipped (P-015 config kept draft).
Locally validated (real extractor + core): suppressed leak silent-but-counted,
present in SARIF with inSource justification; reason-less/empty fire OWN001;
a suppressed-only run exits 0; non-string ignore_reason fails loud at load.
Gates: run_tests.py, ruff, mypy --strict on ownlang all green.
Closes #209
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds end-to-end ChangesInline OwnIgnore suppression
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant RoslynExtractor
participant OwnIR
participant CLI
participant SARIF
RoslynExtractor->>OwnIR: Emit disposable-field fact with ignore_reason
OwnIR->>CLI: Create active or suppressed Finding
CLI->>SARIF: Include suppressed finding
SARIF-->>CLI: Store inSource justification
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://cdn.github.com/in/57789?s=60&v=4){kind=small}
| // CONTROL: a `new`'d IDisposable field never disposed -> OWN001 fires. | ||
| public sealed class UnsuppressedLeak | ||
| { | ||
| private readonly Handle _h = new Handle(); |
| public sealed class SuppressedLeak | ||
| { | ||
| [OwnIgnore("owned and disposed by the DI container, not by this type")] | ||
| private readonly Handle _h = new Handle(); |
| public sealed class ReasonlessLeak | ||
| { | ||
| [OwnIgnore] | ||
| private readonly Handle _h = new Handle(); |
| public sealed class EmptyReasonLeak | ||
| { | ||
| [OwnIgnore("")] | ||
| private readonly Handle _h = new Handle(); |
|
@coderabbitai review |
![coderabbitai[bot]](https://cdn.github.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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 747-754: Replace the blanket `|| true 2>/dev/null` on the `ownlang
ownir` SARIF generation with the established `rc`-capture pattern used elsewhere
in the workflow, such as around line 1228. Preserve the generated SARIF and
allow the expected nonzero result, but capture the command’s exit status and
print stderr or an explicit error when the status is 2 or greater before running
the existing jq validation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 78b21709-fd67-4bfd-a9ae-b8b4fa5109a0
📒 Files selected for processing (9)
.github/workflows/ci.ymldocs/suppression-and-fp-policy.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/samples/OwnIgnoreSample.csownlang/__main__.pyownlang/ownir.pyspec/OwnIR.mdspec/ownir.schema.jsontests/test_ownir.py
…abbit) Blanket `|| true` + 2>/dev/null swallowed a real ownir error (rc >= 2): the jq assertion would still fail the job, but the log showed only the assertion text, not the Python error. Mirror the established rc-capture pattern (same file, proj.sarif step): keep stderr, allow the expected rc<=1, fail loud on rc>=2. Per the PR #211 learning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
Both sides appended to the same wpf-extractor job tail: main's #215 [OwnIgnore] block (sample, assertions, SARIF rc-capture step) and this branch's #218 rotation block (sample, silent + three flagged controls). Kept both, single merged OK line. Program.cs auto-merged (additive). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
3 participants
Что и зачем
Реализует per-site suppression
[OwnIgnore("reason")], которуюdocs/suppression-and-fp-policy.mdчестно помечала как designed-not-implemented (P-004).[OwnIgnore("reason")]наIDisposable-поле теперь глушит его OWN001 — но по принципу visibility over silence: находка всё равно создаётся, убирается из exit-кода и человекочитаемого потока, но считается (tally в summary) и уносится в SARIFsuppressions(kind: "inSource", reason какjustification). Suppressed-находка никогда не валит прогон. Reason обязателен by design:[OwnIgnore]без причины и пустой[OwnIgnore("")]НЕ глушат — never a silent accept.Consumed core-side (P-013 «one checker»): экстрактор эмитит факт с additive-optional маркером
ignore_reason, а вердикт — за ядром.OWNIR_VERSIONне бампается (additive optional, зеркалитsource_provenance).Тип изменения
Как проверено
python tests/run_tests.py— exit 0 (в т.ч.test_ownir261/261 с новыми пинами)ruff check .иmypy(strict наownlang/) — clean / Success (20 files)Связанные issue
Closes #209.
Что внутри
Program.cs):OwnIgnoreReason()читает[OwnIgnore](матч по simple-name — проект может завести свой атрибут) на объявленияхIDisposable-полей через constant-folding SemanticModel; стампитignore_reasonтолько для непустой причины.Finding.ignore_reason+ свойствоsuppressed;check_factsстампит его;cmd_ownirисключает suppressed из exit-кода и печатает tally;_sarif_resultэмититsuppressions;load()валидирует тип; dedup-ключ обновлён.OwnIgnoreSample.cs: контраст unsuppressed / suppressed / reason-less / empty-reason, подключён в CI-джобу C# leak extractor + ассерт на SARIFsuppressions.tests/test_ownir.py;spec/OwnIR.md §4+spec/ownir.schema.json(additive optional, какsource_provenance);docs/suppression-and-fp-policy.md→ shipped (P-015 config оставлен draft).Границы (follow-up, не в этом PR)
[OwnIgnore]пока наIDisposable-полях (самый чистый attribute-site — запись анкорится на поле). Подписки/таймеры/локали — следующие инкременты (локаль не может нести C#-атрибут в принципе). Ядро уже суппрессит любую запись сignore_reason, так что расширение экстрактора тривиально.Чеклист
test_ownir.py+ CI-ассерты, вкл. SARIF)feat:)🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
[OwnIgnore("reason")]suppression for disposable-field findings.