|
| 1 | +# P-025 — Obligation protocols (`Own.Protocols`) — barrier-sensitive project invariants |
| 2 | + |
| 3 | +- **Status:** first slice built (core + bridge + spec + fixtures, `OBL001–005` |
| 4 | + end-to-end over hand-written facts); the Roslyn extractor slice is designed |
| 5 | + below but **not** implemented (this sandbox has no dotnet; extractor work is |
| 6 | + CI-validated). |
| 7 | +- **Built:** [`ownlang/obligations.py`](../../ownlang/obligations.py) (the |
| 8 | + path-sensitive checker), the `protocols[]` / `protocol_functions[]` OwnIR |
| 9 | + blocks ([spec/OwnIR.md §8](../../spec/OwnIR.md)), the `OBL001–OBL005` codes, |
| 10 | + [`tests/test_obligations.py`](../../tests/test_obligations.py) (64 checks), |
| 11 | + and the `protocol_isloaded_*` killer-demo fixtures. |
| 12 | +- **Depends on:** [spec/OwnIR.md](../../spec/OwnIR.md) (the facts seam), |
| 13 | + [P-016](P-016-deep-fact-extraction.md) (the flow lowering the extractor slice |
| 14 | + reuses), [P-006](P-006-di-lifetimes.md)/[P-020](P-020-ownts-react-effects.md) |
| 15 | + (the sidecar-analysis precedent this copies). |
| 16 | +- **Relation to [P-010](P-010-type-disciplines.md):** P-010's `protocol` blocks |
| 17 | + are *typestate on an object across its lifetime* (state machines, consume-self |
| 18 | + transitions on the affine core). P-025 is deliberately smaller: *temporal |
| 19 | + obligations inside a method*, checked against project-declared barriers. P-010 |
| 20 | + can later subsume these rules; nothing here blocks it. |
| 21 | + |
| 22 | +## Motivation — the invariant the type system cannot know |
| 23 | + |
| 24 | +A legacy WPF method breaks its own invariant on purpose, briefly: |
| 25 | + |
| 26 | +```csharp |
| 27 | +IsLoaded = false; // the document tree is now inconsistent — on purpose |
| 28 | +RebuildIndexes(); |
| 29 | +if (hasWarnings) |
| 30 | + OnPropertyChanged(nameof(Document)); // ← published the broken object |
| 31 | +IsLoaded = true; |
| 32 | +OnPropertyChanged(nameof(Document)); // this one is fine |
| 33 | +``` |
| 34 | + |
| 35 | +`IsLoaded = false` is not a bug; **publishing the object while the flag is |
| 36 | +down** is. No general checker can know that `IsLoaded` means "the document is |
| 37 | +consistent", that `PropertyChanged("Document")` hands the object to bindings |
| 38 | +*right now*, or that `PropertyChanged("Progress")` is harmless meanwhile. That |
| 39 | +knowledge is project-specific. Existing tools stop exactly here: analyzers know |
| 40 | +universal protocols (dispose your `IDisposable`, unsubscribe your event); |
| 41 | +NDepend/CodeQL can query structure but have no barrier-sensitive obligation |
| 42 | +model; typestate research languages don't speak legacy C#. The niche is real: |
| 43 | +**barrier-sensitive, project-specific obligation checking for code review** — |
| 44 | +and the OwnAudit STS corpus already shows the shape in the wild (17k |
| 45 | +INPC findings, 8 recorded `IsLoaded` findings, `BrokerDataClasses` as the |
| 46 | +subscription-leak epicenter). |
| 47 | + |
| 48 | +The same three verbs cover the whole family: |
| 49 | + |
| 50 | +```text |
| 51 | +IsLoaded=false must become true before PropertyChanged(Document) |
| 52 | +_suppressNotifications must be restored before return/throw |
| 53 | +BeginUpdate must meet EndUpdate before Refresh / method exit |
| 54 | +SuspendCalculation must be resumed before results are published |
| 55 | +``` |
| 56 | + |
| 57 | +## The model — obligation / barrier / require-closed-before |
| 58 | + |
| 59 | +One protocol = three matchers and a scope (the full shape and its normative |
| 60 | +semantics live in [spec/OwnIR.md §8](../../spec/OwnIR.md)): |
| 61 | + |
| 62 | +- **opens** — the event that creates the obligation (`IsLoaded = false`, or a |
| 63 | + call: `BeginUpdate()`); |
| 64 | +- **closes** — the event that discharges it; |
| 65 | +- **barriers** — events it must not cross while open: configured calls (with an |
| 66 | + optional distinguished-argument set, so `OnPropertyChanged` can be unsafe for |
| 67 | + `Document` but allowed for `Progress`) plus, by default, every method exit |
| 68 | + (`return`, `throw`, falling off the end — the OWN001 shape). |
| 69 | + |
| 70 | +The checker ([`ownlang/obligations.py`](../../ownlang/obligations.py)) walks the |
| 71 | +method's ordered event tree path-sensitively; the obligation state is a set over |
| 72 | +{OPEN, CLOSED} joined by union at merges, so **definite vs maybe** falls out of |
| 73 | +the lattice exactly as OWN002 vs OWN009 do. Loops are solved to a local fixpoint |
| 74 | +and emit once. Findings carry the ordered evidence slice — *opened here → barrier |
| 75 | +fired here → closed only here, after the barrier* — which SARIF renders as a |
| 76 | +click-through `codeFlows` trace. |
| 77 | + |
| 78 | +| Code | Meaning | |
| 79 | +|------|---------| |
| 80 | +| OBL001 | obligation still open when a barrier fires (every path) | |
| 81 | +| OBL002 | obligation may still be open at a barrier (some path) | |
| 82 | +| OBL003 | obligation not closed before the method exits (every path) | |
| 83 | +| OBL004 | obligation may not be closed before an exit (some path) | |
| 84 | +| OBL005 | advisory: a protocol's scope matched no reported method (dead rule) | |
| 85 | + |
| 86 | +## Precision policy (the standing red line, applied here) |
| 87 | + |
| 88 | +False positives kill this feature faster than any competitor — a rule that |
| 89 | +cries on every `IsLoaded=false` gets switched off like a smoke alarm that hates |
| 90 | +toast. Three normative rules (all tested): |
| 91 | + |
| 92 | +1. **Never invent.** An opaque write to a tracked flag (`IsLoaded = Compute()`) |
| 93 | + may *discharge* an open obligation (state gains CLOSED → the crossing |
| 94 | + degrades to a *maybe*) but never *creates* one. |
| 95 | +2. **Unnamed calls are neutral.** A call the protocol doesn't mention neither |
| 96 | + discharges nor crosses. A callee that flips the flag internally is invisible |
| 97 | + in v1 — that is the phase-3 interprocedural slice, not a v1 guess. |
| 98 | +3. **Scope is the throttle.** `scope.methods` restricts a rule to named |
| 99 | + methods; the MVP posture is *one protocol, one method, one historical bug*. |
| 100 | + A scoped rule matching nothing is surfaced (OBL005), not silently dead. |
| 101 | + |
| 102 | +## Why this shape (decisions on the record) |
| 103 | + |
| 104 | +- **Sidecar analysis, not new core instructions.** `di.py`/`effects.py` set the |
| 105 | + pattern: a fact family + a small core analysis routed via `check_facts`. The |
| 106 | + alternative (new `Instr` variants in `cfg.py`) touches the frozen |
| 107 | + `cfg_json.py` oracle seam, `codegen.py`, the grammar, and the Rust mirror — |
| 108 | + all for no v1 gain. Revisit when protocols need loans/RID interplay. |
| 109 | +- **Additive OwnIR blocks, no version bump.** `services` and `effects` landed |
| 110 | + additively at v0; `protocols`/`protocol_functions` follow the same IR3 rule. |
| 111 | + An older core ignores them; their internal vocabularies (`ev`, matcher |
| 112 | + `kind`) are fail-loud per IR4 and version *with the blocks*. |
| 113 | +- **Rules are data, not a language.** The chat-derived requirement is explicit: |
| 114 | + nobody wants to learn OwnLang — including its author. Protocols are declared |
| 115 | + as JSON facts (later: generated from attributes/inference and *approved*, see |
| 116 | + the roadmap), never hand-written `.own`. OwnLang stays what Own.NET |
| 117 | + understands, not what users write. |
| 118 | +- **Messages are line-free.** OwnAudit fingerprints findings on |
| 119 | + (path, rule, message) for the baseline ratchet and the FP-judge overlay; a |
| 120 | + line number in the message would break both on every unrelated edit. Lines |
| 121 | + live in the evidence slice. |
| 122 | + |
| 123 | +## The extractor slice (designed, not built — needs CI/dotnet) |
| 124 | + |
| 125 | +`OwnSharp.Extractor` already collects everything required; the slice is |
| 126 | +emission, not analysis (one checker: the extractor reports, the core decides): |
| 127 | + |
| 128 | +1. **Events.** Extend the P-016 flow lowering (`LowerFlowStmt`/`EmitFlowExpr`, |
| 129 | + with its `onReturn`/`onThrow` continuation threading, so `finally` and |
| 130 | + exceptional paths come sound for free) to emit `protocol_functions[].events` |
| 131 | + for methods in some protocol's scope: member assigns with literal boolean |
| 132 | + RHS (`AssignedFieldName`/`ThisFieldName` already normalize the LHS; a |
| 133 | + non-literal RHS emits an opaque assign with no `value`), self-calls with a |
| 134 | + `nameof(X)`/string-literal first argument as `{"ev":"call","arg":"X"}` |
| 135 | + (`SelfCallName` already recognizes the receiver), and `return`/`throw`. |
| 136 | + Scope-gating keeps the facts file small and the honest-skip discipline |
| 137 | + (`methods_skipped_unmodelled`) carries over. |
| 138 | +2. **Rules.** A project file (e.g. `.own-protocols.json`, schema = |
| 139 | + `$defs/protocol`) merged into the facts by `own-check.sh` — configuration |
| 140 | + travels with the repo, not the tool invocation. |
| 141 | +3. **CI.** A `samples/LoadingProtocolSample.cs` + grep assertions in the |
| 142 | + `wpf-extractor` job, and a corpus case once real-world instances are mined |
| 143 | + (the OwnAudit STS stand is the natural first target). |
| 144 | + |
| 145 | +## Roadmap (each phase lands only after the previous one holds on real code) |
| 146 | + |
| 147 | +1. **v1 (this slice):** core + bridge + fixtures. Killer demo: |
| 148 | + `python -m ownlang ownir tests/fixtures/ownir/protocol_isloaded_violation.facts.json` |
| 149 | + → `OBL001` at `BigDocumentViewModel.cs:241` with the three-hop path. |
| 150 | +2. **Extractor emission** (above) — the same demo on real C#. |
| 151 | +3. **Interprocedural obligations:** per-method summaries |
| 152 | + (`mayOpen/mustClose/mayCross` per protocol) on the MOS/SCC channel of |
| 153 | + [`ownership.py`](../../ownlang/ownership.py), so `ApplyWarnings()` that |
| 154 | + notifies internally stops being invisible. Same tier ladder as D5 |
| 155 | + (inferred → curated → annotation). |
| 156 | +4. **Authoring surfaces:** `[OwnProtocol]`-style C# attributes and/or inferred |
| 157 | + candidate protocols ("in 27 places `IsLoaded=false` … `true` precedes the |
| 158 | + Document notify; 2 places violate — adopt this rule?") emitted as *suggested* |
| 159 | + config a human approves and commits. |
| 160 | +5. **Consumption:** OwnAudit picks OBL findings up as canonical finding records |
| 161 | + (SARIF evidence/codeFlows already flow through `report/sarif.py`; register |
| 162 | + the category for severity mapping and the runtime correlator), and the |
| 163 | + diff-aware baseline gate makes them review-time signals ("fail only new |
| 164 | + violations"). |
| 165 | + |
| 166 | +## Non-goals |
| 167 | + |
| 168 | +- **Not a general temporal-logic engine.** No LTL, no arbitrary predicates, no |
| 169 | + cross-object protocols. Three verbs and a scope; the moment a rule needs a |
| 170 | + formula, it is a P-010/P-002 customer. |
| 171 | +- **Not typestate.** No per-object state machines, no consume-self transitions, |
| 172 | + no aliasing of obligation carriers (the protocol tracks *the method's own* |
| 173 | + flags/calls; `this`-aliasing is out of scope for v1 by construction, and the |
| 174 | + RID machinery exists when that changes). |
| 175 | +- **Not a DSL for people to write.** Facts in, findings out. Any future |
| 176 | + human-facing surface is attributes or approved generated config. |
| 177 | +- **Not on by default anywhere.** No built-in protocol ships with the tool; an |
| 178 | + empty `protocols[]` means the analysis does not exist for that repo. |
| 179 | + |
| 180 | +## Open questions |
| 181 | + |
| 182 | +1. **`await` as a barrier.** During an `await` the broken state is observable |
| 183 | + by the UI thread; is that a barrier by default, opt-in |
| 184 | + (`{"kind": "await"}` in `barriers`), or a per-protocol flag? (The extractor |
| 185 | + currently skips most async bodies anyway — honest-skip.) |
| 186 | +2. **Cross-member protocols** (open in `BeginLoad`, close in `OnLoaded`): needs |
| 187 | + obligation state on the *component*, not the method — the RID model fits, |
| 188 | + but the facts shape does not yet. |
| 189 | +3. **Suggested-protocol mining:** does inference live in the core (over |
| 190 | + `protocol_functions` without rules) or in OwnAudit (over the corpus)? |
0 commit comments