Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ jobs:
frontend/roslyn/samples/SemaphoreFieldSample.cs \
frontend/roslyn/samples/VoidSubscribeSample.cs \
frontend/roslyn/samples/ReturnedPublisherSample.cs \
frontend/roslyn/samples/OwnIgnoreSample.cs \
-o "$RUNNER_TEMP/facts.json"
cat "$RUNNER_TEMP/facts.json"
- name: Check facts through the core
Expand Down Expand Up @@ -723,7 +724,36 @@ jobs:
if echo "$out" | grep -qE "(ScopeUsingService|ClockCachingService)"; then
echo "FAIL: a correct scope use (used-in-scope, or a cached singleton) was wrongly flagged DI005"; exit 1
fi
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"
# issue #209 — inline [OwnIgnore("reason")] per-site suppression (P-004), on an
# IDisposable field. Four contrasting shapes in OwnIgnoreSample.cs: the un-annotated
# leak, a reason-less [OwnIgnore], and an empty [OwnIgnore("")] all FIRE OWN001; only
# [OwnIgnore("reason")] is silent-but-COUNTED (SARIF suppressions), never failing the run.
echo "$out" | grep -qE "OwnIgnoreSample\.cs:[0-9]+: error: \[OWN001\].*'UnsuppressedLeak'" \
|| { echo "FAIL: an un-annotated IDisposable field must raise OWN001"; exit 1; }
echo "$out" | grep -qE "\[OWN001\].*'ReasonlessLeak'" \
|| { echo "FAIL: a reason-less [OwnIgnore] must NOT suppress (OWN001 must still fire)"; exit 1; }
echo "$out" | grep -qE "\[OWN001\].*'EmptyReasonLeak'" \
|| { echo "FAIL: an empty [OwnIgnore(\"\")] reason must NOT suppress (OWN001 must still fire)"; exit 1; }
# the suppressed leak is SILENT in the human findings stream...
if echo "$out" | grep -q "'SuppressedLeak'"; then
echo "FAIL: a [OwnIgnore(\"reason\")] finding must be silent in the human output"; exit 1
fi
# ...but COUNTED in the run summary (visibility over silence).
echo "$out" | grep -qE "[0-9]+ suppressed \(\[OwnIgnore\]\)" \
|| { echo "FAIL: the suppressed finding must be counted in the summary tally"; exit 1; }
# SARIF carries it as a result WITH a `suppressions` array (kind inSource + the
# mandatory reason as justification) — a consumer counts it, GitHub shows it
# suppressed rather than an open alert, and it never fails the run.
rc=0; python -m ownlang ownir "$RUNNER_TEMP/facts.json" --format sarif > "$RUNNER_TEMP/own.sarif" || rc=$?
[ "$rc" -le 1 ] || { echo "FAIL: SARIF generation errored (rc=$rc)"; exit 1; }
jq -e '[.runs[0].results[] | select(.properties.component == "SuppressedLeak")] as $s
| ($s | length) == 1
and ($s[0].suppressions[0].kind == "inSource")
and ($s[0].suppressions[0].justification | contains("owned and disposed by the DI container"))' \
"$RUNNER_TEMP/own.sarif" >/dev/null \
|| { echo "FAIL: SARIF must carry SuppressedLeak once, with an inSource suppressions justification"; \
jq '.runs[0].results[]|{ruleId,component:.properties.component,suppressions}' "$RUNNER_TEMP/own.sarif"; exit 1; }
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"
- name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2)
run: |
# Path-sensitive flow analysis of local IDisposables — bugs the flat D1
Expand Down
34 changes: 20 additions & 14 deletions docs/suppression-and-fp-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,30 +45,36 @@ fire on unprovable input.
|---|---|---|
| `--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. |
| `--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"`. |
| `[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. |
| `[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. |
| 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. |
| `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. |

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

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

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

```text
CLI flag > inline [OwnIgnore] > config file > built-in default
```

`[OwnIgnore("reason")]` (P-004) is a per-site attribute — the *reason* string
is mandatory by design, so a suppression is a documented decision, not a
silent one. Project config (P-015) is the per-category, project-wide
counterpart — "treat subscriptions as warnings, keep disposables as errors,
`[OwnIgnore("reason")]` (P-004, **shipped** #209) is a per-site attribute — the
*reason* string is mandatory by design, so a suppression is a documented
decision, not a silent one. It is consumed **core-side** (the extractor emits
the finding fact with the reason marker; the core decides the verdict, keeps it
out of the exit code, and stamps SARIF `suppressions`), so a suppression is
counted, never a silent drop. Project config (P-015, draft) is the
per-category, project-wide counterpart — "treat subscriptions as warnings, keep disposables as errors,
skip pool checks under `tests/`" — discovered by walking up from the scanned
path, the same convention as `.editorconfig`/`ruff.toml`. Both are consumed
**core-side** ([P-013](proposals/P-013-distribution-surface.md)'s "one
Expand Down
66 changes: 58 additions & 8 deletions frontend/roslyn/OwnSharp.Extractor/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3488,6 +3488,39 @@
return false;
}

// [OwnIgnore("reason")] per-site suppression (P-004 / issue #209). The reason string is
// MANDATORY by design — a suppression is a documented decision, not a silent one — so this
// returns the reason ONLY when the attribute carries a constant, non-empty string first
// argument; it returns null when the attribute is absent, reason-less (`[OwnIgnore]`), or
// carries an empty/non-constant reason. A null therefore never suppresses (the finding fires),
// which is exactly P-004's "never a silent accept" posture. Matched by SIMPLE name
// (`OwnIgnore` / `OwnIgnoreAttribute`), namespace-agnostic, like the BCL `[SuppressMessage]`
// convention — so a user may declare their own attribute type; the core (not this frontend) is
// the sole authority on the resulting verdict (P-013 "one checker"). Uses the SemanticModel's
// constant folding so a `const`/`nameof` reason resolves, not only a bare string literal.
static string? OwnIgnoreReason(SyntaxList<AttributeListSyntax> attrLists, SemanticModel model)
{
foreach (var al in attrLists)
foreach (var a in al.Attributes)
{
var simple = a.Name switch
{
QualifiedNameSyntax q => q.Right.Identifier.Text,
SimpleNameSyntax s => s.Identifier.Text,
_ => a.Name.ToString(),
};
if (simple is not ("OwnIgnore" or "OwnIgnoreAttribute"))
continue;
var arg = a.ArgumentList?.Arguments.FirstOrDefault();
if (arg is null)
return null; // reason-less [OwnIgnore] -> does not suppress
if (model.GetConstantValue(arg.Expression).Value is string r && r.Length > 0)
return r;
return null; // empty / non-constant reason -> does not suppress
}
return null;
}

var components = new List<object>();
// P-016 B0b/B2: per-method flow bodies (only when --flow-locals).
var flowFunctions = new List<object>();
Expand Down Expand Up @@ -3549,7 +3582,7 @@
.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
.Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
.ToList();
var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / ownsharp CLI (gate A) — clean install -> check -> findings (ubuntu-latest)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / P-014 Tier B — external reference resolution (--ref-dir)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / P-014 Tier B — external reference resolution (--ref-dir)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / ownsharp CLI (gate A) — clean install -> check -> findings (ubuntu-latest)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / ownsharp CLI (gate A) — clean install -> check -> findings (windows-latest)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / ownsharp CLI (gate A) — clean install -> check -> findings (windows-latest)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3585 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.
var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
// P-004 WPF profile: widen the reference set with assemblies named by the
// OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
Expand Down Expand Up @@ -4005,14 +4038,31 @@
&& fieldCtors.Count > 0
&& fieldCtors.All(c => IsNoOpDisposeWrapper(c, model)))
continue;
subs.Add(new
{
@event = v.Identifier.Text,
line = LineOf(v),
released = disposed.Contains(v.Identifier.Text),
resource = "disposable",
type = tname,
});
// [OwnIgnore("reason")] on the field declaration (P-004 / #209): emit the
// record WITH the reason marker rather than dropping it, so the core can
// count the suppression and SARIF carries it in `suppressions` (visibility
// over silence). Additive/optional: absent when there is no valid reason, so
// an older core ignores it — exactly the source_provenance convention.
var ignoreReason = OwnIgnoreReason(fd.AttributeLists, model);
if (ignoreReason is not null)
subs.Add(new
{
@event = v.Identifier.Text,
line = LineOf(v),
released = disposed.Contains(v.Identifier.Text),
resource = "disposable",
type = tname,
ignore_reason = ignoreReason,
});
else
subs.Add(new
{
@event = v.Identifier.Text,
line = LineOf(v),
released = disposed.Contains(v.Identifier.Text),
resource = "disposable",
type = tname,
});
}
}

Expand Down
58 changes: 58 additions & 0 deletions frontend/roslyn/samples/OwnIgnoreSample.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
using System;

namespace Own.Samples;

// Issue #209 — [OwnIgnore("reason")] per-site suppression (P-004), on an IDisposable field.
// The reason string is MANDATORY by design: a suppression is a documented decision, never a
// silent one. Three contrasting shapes prove the contract end to end:
// - UnsuppressedLeak : the plain leak, no attribute -> OWN001 fires (control)
// - SuppressedLeak : same leak + [OwnIgnore("reason")] -> silent-but-COUNTED
// (SARIF `suppressions`),
// never fails the run
// - ReasonlessLeak : [OwnIgnore] with no reason -> must NOT suppress (fires)
// - EmptyReasonLeak : [OwnIgnore("")] -> must NOT suppress (fires)
//
// The attribute is matched by SIMPLE name, so a project may declare its own; this sample
// declares a local one (two ctors) so all four shapes compile.

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property
| AttributeTargets.Method | AttributeTargets.Class, AllowMultiple = false)]
public sealed class OwnIgnoreAttribute : Attribute
{
public OwnIgnoreAttribute() { }
public OwnIgnoreAttribute(string reason) { Reason = reason; }
public string? Reason { get; }
}

// A plain owned IDisposable — no BCL special-casing, so it is an unambiguous OWN001.
public sealed class Handle : IDisposable
{
public void Dispose() { }
}

// CONTROL: a `new`'d IDisposable field never disposed -> OWN001 fires.
public sealed class UnsuppressedLeak
{
private readonly Handle _h = new Handle();

Check warning

Code scanning / Own.NET

owned resource not released on all paths (possible leak) Warning

IDisposable field '_h' (type 'Handle') is never disposed — its owner 'UnsuppressedLeak' leaks it (leak) [resource: disposable field]
}

// SUPPRESSED: the same leak, but a documented [OwnIgnore("reason")] -> silent-but-counted.
public sealed class SuppressedLeak
{
[OwnIgnore("owned and disposed by the DI container, not by this type")]
private readonly Handle _h = new Handle();

Check warning

Code scanning / Own.NET

owned resource not released on all paths (possible leak) Warning

IDisposable field '_h' (type 'Handle') is never disposed — its owner 'SuppressedLeak' leaks it (leak) [resource: disposable field]
}

// REASON-LESS: [OwnIgnore] carries no reason -> must NOT suppress (never a silent accept).
public sealed class ReasonlessLeak
{
[OwnIgnore]
private readonly Handle _h = new Handle();

Check warning

Code scanning / Own.NET

owned resource not released on all paths (possible leak) Warning

IDisposable field '_h' (type 'Handle') is never disposed — its owner 'ReasonlessLeak' leaks it (leak) [resource: disposable field]
}

// EMPTY REASON: [OwnIgnore("")] is not a documented decision -> must NOT suppress.
public sealed class EmptyReasonLeak
{
[OwnIgnore("")]
private readonly Handle _h = new Handle();

Check warning

Code scanning / Own.NET

owned resource not released on all paths (possible leak) Warning

IDisposable field '_h' (type 'Handle') is never disposed — its owner 'EmptyReasonLeak' leaks it (leak) [resource: disposable field]
}
19 changes: 15 additions & 4 deletions ownlang/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,15 +349,22 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error",
# Advisory findings (OWN050 "leakage analysis skipped", OBL005 "dead protocol
# rule") are always shown as warnings regardless of --severity, and never
# affect the exit code — they are coverage/hygiene notes, not verdicts.
leaks = [f for f in findings if not f.advisory]
notes = [f for f in findings if f.advisory]
shown = leaks if verbosity == "quiet" else findings
# Inline `[OwnIgnore("reason")]` suppressions (P-004, #209) are counted and carried in
# SARIF `suppressions`, but kept OUT of the human findings stream and the exit code —
# visibility over silence, without failing the run. Everything else is "active".
suppressed = [f for f in findings if f.suppressed]
active = [f for f in findings if not f.suppressed]
leaks = [f for f in active if not f.advisory]
notes = [f for f in active if f.advisory]
shown = leaks if verbosity == "quiet" else active
if fmt == "sarif":
# SARIF is one document for the whole run (not a line per finding): stdout
# carries only the JSON; the summary goes to stderr like the other machine
# formats. build_sarif applies the same per-finding severity policy below.
# Suppressed findings ride along (marked with a `suppressions` array) so a
# SARIF consumer can count them rather than losing them.
import json
print(json.dumps(build_sarif(shown, severity), indent=2))
print(json.dumps(build_sarif(shown + suppressed, severity), indent=2))
else:
for f in shown:
# Severity is the weaker of the host's --severity and the finding's own
Expand All @@ -382,6 +389,10 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error",
note_codes = "/".join(sorted({x.code for x in notes}))
summary += (f" ({len(notes)} advisory hidden)" if verbosity == "quiet"
else f", {len(notes)} advisory ({note_codes})")
if suppressed:
# counted, never silent: [OwnIgnore] suppressions are tallied here and carried in
# SARIF `suppressions`, but they do not print as findings and do not fail the run.
summary += f", {len(suppressed)} suppressed ([OwnIgnore])"
print(summary + ".", file=summary_to)
if verbosity == "verbose" and findings:
by_code: dict[str, int] = {}
Expand Down
Loading
Loading