diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2af56b3..d30d4011 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2205,6 +2205,22 @@ jobs: echo "FAIL: flag-off facts drifted from the pre-S0 golden (byte parity broken)"; exit 1 fi echo "OK: fix-candidate metadata correct; flag-off is byte-identical to the pre-S0 golden" + # ADDITIVE means the flag never REMOVES anything either, and the sample above + # cannot show that: it has subscriptions and no other section. For as long as + # `orphaned_awaitables` existed, the flag-on envelope dropped it — every OWN053 + # site gone from the facts — with this step green. So the same invariant + # (flag-on minus the S0 fields == flag-off, the WHOLE document) is checked on a + # fixture that makes every top-level section the extractor can write non-empty + # under --flow-locals. The checker reads that section list off the extractor's + # own envelope and refuses a pair that leaves one out, so a new section cannot + # arrive without being covered here. + ad=tests/fixtures/fix_candidates/AdditiveSections.cs + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + "$ad" --flow-locals --fix-candidates -o "$RUNNER_TEMP/ad_on.json" + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + "$ad" --flow-locals -o "$RUNNER_TEMP/ad_off.json" + python tests/check_fix_candidates_facts.py --additive-sections \ + "$RUNNER_TEMP/ad_on.json" "$RUNNER_TEMP/ad_off.json" # S0 Part B: the `own-fix subscriptions candidates` collector turns the fix # metadata into a deterministic candidates.json (analysis-only). Reuses fc_on.json. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 10e5074d..bf923259 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -7280,47 +7280,39 @@ or ImplicitObjectCreationExpressionSyntax } init methods_flow_analysed = statMethodsAnalysed, methods_skipped_unmodelled = statMethodsSkipped, }; -// `fix_candidates_version` is a top-level ADDITIVE metadata field, present ONLY under -// --fix-candidates; it does not move `ownir_version` (the fact-schema vocabulary is -// unchanged — no new resource-kind or analysis-routing value). Without the flag the -// object is byte-for-byte the pre-S0 shape. +// ONE envelope, built once, in the key order the facts have always been written. A section +// that is not always there is ONE conditional line here — never a second envelope. There +// used to be three, picked by a nested conditional (`--fix-candidates` ? A : orphans ? B : C), +// and each additive section had to be remembered in every one of them: the `--fix-candidates` +// envelope was written before `orphaned_awaitables` existed and never got it, so the flag +// that only ADDS fix metadata silently removed every OWN053 site from the facts. // -// Every envelope below stamps the SAME `ownir_version`, the core's current one -// (ownlang/ownir.py OWNIR_VERSION; tests/test_ownir.py reads every stamp in this file). -object facts = emitFixCandidates - ? new - { - ownir_version = 1, - fix_candidates_version = 1, - module = "Extracted", - components, - services = factServices, - functions = flowFunctions, - stats = factStats, - } - // OWN053 (promoted from ownership-semantics-lab H-29): an ADDITIVE top-level list of orphaned awaitables from which - // both engines mint the advisory; absent when there is no site, so such a document stays byte-identical to the - // pre-OWN053 shape (and `ownir_version` does not move: the field is additive, like `fix_candidates_version`). - : OrphanedAwaitables.Sites.Count > 0 - ? new - { - ownir_version = 1, - module = "Extracted", - components, - services = factServices, - functions = flowFunctions, - stats = factStats, - orphaned_awaitables = OrphanedAwaitables.Sites.OrderBy(o => JsonSerializer.Serialize(o), StringComparer.Ordinal).ToList(), - } - : new - { - ownir_version = 1, - module = "Extracted", - components, - services = factServices, - functions = flowFunctions, - stats = factStats, - }; +// Every section is written as `facts[""] = ...` and nowhere else: the S0 additivity +// check (tests/check_fix_candidates_facts.py) reads the section list off these lines, so a +// section added here is one its fixture must exercise. +// +// `ownir_version` is the core's current one (ownlang/ownir.py OWNIR_VERSION). It is spelled +// as this one literal on purpose: tests/test_ownir.py (IR2) reads every stamp in this file. +const int ownir_version = 1; +var facts = new Dictionary(); +facts["ownir_version"] = ownir_version; +// S0: ADDITIVE metadata, present ONLY under --fix-candidates. It does not move the version +// (no new resource-kind or analysis-routing value), and removing the S0 fields from a +// flag-on document gives the flag-off document — every other section included +// (tests/check_fix_candidates_facts.py holds exactly that). +if (emitFixCandidates) + facts["fix_candidates_version"] = 1; +facts["module"] = "Extracted"; +facts["components"] = components; +facts["services"] = factServices; +facts["functions"] = flowFunctions; +facts["stats"] = factStats; +// OWN053 (promoted from ownership-semantics-lab H-29): an ADDITIVE top-level list of orphaned +// awaitables from which both engines mint the advisory. Absent when there is no site, so such +// a document stays byte-identical to the pre-OWN053 shape. +if (OrphanedAwaitables.Sites.Count > 0) + facts["orphaned_awaitables"] = OrphanedAwaitables.Sites + .OrderBy(o => JsonSerializer.Serialize(o), StringComparer.Ordinal).ToList(); // P-037 A2.1: a sidecar record that does not satisfy its own vocabulary is a producer // defect, and a producer defect must not become a facts file. Refuse the whole run // (exit 2, the launcher's "extraction failed, no verdict was produced" tier) rather diff --git a/spec/OwnIR.md b/spec/OwnIR.md index 405c59f3..559cae8f 100644 --- a/spec/OwnIR.md +++ b/spec/OwnIR.md @@ -89,6 +89,16 @@ A newer extractor that introduces either against an un-bumped core therefore fai the run instead of mis-analyzing it — which is why both **must** bump `OWNIR_VERSION` per the table above. +**Additive is a promise about the producer too.** A producer option that adds +metadata must change nothing else in the document it writes. The extractor's +`--fix-candidates` adds `fix_candidates_version`, the component shape fields and +a `fix` block per subscription; take those out of a flag-on document and what is +left **is** the flag-off document — every section, `orphaned_awaitables` (§9) +included. The extractor builds its envelope once for that reason (one conditional +line per optional section, never a second envelope), and +`tests/check_fix_candidates_facts.py --additive-sections` holds the equality on a +fixture that exercises every top-level section the extractor can write. + ## 3. What OwnIR is not Verdict logic never lives in a frontend. The core's diagnostics (OWN0xx) come diff --git a/tests/check_fix_candidates_facts.py b/tests/check_fix_candidates_facts.py index 9e5fac72..35e2fc20 100644 --- a/tests/check_fix_candidates_facts.py +++ b/tests/check_fix_candidates_facts.py @@ -1,14 +1,27 @@ -"""Assert the S0 `--fix-candidates` extractor metadata on FixCandidatesSample.cs. +"""Assert the S0 `--fix-candidates` extractor contract at the fact level. Not a ``test_*`` (it needs the C# extractor to produce the facts, so CI runs the -extractor first and passes the JSON path). Encodes the Part-A extractor contract -at the fact level; exits non-zero on any violation. +extractor first and passes the JSON paths). Exits non-zero on any violation. Usage: python tests/check_fix_candidates_facts.py [] + python tests/check_fix_candidates_facts.py --additive-sections fix_on = FixCandidatesSample.cs scanned WITH --fix-candidates off = the SAME sample WITHOUT the flag (optional; asserts NO fix metadata leaks) + +The first form checks the metadata itself on FixCandidatesSample.cs. The second +checks the other half of the contract — the flag only ADDS — on +tests/fixtures/fix_candidates/AdditiveSections.cs scanned with `--flow-locals`, +flag on and flag off: + + flag-on minus the S0 fields == flag-off (the whole document) + +and refuses a pair that does not exercise every top-level section the extractor +can write (read off the extractor's own envelope), so that the equality cannot +be satisfied by a document with nothing in it. It was: the first form's sample +has subscriptions and no other section, and a flag-on envelope that dropped +`orphaned_awaitables` — every OWN053 site — passed it for as long as it existed. """ from __future__ import annotations @@ -16,11 +29,19 @@ import copy import json import os +import re import sys sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) -from ownlang.ownir import OWNIR_VERSION +from ownlang.ownir import OWNIR_VERSION, OwnIRError, check_facts + +_ROOT = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") +# The ONLY top-level field S0 adds. Everything else under --fix-candidates lives inside +# `components[]` (see `_strip_additive`); a second top-level S0 field is a contract change +# and belongs here, in the open. +_ADDITIVE_TOP_LEVEL = ("fix_candidates_version",) +_EXTRACTOR = os.path.join(_ROOT, "frontend", "roslyn", "OwnSharp.Extractor", "Program.cs") _ADDITIVE_COMPONENT_KEYS = ( "qualified_name", @@ -32,9 +53,15 @@ def _strip_additive(facts: dict) -> dict: - """The flag-ON facts with every S0-additive field removed.""" + """The flag-ON facts with every S0-additive field removed — and nothing else. + + This is the whole list of what `--fix-candidates` is allowed to change. It is an + ALLOWLIST of S0's own fields, so the comparison it feeds is over the complete + document: a section the flag drops, reorders or rewrites is a difference, whether + or not anyone thought to name that section in a test.""" f = copy.deepcopy(facts) - f.pop("fix_candidates_version", None) + for k in _ADDITIVE_TOP_LEVEL: + f.pop(k, None) for c in f.get("components", []): for k in _ADDITIVE_COMPONENT_KEYS: c.pop(k, None) @@ -62,6 +89,111 @@ def _fixes(facts: dict, name: str) -> list[dict]: return [s["fix"] for s in (comp.get("subscriptions") or []) if s.get("fix")] +def _additivity_problem(on: dict, off: dict) -> str | None: + """`flag-on minus the S0 fields == flag-off`, or what differs.""" + stripped = _strip_additive(on) + detail = [] + for k in sorted(k for k in set(stripped) | set(off) if stripped.get(k) != off.get(k)): + if k not in stripped: + detail.append(f"`{k}` is in the flag-off facts and MISSING from the flag-on facts") + elif k not in off: + detail.append(f"`{k}` appears only in the flag-on facts and is not an S0 field") + else: + detail.append(f"`{k}` differs") + if not detail and list(stripped) != list(off): + detail.append(f"the top-level key ORDER differs: {list(stripped)} vs {list(off)}") + if not detail: + return None + return ("flag-on minus the S0 fields must equal flag-off: " + "; ".join(detail) + + " — --fix-candidates only adds, it never removes or changes another section") + + +def _extractor_sections() -> list[str]: + """The top-level keys the extractor's envelope can write, read from its source. + + The envelope is built in one place as `facts[""] = ...`. Reading the keys from + there, instead of listing them here, is what makes a NEW section fail this check + until the fixture exercises it.""" + with open(_EXTRACTOR, encoding="utf-8") as fh: + keys = re.findall(r'\bfacts\["([a-z_]+)"\]\s*=', fh.read()) + return list(dict.fromkeys(keys)) + + +def _findings(facts: dict) -> list[tuple[object, ...]] | str: + try: + return sorted((f.code, f.file, f.line, f.column or 0, f.severity, f.message) + for f in check_facts(facts)) + except OwnIRError as e: + return f"refused: {e}" + + +def additive_sections(on_path: str, off_path: str) -> int: + """`--additive-sections`: the flag only adds, over EVERY section the extractor writes.""" + on, off = _load(on_path), _load(off_path) + fails: list[str] = [] + + # 1. the pair really is flag-on / flag-off, at the current version + if on.get("fix_candidates_version") != 1 or "fix_candidates_version" in off: + fails.append("the pair is not (flag-on, flag-off): fix_candidates_version must be 1 " + "in the first document and absent from the second") + components = on.get("components") + if not any(s.get("fix") for c in (components if isinstance(components, list) else []) + for s in c.get("subscriptions") or []): + fails.append("the flag-on facts carry no `fix` block: the pair does not exercise S0") + if on.get("ownir_version") != OWNIR_VERSION or off.get("ownir_version") != OWNIR_VERSION: + fails.append(f"ownir_version must be the core's {OWNIR_VERSION} in both documents: " + f"{on.get('ownir_version')!r} with the flag, " + f"{off.get('ownir_version')!r} without") + + # 2. not vacuous: every section the extractor CAN write is there, and not empty + sections = [k for k in _extractor_sections() if k not in _ADDITIVE_TOP_LEVEL] + readable = {"ownir_version", "module", "components", "functions"} <= set(sections) + if not readable: + fails.append(f"could not read the extractor's envelope from {_EXTRACTOR} (found " + f"{sections}); the envelope moved — update `_extractor_sections`") + for k in sections: + v = off.get(k) + if k not in off or (isinstance(v, (list, dict)) and not v): + fails.append(f"the fixture does not exercise `{k}`: the extractor can write it, " + f"and the flag-off facts have it " + f"{'empty' if k in off else 'absent'}. " + f"Extend tests/fixtures/fix_candidates/AdditiveSections.cs so the " + f"additivity check covers it") + unknown = sorted(set(off) - set(sections)) + if unknown and readable: + fails.append(f"the flag-off facts carry top-level keys this check did not find in the " + f"extractor's envelope: {unknown} — update `_extractor_sections`") + + # 3. the invariant itself, over the whole document + problem = _additivity_problem(on, off) + if problem is not None: + fails.append(problem) + + # 4. and its consequence: the flag changes no verdict and no advisory + got_on, got_off = _findings(on), _findings(off) + if got_on != got_off: + said = [g if isinstance(g, str) else f"{len(g)} finding(s)" for g in (got_on, got_off)] + fails.append(f"the reference gives different findings with the flag ({said[0]}) " + f"and without it ({said[1]})") + elif isinstance(got_off, str): + fails.append(f"the reference refuses the fixture's facts: {got_off}") + else: + orphans = off.get("orphaned_awaitables") or [] + own053 = [f for f in got_off if f[0] == "OWN053"] + if not isinstance(orphans, list) or len(own053) != len(orphans): + fails.append(f"{len(orphans) if isinstance(orphans, list) else '?'} " + f"orphaned_awaitables entr(ies) but {len(own053)} OWN053 advisor(ies)") + + if fails: + for fmsg in fails: + print("FAIL:", fmsg, file=sys.stderr) + return 1 + print(f"fix-candidates additivity: flag-on minus the S0 fields equals flag-off over " + f"all {len(sections)} sections the extractor writes ({', '.join(sections)}); " + f"the reference gives the same {len(got_off)} finding(s) with and without the flag") + return 0 + + def main(on_path: str, off_path: str | None) -> int: on = _load(on_path) fails: list[str] = [] @@ -247,7 +379,8 @@ def only_fix(name: str) -> dict | None: # Additivity, positively: strip every additive field from the flag-ON facts and # the result must EQUAL the flag-off facts (same records, same order, same old # values) -- enabling the metadata changed nothing pre-existing. - check(_strip_additive(on) == off, "flag-on minus additive fields must equal flag-off") + problem = _additivity_problem(on, off) + check(problem is None, problem or "") if fails: for fmsg in fails: @@ -258,7 +391,9 @@ def only_fix(name: str) -> dict | None: if __name__ == "__main__": - if len(sys.argv) not in (2, 3): + if len(sys.argv) == 4 and sys.argv[1] == "--additive-sections": + raise SystemExit(additive_sections(sys.argv[2], sys.argv[3])) + if len(sys.argv) not in (2, 3) or sys.argv[1].startswith("--"): print(__doc__, file=sys.stderr) raise SystemExit(2) raise SystemExit(main(sys.argv[1], sys.argv[2] if len(sys.argv) == 3 else None)) diff --git a/tests/fixtures/fix_candidates/AdditiveSections.cs b/tests/fixtures/fix_candidates/AdditiveSections.cs new file mode 100644 index 00000000..91d99d10 --- /dev/null +++ b/tests/fixtures/fix_candidates/AdditiveSections.cs @@ -0,0 +1,93 @@ +using System; +using System.ComponentModel; +using System.IO; +using System.Threading.Tasks; + +// S0 additivity fixture — read by `tests/check_fix_candidates_facts.py --additive-sections` +// (the CI step "S0 fix-candidates — extractor metadata (Part A)"). +// +// `--fix-candidates` only ADDS: take the S0 fields out of a flag-on document and what is left +// must be the flag-off document. That claim is only as strong as the document it is checked +// on, and FixCandidatesSample.cs has subscriptions and nothing else — so a flag-on envelope +// that dropped a whole section (it dropped `orphaned_awaitables`) passed it. This one file +// makes EVERY top-level section the Roslyn extractor can write non-empty under +// `--flow-locals`, and the checker refuses the pair if a section the extractor can write is +// missing from it. A new section therefore means a new block here, or a red build. +// +// It lives under tests/fixtures/, not frontend/roslyn/samples/: that directory is scanned as +// a whole by jobs that pin its findings. It is self-contained on purpose (no reference +// directory): the two OWN053 families are reached through types declared in this file. +namespace Own.Samples.FixCandidates.Additive +{ + // components[] — a subscription that is never released (under --fix-candidates it also + // carries the S0 `fix` block, and its component the S0 shape fields). + public sealed class Subscriber + { + private readonly INotifyPropertyChanged _source; + + public Subscriber(INotifyPropertyChanged source) + { + _source = source; + _source.PropertyChanged += OnChanged; + } + + private void OnChanged(object? sender, PropertyChangedEventArgs e) { } + } + + // services[] — a DI registration graph. The extraction is syntactic, so the surface only + // has to parse. + public sealed class ScopedThing { } + + public sealed class Holder { public Holder(ScopedThing thing) { } } + + public interface IRegistrar + { + IRegistrar AddScoped(); + IRegistrar AddSingleton(); + } + + public static class Registration + { + public static void Configure(IRegistrar services) + { + services.AddScoped(); + services.AddSingleton(); + } + } + + // functions[] — a flow-sensitive disposable local. + public static class Flow + { + public static long Leak(string path) + { + var stream = new FileStream(path, FileMode.Open); + return stream.Length; + } + } + + // orphaned_awaitables[] — both frozen OWN053 families. + public sealed class Connection : IDisposable + { + private bool _open = true; + + public bool IsOpen => _open; + + public void Dispose() { _open = false; } + } + + public sealed class Store + { + public Task OpenConnectionAsync() => Task.FromResult(new Connection()); + + public Task CommitAsync() => Task.CompletedTask; + } + + public static class Orphans + { + public static void Run(Store store) + { + var connection = store.OpenConnectionAsync(); // A_owned_result: a disposable, never observed + var commit = store.CommitAsync(); // B_protocol_lifecycle: a lifecycle call, never observed + } + } +}