Skip to content

Commit e33c0a7

Browse files
committed
feat(suppression): implement [OwnIgnore("reason")] end to end (P-004)
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
1 parent a5d611c commit e33c0a7

9 files changed

Lines changed: 298 additions & 31 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ jobs:
205205
frontend/roslyn/samples/SemaphoreFieldSample.cs \
206206
frontend/roslyn/samples/VoidSubscribeSample.cs \
207207
frontend/roslyn/samples/ReturnedPublisherSample.cs \
208+
frontend/roslyn/samples/OwnIgnoreSample.cs \
208209
-o "$RUNNER_TEMP/facts.json"
209210
cat "$RUNNER_TEMP/facts.json"
210211
- name: Check facts through the core
@@ -723,7 +724,35 @@ jobs:
723724
if echo "$out" | grep -qE "(ScopeUsingService|ClockCachingService)"; then
724725
echo "FAIL: a correct scope use (used-in-scope, or a cached singleton) was wrongly flagged DI005"; exit 1
725726
fi
726-
echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) at the C# location"
727+
# issue #209 — inline [OwnIgnore("reason")] per-site suppression (P-004), on an
728+
# IDisposable field. Four contrasting shapes in OwnIgnoreSample.cs: the un-annotated
729+
# leak, a reason-less [OwnIgnore], and an empty [OwnIgnore("")] all FIRE OWN001; only
730+
# [OwnIgnore("reason")] is silent-but-COUNTED (SARIF suppressions), never failing the run.
731+
echo "$out" | grep -qE "OwnIgnoreSample\.cs:[0-9]+: error: \[OWN001\].*'UnsuppressedLeak'" \
732+
|| { echo "FAIL: an un-annotated IDisposable field must raise OWN001"; exit 1; }
733+
echo "$out" | grep -qE "\[OWN001\].*'ReasonlessLeak'" \
734+
|| { echo "FAIL: a reason-less [OwnIgnore] must NOT suppress (OWN001 must still fire)"; exit 1; }
735+
echo "$out" | grep -qE "\[OWN001\].*'EmptyReasonLeak'" \
736+
|| { echo "FAIL: an empty [OwnIgnore(\"\")] reason must NOT suppress (OWN001 must still fire)"; exit 1; }
737+
# the suppressed leak is SILENT in the human findings stream...
738+
if echo "$out" | grep -q "'SuppressedLeak'"; then
739+
echo "FAIL: a [OwnIgnore(\"reason\")] finding must be silent in the human output"; exit 1
740+
fi
741+
# ...but COUNTED in the run summary (visibility over silence).
742+
echo "$out" | grep -qE "[0-9]+ suppressed \(\[OwnIgnore\]\)" \
743+
|| { echo "FAIL: the suppressed finding must be counted in the summary tally"; exit 1; }
744+
# SARIF carries it as a result WITH a `suppressions` array (kind inSource + the
745+
# mandatory reason as justification) — a consumer counts it, GitHub shows it
746+
# suppressed rather than an open alert, and it never fails the run.
747+
python -m ownlang ownir "$RUNNER_TEMP/facts.json" --format sarif > "$RUNNER_TEMP/own.sarif" 2>/dev/null || true
748+
jq -e '[.runs[0].results[] | select(.properties.component == "SuppressedLeak")] as $s
749+
| ($s | length) == 1
750+
and ($s[0].suppressions[0].kind == "inSource")
751+
and ($s[0].suppressions[0].justification | contains("owned and disposed by the DI container"))' \
752+
"$RUNNER_TEMP/own.sarif" >/dev/null \
753+
|| { echo "FAIL: SARIF must carry SuppressedLeak once, with an inSource suppressions justification"; \
754+
jq '.runs[0].results[]|{ruleId,component:.properties.component,suppressions}' "$RUNNER_TEMP/own.sarif"; exit 1; }
755+
echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) at the C# location"
727756
- name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2)
728757
run: |
729758
# Path-sensitive flow analysis of local IDisposables — bugs the flat D1

‎docs/suppression-and-fp-policy.md‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,30 +45,36 @@ fire on unprovable input.
4545
|---|---|---|
4646
| `--severity warning` | **works today** (P-013) | Global: downgrades every error-tier finding for that run to advisory. Per-run, not per-finding — an escape hatch for "show me everything, but don't fail the build yet," not a way to silence one specific site. |
4747
| `--fail-on-finding` set to off | **works today** (P-013) | Global: findings still print/annotate, but the process/step exit code stays 0. The CLI (`own-check.sh`) is off by default — you must pass the flag to make findings fail the shell. The GitHub Action inverts that for safety: its `fail-on-finding` input defaults to `"true"` (fails the step on a finding), so to get the "annotate but don't fail" behavior in CI you must explicitly set `fail-on-finding: "false"`. |
48-
| `[OwnIgnore("reason")]` | **designed, not implemented** (P-004) | Inline, per-site suppression attribute — the intended fine-grained escape hatch for a specific subscription/field the checker can't see enough context to clear. Referenced across P-001/P-004/P-010/P-014/P-017 as the standing design; there is no code behind it yet. If you need this today, the honest answer is: you don't have it — file the case so it informs the implementation. |
48+
| `[OwnIgnore("reason")]` | **works today** on `IDisposable` fields (P-004, #209) | Inline, per-site suppression attribute — the fine-grained escape hatch for a specific site the checker can't see enough context to clear. Put `[OwnIgnore("reason")]` on the field; the finding is then **silent-but-counted** — kept out of the exit code and the human findings stream, but tallied in the run summary and carried in SARIF `suppressions` (`kind: "inSource"`, your reason as the `justification`) so nothing is lost and a consumer can audit it. The **reason is mandatory**: a reason-less `[OwnIgnore]` (or an empty `[OwnIgnore("")]`) does **not** suppress — a suppression is a documented decision, never a silent accept. The attribute is matched by simple name, so you can declare your own `OwnIgnoreAttribute`. Currently reads on `IDisposable` **field** declarations (the clearest attribute site); other sites (subscriptions, timers) are follow-up increments. |
4949
| Project-wide config (`.ownrc`/`own.toml`) | **draft, not implemented** (P-015) | Per-check-category enable/disable + severity + per-path overrides (e.g. relax a category under `tests/`). Stub status — format (TOML vs INI vs JSON) and enforcement point are still open questions in the proposal. |
5050
| `corpus/oracle-fp-baseline.txt` | **exists, but not a user-facing suppression tool** | An allowlist the *oracle comparator* (`scripts/oracle_compare.py`, a dev/maintainer tool) uses to keep already-triaged false positives out of the `own-only` bucket on re-runs. It doesn't change what `own-check`/the Action reports — it only keeps the oracle's own triage queue from re-showing confirmed noise. |
5151

52-
So today, honestly: there is no way to suppress **one specific finding** in
53-
your own repo. The two escape hatches for that (`[OwnIgnore]`, project config)
54-
are designed and drafted respectively, not shipped. What you have is a global
55-
severity dial and the extractor's own honest-skip behavior, which is why the
56-
precision bar above matters as much as the (currently thin) suppression
57-
surface — the fewer false positives reach you, the less suppression UX has to
58-
carry.
52+
So today: you **can** suppress one specific finding with an inline
53+
`[OwnIgnore("reason")]` on the field it fires on (shipped, #209) — the finding
54+
goes silent but stays counted (summary tally + SARIF `suppressions`). The
55+
project-wide counterpart (`.ownrc`/`own.toml`, P-015) is drafted, not shipped.
56+
Together with the global severity dial and the extractor's own honest-skip
57+
behavior, that covers per-site and per-run; the per-*category*, per-*path*
58+
config is the remaining gap. The precision bar above still matters as much as
59+
the suppression surface — the fewer false positives reach you, the less
60+
suppression UX has to carry.
5961

60-
## The designed shape (so you know what's coming)
62+
## The full shape (`[OwnIgnore]` shipped; config still to come)
6163

62-
Precedence, once both land (P-015's draft order):
64+
Precedence (P-015's draft order — the inline attribute half is shipped, #209;
65+
the config-file half is still draft):
6366

6467
```text
6568
CLI flag > inline [OwnIgnore] > config file > built-in default
6669
```
6770

68-
`[OwnIgnore("reason")]` (P-004) is a per-site attribute — the *reason* string
69-
is mandatory by design, so a suppression is a documented decision, not a
70-
silent one. Project config (P-015) is the per-category, project-wide
71-
counterpart — "treat subscriptions as warnings, keep disposables as errors,
71+
`[OwnIgnore("reason")]` (P-004, **shipped** #209) is a per-site attribute — the
72+
*reason* string is mandatory by design, so a suppression is a documented
73+
decision, not a silent one. It is consumed **core-side** (the extractor emits
74+
the finding fact with the reason marker; the core decides the verdict, keeps it
75+
out of the exit code, and stamps SARIF `suppressions`), so a suppression is
76+
counted, never a silent drop. Project config (P-015, draft) is the
77+
per-category, project-wide counterpart — "treat subscriptions as warnings, keep disposables as errors,
7278
skip pool checks under `tests/`" — discovered by walking up from the scanned
7379
path, the same convention as `.editorconfig`/`ruff.toml`. Both are consumed
7480
**core-side** ([P-013](proposals/P-013-distribution-surface.md)'s "one

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3448,6 +3448,39 @@ static bool IsPublicCtor(SyntaxTokenList modifiers)
34483448
return false;
34493449
}
34503450

3451+
// [OwnIgnore("reason")] per-site suppression (P-004 / issue #209). The reason string is
3452+
// MANDATORY by design — a suppression is a documented decision, not a silent one — so this
3453+
// returns the reason ONLY when the attribute carries a constant, non-empty string first
3454+
// argument; it returns null when the attribute is absent, reason-less (`[OwnIgnore]`), or
3455+
// carries an empty/non-constant reason. A null therefore never suppresses (the finding fires),
3456+
// which is exactly P-004's "never a silent accept" posture. Matched by SIMPLE name
3457+
// (`OwnIgnore` / `OwnIgnoreAttribute`), namespace-agnostic, like the BCL `[SuppressMessage]`
3458+
// convention — so a user may declare their own attribute type; the core (not this frontend) is
3459+
// the sole authority on the resulting verdict (P-013 "one checker"). Uses the SemanticModel's
3460+
// constant folding so a `const`/`nameof` reason resolves, not only a bare string literal.
3461+
static string? OwnIgnoreReason(SyntaxList<AttributeListSyntax> attrLists, SemanticModel model)
3462+
{
3463+
foreach (var al in attrLists)
3464+
foreach (var a in al.Attributes)
3465+
{
3466+
var simple = a.Name switch
3467+
{
3468+
QualifiedNameSyntax q => q.Right.Identifier.Text,
3469+
SimpleNameSyntax s => s.Identifier.Text,
3470+
_ => a.Name.ToString(),
3471+
};
3472+
if (simple is not ("OwnIgnore" or "OwnIgnoreAttribute"))
3473+
continue;
3474+
var arg = a.ArgumentList?.Arguments.FirstOrDefault();
3475+
if (arg is null)
3476+
return null; // reason-less [OwnIgnore] -> does not suppress
3477+
if (model.GetConstantValue(arg.Expression).Value is string r && r.Length > 0)
3478+
return r;
3479+
return null; // empty / non-constant reason -> does not suppress
3480+
}
3481+
return null;
3482+
}
3483+
34513484
var components = new List<object>();
34523485
// P-016 B0b/B2: per-method flow bodies (only when --flow-locals).
34533486
var flowFunctions = new List<object>();
@@ -3965,14 +3998,31 @@ or ImplicitObjectCreationExpressionSyntax
39653998
&& fieldCtors.Count > 0
39663999
&& fieldCtors.All(c => IsNoOpDisposeWrapper(c, model)))
39674000
continue;
3968-
subs.Add(new
3969-
{
3970-
@event = v.Identifier.Text,
3971-
line = LineOf(v),
3972-
released = disposed.Contains(v.Identifier.Text),
3973-
resource = "disposable",
3974-
type = tname,
3975-
});
4001+
// [OwnIgnore("reason")] on the field declaration (P-004 / #209): emit the
4002+
// record WITH the reason marker rather than dropping it, so the core can
4003+
// count the suppression and SARIF carries it in `suppressions` (visibility
4004+
// over silence). Additive/optional: absent when there is no valid reason, so
4005+
// an older core ignores it — exactly the source_provenance convention.
4006+
var ignoreReason = OwnIgnoreReason(fd.AttributeLists, model);
4007+
if (ignoreReason is not null)
4008+
subs.Add(new
4009+
{
4010+
@event = v.Identifier.Text,
4011+
line = LineOf(v),
4012+
released = disposed.Contains(v.Identifier.Text),
4013+
resource = "disposable",
4014+
type = tname,
4015+
ignore_reason = ignoreReason,
4016+
});
4017+
else
4018+
subs.Add(new
4019+
{
4020+
@event = v.Identifier.Text,
4021+
line = LineOf(v),
4022+
released = disposed.Contains(v.Identifier.Text),
4023+
resource = "disposable",
4024+
type = tname,
4025+
});
39764026
}
39774027
}
39784028

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
using System;
2+
3+
namespace Own.Samples;
4+
5+
// Issue #209 — [OwnIgnore("reason")] per-site suppression (P-004), on an IDisposable field.
6+
// The reason string is MANDATORY by design: a suppression is a documented decision, never a
7+
// silent one. Three contrasting shapes prove the contract end to end:
8+
// - UnsuppressedLeak : the plain leak, no attribute -> OWN001 fires (control)
9+
// - SuppressedLeak : same leak + [OwnIgnore("reason")] -> silent-but-COUNTED
10+
// (SARIF `suppressions`),
11+
// never fails the run
12+
// - ReasonlessLeak : [OwnIgnore] with no reason -> must NOT suppress (fires)
13+
// - EmptyReasonLeak : [OwnIgnore("")] -> must NOT suppress (fires)
14+
//
15+
// The attribute is matched by SIMPLE name, so a project may declare its own; this sample
16+
// declares a local one (two ctors) so all four shapes compile.
17+
18+
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property
19+
| AttributeTargets.Method | AttributeTargets.Class, AllowMultiple = false)]
20+
public sealed class OwnIgnoreAttribute : Attribute
21+
{
22+
public OwnIgnoreAttribute() { }
23+
public OwnIgnoreAttribute(string reason) { Reason = reason; }
24+
public string? Reason { get; }
25+
}
26+
27+
// A plain owned IDisposable — no BCL special-casing, so it is an unambiguous OWN001.
28+
public sealed class Handle : IDisposable
29+
{
30+
public void Dispose() { }
31+
}
32+
33+
// CONTROL: a `new`'d IDisposable field never disposed -> OWN001 fires.
34+
public sealed class UnsuppressedLeak
35+
{
36+
private readonly Handle _h = new Handle();
37+
}
38+
39+
// SUPPRESSED: the same leak, but a documented [OwnIgnore("reason")] -> silent-but-counted.
40+
public sealed class SuppressedLeak
41+
{
42+
[OwnIgnore("owned and disposed by the DI container, not by this type")]
43+
private readonly Handle _h = new Handle();
44+
}
45+
46+
// REASON-LESS: [OwnIgnore] carries no reason -> must NOT suppress (never a silent accept).
47+
public sealed class ReasonlessLeak
48+
{
49+
[OwnIgnore]
50+
private readonly Handle _h = new Handle();
51+
}
52+
53+
// EMPTY REASON: [OwnIgnore("")] is not a documented decision -> must NOT suppress.
54+
public sealed class EmptyReasonLeak
55+
{
56+
[OwnIgnore("")]
57+
private readonly Handle _h = new Handle();
58+
}

‎ownlang/__main__.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -349,15 +349,22 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error",
349349
# Advisory findings (OWN050 "leakage analysis skipped", OBL005 "dead protocol
350350
# rule") are always shown as warnings regardless of --severity, and never
351351
# affect the exit code — they are coverage/hygiene notes, not verdicts.
352-
leaks = [f for f in findings if not f.advisory]
353-
notes = [f for f in findings if f.advisory]
354-
shown = leaks if verbosity == "quiet" else findings
352+
# Inline `[OwnIgnore("reason")]` suppressions (P-004, #209) are counted and carried in
353+
# SARIF `suppressions`, but kept OUT of the human findings stream and the exit code —
354+
# visibility over silence, without failing the run. Everything else is "active".
355+
suppressed = [f for f in findings if f.suppressed]
356+
active = [f for f in findings if not f.suppressed]
357+
leaks = [f for f in active if not f.advisory]
358+
notes = [f for f in active if f.advisory]
359+
shown = leaks if verbosity == "quiet" else active
355360
if fmt == "sarif":
356361
# SARIF is one document for the whole run (not a line per finding): stdout
357362
# carries only the JSON; the summary goes to stderr like the other machine
358363
# formats. build_sarif applies the same per-finding severity policy below.
364+
# Suppressed findings ride along (marked with a `suppressions` array) so a
365+
# SARIF consumer can count them rather than losing them.
359366
import json
360-
print(json.dumps(build_sarif(shown, severity), indent=2))
367+
print(json.dumps(build_sarif(shown + suppressed, severity), indent=2))
361368
else:
362369
for f in shown:
363370
# Severity is the weaker of the host's --severity and the finding's own
@@ -382,6 +389,10 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error",
382389
note_codes = "/".join(sorted({x.code for x in notes}))
383390
summary += (f" ({len(notes)} advisory hidden)" if verbosity == "quiet"
384391
else f", {len(notes)} advisory ({note_codes})")
392+
if suppressed:
393+
# counted, never silent: [OwnIgnore] suppressions are tallied here and carried in
394+
# SARIF `suppressions`, but they do not print as findings and do not fail the run.
395+
summary += f", {len(suppressed)} suppressed ([OwnIgnore])"
385396
print(summary + ".", file=summary_to)
386397
if verbosity == "verbose" and findings:
387398
by_code: dict[str, int] = {}

0 commit comments

Comments
 (0)