diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7ac86b2b..c4ce0f7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -209,6 +209,15 @@ jobs: echo "--- diagnostics ---"; echo "$out"; echo "-------------------" echo "$out" | grep -qE "CustomerViewModel\.cs\([0-9]+\): error OWN001:" \ || { echo "FAIL: expected an MSBuild-format error line"; exit 1; } + - name: --severity warning renders advisory diagnostics + run: | + out=$(scripts/own-check.sh --format msbuild --severity warning -- frontend/roslyn/samples) + echo "$out" + echo "$out" | grep -qE "CustomerViewModel\.cs\([0-9]+\): warning OWN001:" \ + || { echo "FAIL: expected an MSBuild-format warning line"; exit 1; } + if echo "$out" | grep -qE ": error OWN001:"; then + echo "FAIL: --severity warning should not emit error-level lines"; exit 1 + fi - name: --fail-on-finding propagates the core's exit code run: | if scripts/own-check.sh --fail-on-finding -- frontend/roslyn/samples >/dev/null 2>&1; then diff --git a/action.yml b/action.yml index c95ecff2..119551c7 100644 --- a/action.yml +++ b/action.yml @@ -17,6 +17,10 @@ inputs: description: "Finding surface: github (PR annotations), msbuild, or human." required: false default: "github" + severity: + description: "How findings are shown: error (default) or warning (advisory)." + required: false + default: "error" fail-on-finding: description: "Fail the step when any leak is found." required: false @@ -52,9 +56,10 @@ runs: # it. CodeRabbit #10. OWN_PATH: ${{ inputs.path }} OWN_FORMAT: ${{ inputs.format }} + OWN_SEVERITY: ${{ inputs.severity }} OWN_FAIL_ON_FINDING: ${{ inputs.fail-on-finding }} run: | - args=(--root "${{ github.action_path }}" --format "$OWN_FORMAT") + args=(--root "${{ github.action_path }}" --format "$OWN_FORMAT" --severity "$OWN_SEVERITY") if [ "$OWN_FAIL_ON_FINDING" = "true" ]; then args+=(--fail-on-finding) fi diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index d10cdab4..23b8365a 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -69,7 +69,8 @@ architectural strictness, and the borrow-checker showcase): 1. `WPF001` — event/subscription `+=` without `-=` (the WPF spike; P-001 v0 ✅) 2. `WPF002` — `DispatcherTimer`/`Timer` `Tick`/`Elapsed` without stop/detach ✅ 3. `OWN001` — `IDisposable` field the class `new`s but never disposes ✅ -4. `DI001` — singleton captures a scoped dependency +4. `DI001` — singleton captures a scoped dependency ✅ (core check built; + C# registration-graph extractor pending) 5. `POOL001` — `ArrayPool` buffer `Rent`ed but never `Return`ed ✅ (`POOL002` `Span`/view used after `Return` next) @@ -149,7 +150,7 @@ own scan. Label them as estimates wherever they appear. | [P-003](proposals/P-003-lifetime-visualization.md) | Lifetime visualization (RustOwl-style) | horizon | draft | | [P-004](proposals/P-004-wpf-lifetime-profile.md) | WPF / UI lifetime leak profile | P0 | draft | | [P-005](proposals/P-005-idisposable-ownership.md) | `IDisposable` ownership profile | P0 | draft | -| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | draft | +| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | in progress (DI001 core check built) | | [P-007](proposals/P-007-arraypool-span.md) | ArrayPool / Span borrow-view | P1 | draft | | [P-008](proposals/P-008-effects-and-resources.md) | Effects & resources (`Own.Effects`) | P1/P2 | draft | | [P-009](proposals/P-009-nogc-regions.md) | No-GC / allocation-free regions | horizon | draft | diff --git a/docs/howto-visual-studio.md b/docs/howto-visual-studio.md new file mode 100644 index 00000000..75d23807 --- /dev/null +++ b/docs/howto-visual-studio.md @@ -0,0 +1,160 @@ +# How-to: run Own.NET on your C# (CLI · CI · Visual Studio) + +Own.NET finds lifetime/resource leaks C# can't express (event/timer leaks, +undisposed `IDisposable`, ignored `Subscribe()` tokens, `ArrayPool` rent-without- +return). This guide shows the three ways to actually *run* it on your code. + +> **There is no VSIX and no Roslyn analyzer.** On purpose — the checker is one +> Python core, and an in-process analyzer would force a second checker in C# (or +> shell out to Python on every keystroke). Instead the same finding is rendered +> in a format the host already understands: GitHub annotations for CI, and the +> **MSBuild diagnostic format** (`file(line): error CODE: …`) that the Visual +> Studio **Error List** parses for free. See +> [P-013](proposals/P-013-distribution-surface.md) for the why. + +## 0. Prerequisites + +The pipeline is two stages — a Roslyn extractor (C#) and the Python core — so you +need both runtimes plus an Own.NET checkout: + +- **.NET SDK** 8.0+ (`dotnet`) — builds/runs the extractor. +- **Python** 3.11+ — runs the core. +- An **Own.NET checkout** somewhere on disk; below it is `$OWN` (e.g. + `git clone https://github.com/PhysShell/Own.NET ~/own.net`). + +Nothing is installed into your project — the tool reads your `.cs`, it does not +add a dependency. + +## 1. From a terminal (the foundation everything else wraps) + +```bash +# human-readable (default) +"$OWN/scripts/own-check.sh" -- path/to/your/Project + +# Visual Studio / MSBuild Error List format +"$OWN/scripts/own-check.sh" --format msbuild -- path/to/your/Project + +# CI annotations; non-zero exit on any finding +"$OWN/scripts/own-check.sh" --format github --fail-on-finding -- . + +# advisory: render findings as warnings (won't fail a build) +"$OWN/scripts/own-check.sh" --format msbuild --severity warning -- path/to/your/Project +``` + +`own-check.sh` walks the path for `*.cs` (skipping `bin`/`obj`/`.git`/ +`node_modules`/`packages` and generated files), runs the extractor → the core, +and prints findings. `--format msbuild` prints exactly: + +```text +src/Vm/CustomerViewModel.cs(12): error OWN001: event 'bus.CustomerChanged' is subscribed (handler 'OnCustomerChanged') but never unsubscribed — the source keeps 'CustomerViewModel' alive (leak) [resource: subscription token] +``` + +That line shape is the canonical MSBuild diagnostic — which is the whole trick +for Visual Studio. + +Exit codes: `0` clean · `1` findings (only with `--fail-on-finding`) · `≥2` a +hard error (bad facts). Without `--fail-on-finding` the script always exits `0` +so it never breaks a wrapping build by accident. + +## 2. In Visual Studio + +Two approaches, cheapest first. + +### Option A — External Tool (on-demand, no build coupling) — recommended + +Run the check from a menu item and get clickable results. **Tools → External +Tools… → Add:** + +| Field | Value | +| --- | --- | +| Title | `Own.NET leak check` | +| Command | `bash` (Linux/macOS), or `C:\Program Files\Git\bin\bash.exe` (Git Bash on Windows) | +| Arguments | `"/scripts/own-check.sh" --format msbuild -- "$(ProjectDir)"` | +| Initial directory | `$(SolutionDir)` | +| ✔ **Use Output window** | checked | + +Run it from **Tools → Own.NET leak check**. Output appears in the Output window, +and because the lines are in canonical MSBuild format, Visual Studio makes each +one **double-click-to-navigate** to the exact file and line. + +> On Windows the script needs a bash (WSL or Git Bash). If you'd rather not, use +> the raw two-command form from §4 in a `.cmd`/PowerShell External Tool instead. + +### Option B — MSBuild target (findings in the Error List on every build) + +Drop a `Directory.Build.targets` next to your solution (or add the `` to +a `.csproj`): + +```xml + + + + /abs/path/to/own.net + + + + + + +``` + +The MSBuild `Exec` task scans the command's output for canonical diagnostic +lines and raises them as build diagnostics, so they land in the **Error List** +and the build log without any analyzer. + +**Severity:** `--severity` chooses how the host shows a finding. The target +above uses `--severity warning` so findings are **advisory** — they appear in +the Error List but don't fail the build, which is what you want on every +inner-loop build. Drop `--severity warning` (the default is `error`) if you'd +rather a leak break the build, e.g. on a release/CI configuration. + +> Note: this runs the extractor build (`dotnet run`) as part of your build, which +> adds a few seconds. For large solutions prefer Option A or the CI job (§3) over +> a `BeforeTargets="Build"` hook on every inner-loop build. + +## 3. In CI (GitHub Actions) + +The reusable composite action annotates the PR diff. Add to a workflow (full +example in [`examples/ci/own-check.yml`](../examples/ci/own-check.yml)): + +```yaml +- uses: actions/checkout@v4 +- uses: PhysShell/own.net@main # pin a tag for stability + with: + path: . + format: github + fail-on-finding: "true" +``` + +Findings appear as inline `error` annotations on the changed lines, and the +check goes red. This is the path that needs no local setup at all. + +## 4. Windows without bash (PowerShell) + +If you have no bash, use the bundled PowerShell twin — same flags, same output, +no shell dependency: + +```powershell +& "$OWN\scripts\own-check.ps1" -Format msbuild -- src\MyApp +& "$OWN\scripts\own-check.ps1" -Format github -Severity warning -FailOnFinding -- . +``` + +Point a Visual Studio External Tool (§2A) at `powershell.exe` with arguments +`-File "\scripts\own-check.ps1" -Format msbuild -- "$(ProjectDir)"`, or an +MSBuild `Exec` (§2B) at `powershell -File "$(OwnNetRoot)\scripts\own-check.ps1" …`, +instead of the bash command. + +## Caveats + +- **Heuristic findings can be false positives** (e.g. ownership handed to a + callee). Treat output as a reviewer, not a gate, until you've calibrated it on + your codebase — and prefer `--fail-on-finding` only once it's quiet. +- **One method at a time, syntax-only.** The extractor does not do + interprocedural/`async`/whole-program analysis yet (by design — see the + ROADMAP). It honestly skips what it can't model rather than guessing. +- **`bin`/`obj`/generated files are skipped**; paths are reported relative to the + scan root so they resolve in the editor and on the PR. + +For the design rationale and the full surface ladder, see +[P-013](proposals/P-013-distribution-surface.md). diff --git a/docs/proposals/P-006-di-lifetimes.md b/docs/proposals/P-006-di-lifetimes.md index c10f1bf3..106fd596 100644 --- a/docs/proposals/P-006-di-lifetimes.md +++ b/docs/proposals/P-006-di-lifetimes.md @@ -1,6 +1,11 @@ # P-006 — DI lifetime / captive dependency profile -- **Status:** draft (P0 — clean lifetime model, little R&D, sells to ASP.NET) +- **Status:** in progress (P0 — clean lifetime model, little R&D, sells to + ASP.NET). DI001 captive-dependency check built in the core (`ownlang/di.py`) + over an OwnIR `services` registration graph, surfaced through the bridge with + hand-written facts + tests. Next: the C# extractor that builds the registration + graph from `services.Add{Singleton,Scoped,Transient}` + constructor injection + (CI-only, like the rest of the extractor). - **Depends on:** `spec/Lifetimes.md` (the region-ordering model behind OWN014), [P-001](P-001-csharp-extractor.md) (the C# seam). See [`docs/ROADMAP.md`](../ROADMAP.md) (Milestone 3). diff --git a/docs/proposals/P-013-distribution-surface.md b/docs/proposals/P-013-distribution-surface.md index f3248440..4337c8c6 100644 --- a/docs/proposals/P-013-distribution-surface.md +++ b/docs/proposals/P-013-distribution-surface.md @@ -54,9 +54,10 @@ and defer the native analyzer until (if ever) the core itself moves to .NET. files (`*.g.cs`, `*.Designer.cs`, `*.AssemblyInfo.cs`). Finding paths are reported relative to the working directory (forward slashes) so annotations point at the right file even when names collide. -- **`scripts/own-check.sh`.** One command that chains both stages (extractor → - core) with `--format` and `--fail-on-finding`. The body of the Action and a - standalone local command. +- **`scripts/own-check.sh`** (+ a PowerShell twin **`scripts/own-check.ps1`** for + Windows/VS users without bash). One command that chains both stages (extractor + → core) with `--format`, `--severity`, and `--fail-on-finding`. The body of the + Action and a standalone local command. - **`action.yml`** — a composite GitHub Action (`uses: PhysShell/own.net@…`): sets up Python + .NET, runs `own-check.sh` over the consumer's checkout, annotates the PR. Example consumer workflow in `examples/ci/own-check.yml`. @@ -84,9 +85,11 @@ not overlap. ## Open questions -1. MSBuild severity: emit findings as `error` (fails a parsing build) or - `warning` (advisory)? v0 uses `error` to match the core; the Action's - `fail-on-finding` already gates CI independently. +1. ~~MSBuild severity: emit findings as `error` or `warning`?~~ **Resolved:** + a `--severity {error,warning}` flag (default `error`) is threaded through the + core renderer, `own-check.sh`/`.ps1`, and the Action's `severity` input, so a + build can show findings advisory (warning) without failing. The Action's + `fail-on-finding` still gates CI independently. 2. Should `own-check` grow a `--baseline`/diff mode (only new findings on a PR) so adopting it on a legacy repo isn't an immediate wall of red? 3. Action distribution: a moving `@main`, or tagged releases (`@v0.1.0`) + diff --git a/frontend/roslyn/README.md b/frontend/roslyn/README.md index 105af69b..e47ac4f2 100644 --- a/frontend/roslyn/README.md +++ b/frontend/roslyn/README.md @@ -63,6 +63,9 @@ in-process and would force a *second* checker in C# (or shelling out to Python per keystroke) — a conflict with "one checker", not just effort. See [P-013](../../docs/proposals/P-013-distribution-surface.md). +**Step-by-step usage** (terminal, Visual Studio Error List, CI) lives in +[`docs/howto-visual-studio.md`](../../docs/howto-visual-studio.md). + ## Scope / honesty This sandbox has no local `dotnet`, so the extractor is built and run only in CI diff --git a/ownlang/__main__.py b/ownlang/__main__.py index d24bf512..56263b91 100644 --- a/ownlang/__main__.py +++ b/ownlang/__main__.py @@ -6,10 +6,13 @@ python -m ownlang cfg file.own # dump the control-flow graph python -m ownlang report file.own # buffer storage report + .ownreport.json python -m ownlang ownir facts.json # check OwnIR facts extracted from C# (P-001) - python -m ownlang ownir facts.json --format github|msbuild|human + python -m ownlang ownir facts.json --format github|msbuild|human [--severity error|warning] `--format` (ownir only) selects the finding surface: `human` (default CLI line), `github` (CI annotations on the PR diff), or `msbuild` (VS Error List). +`--severity` (ownir only) picks how the host shows a finding — `error` (default, +fails a build / red check) or `warning` (advisory). It is a presentation choice; +the finding is still the core's verdict. Exit code is non-zero if any error-level diagnostic was produced. """ @@ -185,10 +188,11 @@ def _read(path: str) -> str: return f.read() -def cmd_ownir(path: str, fmt: str = "human") -> int: +def cmd_ownir(path: str, fmt: str = "human", severity: str = "error") -> int: """Check OwnIR facts (extracted from real C# by the Roslyn frontend) through the same core, surfacing findings at their C# locations (P-001). `fmt` - selects the surface: human (CLI), github (CI annotations), msbuild (VS).""" + selects the surface: human (CLI), github (CI annotations), msbuild (VS); + `severity` picks how the host shows them (error/warning).""" from .ownir import OwnIRError, check_facts, load, render_finding try: findings = check_facts(load(path)) @@ -202,7 +206,7 @@ def cmd_ownir(path: str, fmt: str = "human") -> int: machine = fmt in {"github", "msbuild"} summary_to = sys.stderr if machine else sys.stdout for f in findings: - print(render_finding(f, fmt)) + print(render_finding(f, fmt, severity)) if not findings: print(f"{path}: ok — no subscription leaks found", file=summary_to) n = len(findings) @@ -211,6 +215,7 @@ def cmd_ownir(path: str, fmt: str = "human") -> int: _FORMATS = {"human", "github", "msbuild"} +_SEVERITIES = {"error", "warning"} def main(argv: list[str]) -> int: @@ -218,24 +223,30 @@ def main(argv: list[str]) -> int: print(__doc__) return 2 cmd = argv[0] - # Pull the optional `--format X` / `--format=X` flag (ownir only) out of the - # arguments; everything else is positional. Keeps the other commands' single - # positional-path contract intact. - fmt = "human" + # Pull the optional value-flags (`--format`/`--severity`, ownir only) out of + # the arguments in either `--flag V` or `--flag=V` form; everything else is + # positional. Keeps the other commands' single positional-path contract. + opts = {"--format": "human", "--severity": "error"} + seen_value_flags = False positional: list[str] = [] rest = argv[1:] i = 0 while i < len(rest): a = rest[i] - if a == "--format": - if i + 1 >= len(rest): - print("--format requires a value: human|github|msbuild", - file=sys.stderr) - return 2 - fmt, i = rest[i + 1], i + 2 - continue - if a.startswith("--format="): - fmt, i = a.split("=", 1)[1], i + 1 + matched = False + for flag in opts: + if a == flag: + if i + 1 >= len(rest): + print(f"{flag} requires a value", file=sys.stderr) + return 2 + opts[flag], i = rest[i + 1], i + 2 + seen_value_flags = matched = True + break + if a.startswith(flag + "="): + opts[flag], i = a.split("=", 1)[1], i + 1 + seen_value_flags = matched = True + break + if matched: continue positional.append(a) i += 1 @@ -244,16 +255,24 @@ def main(argv: list[str]) -> int: if len(positional) != 1: print(__doc__) return 2 + fmt, severity = opts["--format"], opts["--severity"] if fmt not in _FORMATS: print(f"unknown --format {fmt!r} (choose: {', '.join(sorted(_FORMATS))})", file=sys.stderr) return 2 - if cmd != "ownir" and fmt != "human": - print("--format only applies to `ownir`", file=sys.stderr) + if severity not in _SEVERITIES: + print(f"unknown --severity {severity!r} (choose: " + f"{', '.join(sorted(_SEVERITIES))})", file=sys.stderr) + return 2 + # `--format`/`--severity` are ownir-only — reject them on other commands by + # *presence*, not just non-default value (so `check x --format human` is a + # clear error, not a silent no-op). + if cmd != "ownir" and seen_value_flags: + print("--format/--severity only apply to `ownir`", file=sys.stderr) return 2 path = positional[0] if cmd == "ownir": - return cmd_ownir(path, fmt) + return cmd_ownir(path, fmt, severity) return {"check": cmd_check, "emit": cmd_emit, "cfg": cmd_cfg, "report": cmd_report}[cmd](path) diff --git a/ownlang/di.py b/ownlang/di.py new file mode 100644 index 00000000..2d8a8556 --- /dev/null +++ b/ownlang/di.py @@ -0,0 +1,104 @@ +"""DI lifetime analysis — DI001, captive dependency (P-006). + +A **singleton** that depends — directly, or through **transient** services — on a +**scoped** service captures that scoped instance for the whole application +lifetime. The scoped service then outlives the scope it was meant to live in (a +"captive dependency"); in ASP.NET Core this is the classic *"Cannot consume +scoped service from singleton"* bug, and a `DbContext` held by a singleton is the +canonical example. + +This is a deterministic, static-friendly property of the **registration graph** +(who is registered with which lifetime, and who they depend on) — not of the +acquire/release lifetime model the rest of the core checks. So it lives in its +own small analyzer that the OwnIR bridge feeds registration facts to: one +checker, several analyses (the frontend still only *produces facts*). + +The rule (matching the .NET DI guidance): + + - singleton -> scoped : captive (the edge itself is the bug) + - singleton -> transient -> scoped : captive (the transient is resolved by + the singleton, so it is singleton-lived + and drags the scoped along) + - singleton -> singleton -> scoped : NOT reported here — the *inner* singleton + is the captor and is flagged on its own. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +SINGLETON = "singleton" +SCOPED = "scoped" +TRANSIENT = "transient" +LIFETIMES = frozenset({SINGLETON, SCOPED, TRANSIENT}) + + +@dataclass(frozen=True) +class Service: + """One DI registration: a service `name`, its `lifetime`, and the service + names it depends on (constructor injection). `file`/`line` point at the + registration site so a finding lands there.""" + + name: str + lifetime: str + deps: tuple[str, ...] = () + file: str = "?" + line: int = 0 + + +@dataclass(frozen=True) +class CaptiveDependency: + """A singleton capturing a scoped service, with the dependency path that + reaches it (singleton -> ... -> captured).""" + + singleton: str + captured: str + path: tuple[str, ...] + file: str + line: int + + @property + def message(self) -> str: + chain = " -> ".join(self.path) + return (f"singleton '{self.singleton}' captures scoped service " + f"'{self.captured}' (captive dependency: {chain})") + + +def find_captive_dependencies(services: list[Service]) -> list[CaptiveDependency]: + """Return every captive dependency in the registration graph. For each + singleton, walk its dependencies: an edge into a scoped service is a + violation (reported on the singleton); transients are followed (a transient + held by a singleton is itself singleton-lived); singletons are not followed + (the inner singleton is reported on its own pass). Cycles are guarded.""" + by_name = {s.name: s for s in services} + findings: list[CaptiveDependency] = [] + for s in services: + if s.lifetime != SINGLETON: + continue + reported: set[str] = set() + visited: set[str] = set() + # DFS over the dependency chain rooted at this singleton. + stack: list[tuple[str, tuple[str, ...]]] = [(s.name, (s.name,))] + while stack: + cur, path = stack.pop() + node = by_name.get(cur) + if node is None: + continue + for dep in node.deps: + dnode = by_name.get(dep) + if dnode is None: + continue + npath = (*path, dep) + if dnode.lifetime == SCOPED: + if dep not in reported: + reported.add(dep) + findings.append(CaptiveDependency( + singleton=s.name, captured=dep, path=npath, + file=s.file, line=s.line)) + continue # the violating edge is found; don't recurse past it + if dnode.lifetime == TRANSIENT and dep not in visited: + visited.add(dep) + stack.append((dep, npath)) + # a singleton dependency is safe here (captor reported on its own) + findings.sort(key=lambda f: (f.file, f.line, f.singleton, f.captured)) + return findings diff --git a/ownlang/ownir.py b/ownlang/ownir.py index bca376c7..466bfce9 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -49,6 +49,19 @@ `line`. The `resource`/`type` fields are additive and optional, so they do NOT bump `ownir_version`: an older core just reads every entry as a subscription. Region escape (OWN014) is later (see docs/proposals/P-004). + +An optional top-level `services` array carries the DI registration graph for the +DI001 captive-dependency check (P-006) — a separate core analysis (ownlang/di.py) +over who is registered with which lifetime and who they depend on:: + + "services": [ + {"name": "EmailSender", "lifetime": "singleton", "deps": ["AppDbContext"], + "file": "Startup.cs", "line": 12}, + {"name": "AppDbContext", "lifetime": "scoped", "deps": []} + ] + +A singleton that reaches a scoped service (directly, or through a transient) is a +DI001 finding at its registration site. The block is additive/optional too. """ from __future__ import annotations @@ -57,6 +70,8 @@ from dataclasses import dataclass from typing import Any +from .di import LIFETIMES as DI_LIFETIMES +from .di import Service, find_captive_dependencies from .diagnostics import Severity # The OwnIR schema version this core understands. Bump it whenever the fact @@ -133,35 +148,39 @@ class Finding: message: str kind: str = "subscription token" - def render(self) -> str: - return (f"{self.file}:{self.line}: error: [{self.code}] " + def render(self, severity: str = "error") -> str: + return (f"{self.file}:{self.line}: {severity}: [{self.code}] " f"{self.message} [resource: {self.kind}]") - def render_github(self) -> str: + def render_github(self, severity: str = "error") -> str: """A GitHub Actions workflow annotation. Printed on a CI step's stdout, - GitHub renders it as an inline error on the PR diff at the C# location. - `title` carries the OWN code; the message keeps the [resource:] tag.""" + GitHub renders it inline on the PR diff at the C# location. `severity` + is the annotation level (`error`/`warning`); `title` carries the OWN + code; the message keeps the [resource:] tag.""" msg = f"[{self.code}] {self.message} [resource: {self.kind}]" - return (f"::error file={_esc_prop(self.file)},line={self.line}," + return (f"::{severity} file={_esc_prop(self.file)},line={self.line}," f"title={_esc_prop(self.code)}::{_esc_data(msg)}") - def render_msbuild(self) -> str: + def render_msbuild(self, severity: str = "error") -> str: """The canonical MSBuild diagnostic format `file(line): error CODE: msg`. `dotnet build` and the Visual Studio Error List parse exactly this, so the findings surface in-IDE without a Roslyn analyzer — one checker, not - a second one reimplemented in C#.""" - return (f"{self.file}({self.line}): error {self.code}: " + a second one reimplemented in C#. `severity` picks `error`/`warning` so a + build can show them advisory instead of failing.""" + return (f"{self.file}({self.line}): {severity} {self.code}: " f"{self.message} [resource: {self.kind}]") -def render_finding(f: Finding, fmt: str) -> str: +def render_finding(f: Finding, fmt: str, severity: str = "error") -> str: """Render a finding in one of the supported surfaces: `human` (the default - CLI line), `github` (CI annotation), or `msbuild` (VS Error List).""" + CLI line), `github` (CI annotation), or `msbuild` (VS Error List). `severity` + is a presentation choice — the finding is still the core's verdict; it only + controls whether the host shows it as an error (default) or a warning.""" if fmt == "github": - return f.render_github() + return f.render_github(severity) if fmt == "msbuild": - return f.render_msbuild() - return f.render() + return f.render_msbuild(severity) + return f.render(severity) def load(path: str) -> dict[str, Any]: @@ -203,6 +222,28 @@ def load(path: str) -> dict[str, Any]: if t is not None and not isinstance(t, str): raise OwnIRError( f"subscription 'type' must be a string, got {t!r}") + # Optional DI registration graph (DI001 — captive dependency, P-006). Additive + # and optional: an older core simply ignores it. + svcs = result.get("services", []) + if not isinstance(svcs, list) or not all(isinstance(s, dict) for s in svcs): + raise OwnIRError("OwnIR 'services' must be a JSON array of objects") + for s in svcs: + lt = s.get("lifetime") + if lt not in DI_LIFETIMES: + raise OwnIRError( + f"service 'lifetime' must be one of {sorted(DI_LIFETIMES)}, " + f"got {lt!r}") + name = s.get("name") + if not isinstance(name, str) or not name: + raise OwnIRError("service 'name' must be a non-empty string") + deps = s.get("deps", []) + if not isinstance(deps, list) or not all(isinstance(d, str) for d in deps): + raise OwnIRError("service 'deps' must be an array of strings") + if not isinstance(s.get("file", "?"), str): + raise OwnIRError("service 'file' must be a string") + ln = s.get("line", 0) + if not isinstance(ln, int) or isinstance(ln, bool): + raise OwnIRError("service 'line' must be an integer") return result @@ -329,5 +370,43 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: file=sub["file"], line=int(sub.get("line", 0)), code=d.code, component=component, event=event, handler=handler, message=message, kind=kind)) + + # DI001 (captive dependency): a separate core analysis over the registration + # graph, not the acquire/release model — the bridge just routes the facts to + # it (see ownlang/di.py). Findings carry the registration site as file/line. + findings.extend(_di_findings(facts)) + findings.sort(key=lambda f: (f.file, f.line, f.code)) return findings + + +def _as_int(v: Any) -> int: + """A non-throwing int coercion: load() already validates `line`, but + check_facts may be called directly (tests, embedders) on un-validated facts, + so a bad `line` degrades to 0 rather than raising a bare ValueError.""" + return v if isinstance(v, int) and not isinstance(v, bool) else 0 + + +def _di_findings(facts: dict[str, Any]) -> list[Finding]: + """Run the DI captive-dependency check over the facts' `services` graph and + map each result to a DI001 Finding at its registration site.""" + raw = facts.get("services", []) + if not isinstance(raw, list): + return [] + services = [ + Service( + name=str(s.get("name", "?")), + lifetime=str(s.get("lifetime", "")), + deps=tuple(s.get("deps", [])), + file=str(s.get("file", "?")), + line=_as_int(s.get("line", 0)), + ) + for s in raw if isinstance(s, dict) + ] + return [ + Finding( + file=c.file, line=c.line, code="DI001", + component=c.singleton, event=c.captured, handler="", + message=c.message, kind="DI lifetime") + for c in find_captive_dependencies(services) + ] diff --git a/scripts/own-check.ps1 b/scripts/own-check.ps1 new file mode 100644 index 00000000..1a49925b --- /dev/null +++ b/scripts/own-check.ps1 @@ -0,0 +1,77 @@ +<# +.SYNOPSIS + own-check — run the Own.NET C# leak check over a path (Windows/PowerShell). + +.DESCRIPTION + The PowerShell twin of own-check.sh, for Windows/Visual Studio users who have + no bash. Chains the two stages of the P-001 pipeline into one command: + + *.cs --[OwnSharp.Extractor (Roslyn)]--> facts.json --[python -m ownlang ownir]--> findings + + There is one checker — the Python core; the C# side only extracts facts. + Requires a .NET SDK (`dotnet`) and Python 3.11+ on PATH. + +.PARAMETER Root + The Own.NET checkout (where the extractor + ownlang live). Defaults to the + repo this script lives in (scripts\..). + +.PARAMETER Format + Finding surface: human (default), github, or msbuild (Visual Studio Error List). + +.PARAMETER Severity + How a host shows findings: error (default) or warning (advisory). + +.PARAMETER FailOnFinding + Exit non-zero (the core's code) when any leak is found. + +.PARAMETER Paths + Files or directories to scan (directories are walked for *.cs). Defaults to ".". + +.EXAMPLE + scripts\own-check.ps1 -Format msbuild -- src\MyApp +.EXAMPLE + scripts\own-check.ps1 -Format github -Severity warning -FailOnFinding -- . +#> +[CmdletBinding()] +param( + [string]$Root, + [string]$Format = "human", + [string]$Severity = "error", + [switch]$FailOnFinding, + [Parameter(ValueFromRemainingArguments = $true)] + [string[]]$Paths +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" + +# Default root = the Own.NET checkout this script lives in (scripts\..). +if ([string]::IsNullOrEmpty($Root)) { + $Root = Split-Path -Parent $PSScriptRoot +} +# A bare "--" separator (shell habit) is harmless; drop it. +if ($Paths) { $Paths = @($Paths | Where-Object { $_ -ne "--" }) } +if (-not $Paths -or $Paths.Count -eq 0) { $Paths = @(".") } + +$extractor = Join-Path $Root "frontend\roslyn\OwnSharp.Extractor" +$facts = New-TemporaryFile +try { + # Stage 1: extract facts. dotnet's build chatter is sent to the host (not + # stdout) so stdout stays clean for the host-parseable findings; -o writes + # the facts to a file. + & dotnet run --project $extractor -- @Paths -o $facts.FullName 1>$null + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + + # Stage 2: the one checker produces the verdict at the C# location. + $env:PYTHONPATH = $Root + & python -m ownlang ownir $facts.FullName --format $Format --severity $Severity + $rc = $LASTEXITCODE +} +finally { + Remove-Item $facts.FullName -ErrorAction SilentlyContinue +} + +# rc: 0 = clean, 1 = findings, >=2 = a hard error (bad facts / drifted contract). +if ($FailOnFinding) { exit $rc } +if ($rc -ge 2) { exit $rc } +exit 0 diff --git a/scripts/own-check.sh b/scripts/own-check.sh index 18b5e227..c392073a 100755 --- a/scripts/own-check.sh +++ b/scripts/own-check.sh @@ -11,11 +11,13 @@ # only extracts facts. # # Usage: -# scripts/own-check.sh [--format human|github|msbuild] [--fail-on-finding] -# [--root ] [--] [more ...] +# scripts/own-check.sh [--format human|github|msbuild] [--severity error|warning] +# [--fail-on-finding] [--root ] +# [--] [more ...] # -# Defaults: --format human, scans ".", does not fail the shell on findings, -# --root is the repo this script lives in. With --fail-on-finding the exit code +# Defaults: --format human, --severity error, scans ".", does not fail the shell +# on findings, --root is the repo this script lives in. --severity picks how a +# host shows findings (warning = advisory). With --fail-on-finding the exit code # is the core's (1 = leaks found). A hard error (bad facts) always exits non-zero. # # Requirements: a .NET SDK (`dotnet`) and Python 3.11+ on PATH. @@ -24,6 +26,7 @@ set -euo pipefail root="" format="human" +severity="error" fail_on_finding=0 paths=() @@ -35,6 +38,9 @@ while [[ $# -gt 0 ]]; do --format) [[ $# -ge 2 ]] || { echo "own-check: --format requires a value" >&2; exit 2; } format="$2"; shift 2 ;; + --severity) + [[ $# -ge 2 ]] || { echo "own-check: --severity requires a value" >&2; exit 2; } + severity="$2"; shift 2 ;; --fail-on-finding) fail_on_finding=1; shift ;; --) shift; while [[ $# -gt 0 ]]; do paths+=("$1"); shift; done ;; -h|--help) sed -n '2,30p' "$0"; exit 0 ;; @@ -60,7 +66,7 @@ dotnet run --project "$extractor" -- "${paths[@]}" -o "$facts" 1>&2 # Stage 2: the one checker produces the verdict at the C# location. set +e -PYTHONPATH="$root" python -m ownlang ownir "$facts" --format "$format" +PYTHONPATH="$root" python -m ownlang ownir "$facts" --format "$format" --severity "$severity" rc=$? set -e diff --git a/tests/fixtures/ownir/di.facts.json b/tests/fixtures/ownir/di.facts.json new file mode 100644 index 00000000..a3d0e1c2 --- /dev/null +++ b/tests/fixtures/ownir/di.facts.json @@ -0,0 +1,13 @@ +{ + "ownir_version": 0, + "module": "DiDemo", + "components": [], + "services": [ + {"name": "EmailSender", "lifetime": "singleton", "file": "Startup.cs", "line": 12, "deps": ["AppDbContext"]}, + {"name": "AppDbContext", "lifetime": "scoped", "file": "Startup.cs", "line": 13, "deps": []}, + {"name": "Clock", "lifetime": "singleton", "file": "Startup.cs", "line": 14, "deps": []}, + {"name": "ReportService", "lifetime": "singleton", "file": "Startup.cs", "line": 15, "deps": ["UnitOfWork"]}, + {"name": "UnitOfWork", "lifetime": "transient", "file": "Startup.cs", "line": 16, "deps": ["AppDbContext"]}, + {"name": "RequestLog", "lifetime": "scoped", "file": "Startup.cs", "line": 17, "deps": ["AppDbContext"]} + ] +} diff --git a/tests/test_ownir.py b/tests/test_ownir.py index ee354643..93add257 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -48,6 +48,8 @@ "ownir", "pool.facts.json") _LOCAL_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", "ownir", "local_disposable.facts.json") +_DI_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", + "ownir", "di.facts.json") def _write_facts(obj: dict) -> str: @@ -234,6 +236,65 @@ def run() -> int: if "[resource: disposable]" not in l0.render(): fails.append(f"local finding missing kind tag: {l0.render()!r}") + # --- DI001 captive dependency (P-006): a singleton capturing a scoped + # service (directly or through a transient) is flagged at its + # registration site; safe registrations stay silent. + from ownlang.di import Service, find_captive_dependencies + + # unit: the graph check itself (singleton->scoped and singleton->transient-> + # scoped are captive; singleton->singleton->scoped and scoped->scoped are not). + svcs = [ + Service("A", "singleton", ("B",)), # A -> scoped B : captive + Service("B", "scoped", ()), + Service("C", "singleton", ("T",)), # C -> transient -> scoped : captive + Service("T", "transient", ("B",)), + Service("D", "singleton", ("E",)), # D -> singleton E : safe here + Service("E", "singleton", ("B",)), # E -> scoped B : E's own bug + Service("F", "scoped", ("B",)), # scoped -> scoped : safe + ] + captives = find_captive_dependencies(svcs) + checks += 1 + captors = sorted((c.singleton, c.captured) for c in captives) + if captors != [("A", "B"), ("C", "B"), ("E", "B")]: + fails.append(f"captive-dependency set wrong: {captors}") + checks += 1 + cpath = next((c.path for c in captives if c.singleton == "C"), None) + if cpath != ("C", "T", "B"): + fails.append(f"transitive captive path wrong: {cpath}") + + # bridge: the fixture surfaces exactly the two captive singletons as DI001 + # at their registration lines; the clock/scoped-to-scoped stay silent. + with open(_DI_FIXTURE, encoding="utf-8") as f: + difacts = json.load(f) + difindings = check_facts(difacts) + checks += 1 + di = sorted((x.component, x.line, x.code) for x in difindings + if x.code == "DI001") + if di != [("EmailSender", 12, "DI001"), ("ReportService", 15, "DI001")]: + fails.append(f"DI001 findings wrong: {di}") + checks += 1 + if any(x.component == "Clock" for x in difindings): + fails.append("a dependency-free singleton was wrongly flagged") + checks += 1 + em = next((x for x in difindings if x.component == "EmailSender"), None) + if em is None or "captures scoped service 'AppDbContext'" not in em.message: + fails.append(f"DI001 message missing captive text: " + f"{em.message if em else None!r}") + checks += 1 + # an unknown lifetime must fail loudly at load (external input). + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"name": "X", "lifetime": "perpetual"}]}): + fails.append("an invalid service lifetime did not raise OwnIRError") + checks += 1 + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"lifetime": "singleton"}]}): + fails.append("a missing/empty service name did not raise OwnIRError") + checks += 1 + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"name": "X", "lifetime": "singleton", + "line": "NaN"}]}): + fails.append("a non-integer service line did not raise OwnIRError") + # --- output surfaces (Уровень 1): the same finding renders for a human, a # GitHub annotation, and an MSBuild/VS Error List line. The format lives # in the core (one checker), so the Action/script stay thin wrappers. @@ -264,6 +325,23 @@ def run() -> int: if "a%2Cb%3Ac.cs" not in g2 or "%0A" not in g2 or "50%25 off" not in g2: fails.append(f"github render did not escape metacharacters: {g2!r}") + # --severity is a presentation choice: warning renders ::warning / : warning: + # / warning:, error (default) is unchanged. + checks += 1 + if not render_finding(fnd, "github", "warning").startswith( + "::warning file=src/A.cs,line=42,"): + fails.append("github render did not honor severity=warning") + checks += 1 + if "src/A.cs(42): warning OWN001:" not in render_finding(fnd, "msbuild", "warning"): + fails.append("msbuild render did not honor severity=warning") + checks += 1 + if "src/A.cs:42: warning: [OWN001]" not in render_finding(fnd, "human", "warning"): + fails.append("human render did not honor severity=warning") + checks += 1 + # the default stays error (no accidental severity drift). + if not render_finding(fnd, "msbuild").startswith("src/A.cs(42): error OWN001:"): + fails.append("msbuild default severity should remain error") + for f in fails: print(f"OWNIR FAIL: {f}") print(f"ownir: {checks - len(fails)}/{checks} bridge checks passed")