From 0c179d5298e22c4ccbcfac08a0d693d8a680882d Mon Sep 17 00:00:00 2001 From: PhysShell <45852143+PhysShell@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:16:59 +0500 Subject: [PATCH 1/2] fix(roslyn): a field released in another partial file at teardown is not a leak (#410) OWN001 [disposable field] looked for `_f.Dispose()` only inside the one class declaration that declares the field, and *.Designer.cs files are never parsed. A WinForms field released in the designer Dispose(bool), or a field declared in Foo.ContextMenu.cs and disposed in Foo.cs, was reported as never disposed. - Other partial declarations: a Dispose/DisposeAsync/Close (or a receiver-releasing call) whose receiver binds to a field of THIS class is credited when it is reached at this object's teardown: a teardown method of that declaration or a method it provably calls, or a handler wired to our own Disposed/Closed (the existing TeardownContextMethods / InTeardownContext model), with every enclosing branch either the positive `if (disposing)` or a null check of the field, and no parameter-guarded early return. - A companion Foo.Designer.cs is parsed but not compiled (compiling it resolves the designer-declared child controls and turned OWN050 advisories into subscription findings on gitextensions). Its declarations of the same qualified class are read only as syntactic release evidence: the call directly in a teardown method, the name a field of the class and not shadowed, the same guard rule. It never becomes a source of facts. A release in an uncalled method, behind a flag, in the finalizer branch, in an unwired lambda or on another object's Disposed keeps OWN001. Closes #410 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 7 + frontend/roslyn/OwnSharp.Extractor/Program.cs | 180 ++++++++++++++ tests/test_extractor_partial_dispose.py | 222 ++++++++++++++++++ 3 files changed, 409 insertions(+) create mode 100644 tests/test_extractor_partial_dispose.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97afd043..7e62af7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -825,6 +825,13 @@ jobs: env: OWN_TIERB_REQUIRED: "1" run: python tests/test_extractor_sibling_locals.py + - name: A field released in another partial file at teardown is not a leak (Own.NET#410) + # OWN001 [disposable field] only looked inside the declaring class declaration, and + # *.Designer.cs files are never parsed: a designer Dispose(bool) release read as a leak. + # The release must be reached at teardown; a designer file stays evidence only. + env: + OWN_TIERB_REQUIRED: "1" + run: python tests/test_extractor_partial_dispose.py - name: Guarded release is not a call-site handoff (INF-S2, #380) # A callee that releases its parameter only under a guard is a PARTIAL release, so # its call must not be lowered as the caller's `release` (the fabricated `must`), diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index b9c02d7e..da4fa233 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -930,6 +930,61 @@ left is MemberAccessExpressionSyntax m // symbol-based closure proves a teardown calls it, a lambda only when it is // the handler wired to a lifecycle event. +// #410: every enclosing branch of a release found in ANOTHER partial declaration must run on +// each teardown — the positive `if (disposing)` of Dispose(bool) (never its else), or a null check of the +// released field itself (`if (_f != null)`). Any other condition (a flag, a parameter, a loop) leaves the +// release unproven, so the honest OWN001 stays. Stops at a lambda: its wiring is InTeardownContext's call. +static bool CrossPartialGuardsOk(SyntaxNode site, string field) +{ + for (var cur = site.Parent; cur is not null && cur is not MemberDeclarationSyntax; cur = cur.Parent) + { + if (cur is AnonymousFunctionExpressionSyntax) + break; + if (GuardCondition(cur) is not { } cond) + continue; + if (cur is not IfStatementSyntax || SiteOnNegativeBranch(cur, site)) + return false; + var c = cond is ParenthesizedExpressionSyntax pe ? pe.Expression : cond; + if (c is IdentifierNameSyntax { Identifier.Text: "disposing" } || IsNullCheckOfField(c, field)) + continue; + return false; + } + return true; +} + +static bool IsNullCheckOfField(ExpressionSyntax c, string field) +{ + static bool IsField(ExpressionSyntax e, string f) => ThisFieldName(e) == f; + return c switch + { + BinaryExpressionSyntax b when b.IsKind(SyntaxKind.NotEqualsExpression) => + (IsField(b.Left, field) && b.Right.IsKind(SyntaxKind.NullLiteralExpression)) + || (IsField(b.Right, field) && b.Left.IsKind(SyntaxKind.NullLiteralExpression)), + IsPatternExpressionSyntax ip => IsField(ip.Expression, field) + && ip.Pattern is UnaryPatternSyntax { Pattern: ConstantPatternSyntax { Expression: LiteralExpressionSyntax lit } } + && lit.IsKind(SyntaxKind.NullLiteralExpression), + _ => false, + }; +} + +// #410, designer files (syntax only, no model): the release must sit directly in the body of a +// teardown-named method (Dispose / DisposeAsync / OnClosed ...) of the designer half — not in a lambda or a +// local function, not in a helper (no call closure without symbols) — behind allowed guards only, and a +// `disposing` guard must name that method's own bool parameter. +static bool DesignerTeardownSite(SyntaxNode site, string field) +{ + var member = site.Ancestors().FirstOrDefault(a => a is MemberDeclarationSyntax or AnonymousFunctionExpressionSyntax + or LocalFunctionStatementSyntax); + if (member is not MethodDeclarationSyntax md || !IsTeardownMethodName(md.Identifier.Text)) + return false; + var usesDisposing = site.Ancestors().TakeWhile(a => a != md).OfType() + .Any(i => i.Condition is IdentifierNameSyntax { Identifier.Text: "disposing" }); + if (usesDisposing && !md.ParameterList.Parameters.Any(p => p.Identifier.Text == "disposing" + && p.Type?.ToString() == "bool")) + return false; + return CrossPartialGuardsOk(site, field); +} + // A method name the platform (or the IDisposable contract) itself invokes at the // end of the object's life. static bool IsTeardownMethodName(string name) => @@ -5877,6 +5932,54 @@ static bool IsPublicCtor(SyntaxTokenList modifiers) } Console.Error.WriteLine($"extractor: +{added} references from --ref-dir {dir} (recursive)"); } +// #410: `Ns.Outer.Foo` for a class declaration, from syntax alone (namespaces + enclosing types). +static string SyntaxQualifiedName(ClassDeclarationSyntax cls) +{ + var parts = new List { cls.Identifier.Text }; + foreach (var a in cls.Ancestors()) + switch (a) + { + case TypeDeclarationSyntax t: parts.Add(t.Identifier.Text); break; + case BaseNamespaceDeclarationSyntax ns: parts.Add(ns.Name.ToString()); break; + } + parts.Reverse(); + return string.Join(".", parts); +} + +// #410: a WinForms `Foo.Designer.cs` beside a parsed `Foo.cs` is never analysed (IsGenerated), yet it is +// where the designer `Dispose(bool)` releases the form's fields. It is parsed here but NOT compiled: adding it +// to the compilation resolves the designer-declared child controls and turns their OWN050 advisories into +// subscription findings (measured on gitextensions). Its class declarations are kept by qualified name and +// read ONLY as syntactic evidence that a field is released at teardown (see the field pass) — never as a +// source of facts. +var designerDecls = new Dictionary>(StringComparer.Ordinal); +{ + var seen = new HashSet(inputs, StringComparer.OrdinalIgnoreCase); + foreach (var path in inputs) + { + if (!path.EndsWith(".cs", StringComparison.OrdinalIgnoreCase) + || path.EndsWith(".Designer.cs", StringComparison.OrdinalIgnoreCase)) + continue; + var designer = path[..^3] + ".Designer.cs"; + if (!seen.Add(designer) || !File.Exists(designer)) + continue; + try + { + var dtree = CSharpSyntaxTree.ParseText(File.ReadAllText(designer), path: designer); + foreach (var dcls in dtree.GetRoot().DescendantNodes().OfType()) + { + var key = SyntaxQualifiedName(dcls); + if (!designerDecls.TryGetValue(key, out var list)) + designerDecls[key] = list = new List(); + list.Add(dcls); + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // An unreadable designer file just means "no extra release evidence". + } + } +} var compilation = CSharpCompilation.Create( "own", parsed.Select(p => p.tree), references, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); @@ -6382,6 +6485,83 @@ or ImplicitObjectCreationExpressionSyntax disposed.Add(model.GetSymbolInfo(cae.Expression).Symbol is ILocalSymbol lc && aliasToField.TryGetValue(lc, out var fc) ? fc : cdf); + // #410: the same two release shapes in the class's OTHER partial declarations (`Foo.cs` declares + // the field, `Foo.Lifecycle.cs` disposes it). The scan above only sees THIS declaration, which made + // OWN001 call such a field never disposed. Credited only when the receiver BINDS to a field of THIS + // class (symbol, not name: a same-named field of another class never counts) AND the release is + // reached at this object's teardown — stricter than the same-declaration scan above, which credits + // a release anywhere: a release in an uncalled method or behind an arbitrary flag stays OWN001. + if (clsSymbol is not null) + foreach (var sref in clsSymbol.DeclaringSyntaxReferences) + { + if (sref.GetSyntax() is not ClassDeclarationSyntax other || other == cls) + continue; + var om = model.Compilation.GetSemanticModel(other.SyntaxTree); + var otherTeardown = TeardownContextMethods(other, om, clsSymbol); + // a proven teardown method of that declaration (or one it provably calls), or a handler wired + // to our own lifecycle event; every enclosing branch must run on each teardown. + bool Reached(SyntaxNode site, string field) => + InTeardownContext(site, other, otherTeardown, om) + && CrossPartialGuardsOk(site, field) + && !IsParamGuardedByEarlyReturn(site, om); + bool OwnField(ExpressionSyntax e) => + om.GetSymbolInfo(e).Symbol is IFieldSymbol f + && SymbolEqualityComparer.Default.Equals(f.ContainingType.OriginalDefinition, clsSymbol.OriginalDefinition); + foreach (var inv in other.DescendantNodes().OfType()) + if (inv.Expression is MemberAccessExpressionSyntax pm + && ThisFieldName(pm.Expression) is { } pdf + && OwnField(pm.Expression) + && (pm.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close" + || CallReleasesReceiver(om.GetSymbolInfo(inv).Symbol as IMethodSymbol, om)) + && Reached(inv, pdf)) + disposed.Add(pdf); + foreach (var pcae in other.DescendantNodes().OfType()) + if (ThisFieldName(pcae.Expression) is { } pcdf + && OwnField(pcae.Expression) + && pcae.WhenNotNull is InvocationExpressionSyntax { Expression: MemberBindingExpressionSyntax pmb } pcinv + && (pmb.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close" + || CallReleasesReceiver(om.GetSymbolInfo(pcinv).Symbol as IMethodSymbol, om)) + && Reached(pcae, pcdf)) + disposed.Add(pcdf); + } + + // #410: a companion designer declaration of THIS class (same qualified name) that disposes `_f` / + // `this._f` where `_f` IS a field of this class (member lookup on the class symbol), the name is not + // shadowed by a local or parameter of the enclosing member, and the call sits directly in a + // teardown method behind teardown-safe guards only (DesignerTeardownSite). + if (clsSymbol is not null + && designerDecls.TryGetValue(SyntaxQualifiedName(cls), out var dparts)) + foreach (var dpart in dparts) + { + bool OwnFieldName(ExpressionSyntax e, out string name) + { + name = ThisFieldName(e) ?? ""; + if (name.Length == 0 || !clsSymbol.GetMembers(name).OfType().Any()) + return false; + var member = e.Ancestors().FirstOrDefault(a => a is BaseMethodDeclarationSyntax or AccessorDeclarationSyntax + or LocalFunctionStatementSyntax or AnonymousFunctionExpressionSyntax); + var n = name; + return e is MemberAccessExpressionSyntax // `this._f` can never be a local + || member is null + || !member.DescendantNodesAndSelf().Any(d => + d is VariableDeclaratorSyntax vd && vd.Identifier.Text == n + || d is ParameterSyntax ps && ps.Identifier.Text == n + || d is SingleVariableDesignationSyntax sd && sd.Identifier.Text == n); + } + foreach (var inv in dpart.DescendantNodes().OfType()) + if (inv.Expression is MemberAccessExpressionSyntax dm + && dm.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close" + && OwnFieldName(dm.Expression, out var ddf) + && DesignerTeardownSite(inv, ddf)) + disposed.Add(ddf); + foreach (var dcae in dpart.DescendantNodes().OfType()) + if (dcae.WhenNotNull is InvocationExpressionSyntax { Expression: MemberBindingExpressionSyntax dmb } + && dmb.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close" + && OwnFieldName(dcae.Expression, out var dcdf) + && DesignerTeardownSite(dcae, dcdf)) + disposed.Add(dcdf); + } + // P-004 (issue #220): `using (field = new T()) { ... }` — the STATEMENT form of // `using` whose parenthesized expression IS the acquisition, assigned directly to // a FIELD rather than a local. `using (expr) { }` disposes whatever `expr` diff --git a/tests/test_extractor_partial_dispose.py b/tests/test_extractor_partial_dispose.py new file mode 100644 index 00000000..cf93cade --- /dev/null +++ b/tests/test_extractor_partial_dispose.py @@ -0,0 +1,222 @@ +#!/usr/bin/env python3 +"""Own.NET#410 - a field released in ANOTHER partial file of its class, at teardown, is no leak. + +THE DEFECT + +OWN001 [disposable field] looked for `_f.Dispose()` only inside the ONE class declaration that +declares the field, and `*.Designer.cs` files are never parsed (IsGenerated). A WinForms field +disposed in the designer `Dispose(bool)` (gitextensions RepoObjectsTree, FormAddToGitIgnore), or +a field declared in `Foo.ContextMenu.cs` and disposed in `Foo.cs` (FileStatusList, rabbitmq +Connection, garnet ClusterManager, honua FileBackedLicenseService), was reported never disposed. + +WHAT THIS TEST PINS (each a separate PASS/KILL criterion of the fix) + + S silent: the release in another partial file runs at this object's teardown - Dispose(), + a helper Dispose() provably calls, a lambda wired to our own `Disposed`, a null check of + the field, and the designer `if (disposing) { _f.Dispose(); }`. + W still OWN001: the release is not proven to run at teardown - an uncalled method, a flag + guard, a lambda on ANOTHER object's `Disposed`, an unwired lambda, the finalizer branch + (`!disposing`, else), a designer helper / lambda, a release-and-recreate on a recovery + path (garnet resetHandler) - or it is not THIS field (another class, another instance, + another field, a shadowing local in a partial or designer file, a designer file of another + namespace). + D a designer file is evidence only: an undisposed field it declares produces no record. + +Both with and without --flow-locals. REQUIRED VS SKIPPED: as tests/test_extractor_columns.py - +required when OWN_TIERB_REQUIRED=1, a clean, explained skip when dotnet is absent. + +Run: OWN_TIERB_REQUIRED=1 python3 tests/test_extractor_partial_dispose.py +""" + +from __future__ import annotations + +import json +import os +import re +import shutil +import subprocess +import sys +import tempfile + +_REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +_EXT = os.path.join(_REPO, "frontend", "roslyn", "OwnSharp.Extractor") + +_MAIN = """using System.ComponentModel; +using System.IO; + +namespace Repro; + +public partial class {cls} : Component +{{ + private readonly MemoryStream _f = new(); + private bool _enabled; + public void Touch() {{ _f.WriteByte(1); _enabled = !_enabled; }} +}} +""" +_PART = "namespace {ns};\n\npartial class {cls}\n{{\n{body}\n}}\n" + +_DISPOSING = (" protected override void Dispose(bool disposing)\n {{\n {stmt}\n" + " base.Dispose(disposing);\n }}") + + +def _disposing(stmt: str) -> str: + return _DISPOSING.format(stmt=stmt) + + +# case -> (part file suffix, body, expect OWN001 on _f) +_CASES: dict[str, tuple[str, str, bool]] = { + "P1Dispose": (".Part.cs", " public void Dispose() => _f.Dispose();", False), + "P2Uncalled": (".Part.cs", " private void UnusedCleanup() => _f.Dispose();", True), + "P3Flag": (".Part.cs", " public void Dispose() { if (_enabled) _f.Dispose(); }", True), + "P4Helper": (".Part.cs", + " public void Dispose() => Cleanup();\n" + " private void Cleanup() => _f.Dispose();", False), + "P5OwnDisposed": (".Part.cs", + " public {cls}() { Disposed += (s, e) => { _f.Dispose(); }; }", False), + "P6NullCheck": (".Part.cs", + " public void Dispose() { if (_f != null) _f.Dispose(); }", False), + "P7OtherDisposed": (".Part.cs", + " public {cls}(Component other)\n" + " { other.Disposed += (s, e) => _f.Dispose(); }", True), + "P8UnwiredLambda": (".Part.cs", + " public void Dispose()\n" + " { System.Action a = () => _f.Dispose(); }", True), + "P9RecoveryRecreate": (".Part.cs", + " public void Recover()\n {\n _f.Dispose();\n" + " _f2 = new System.IO.MemoryStream();\n }\n" + " private System.IO.MemoryStream? _f2;", True), + "D1Designer": (".Designer.cs", _disposing("if (disposing) { _f.Dispose(); }"), False), + "D2DesignerUnused": (".Designer.cs", " private void Unused() { _f.Dispose(); }", True), + "D3DesignerFinalizer": (".Designer.cs", _disposing("if (!disposing) { _f.Dispose(); }"), True), + "D4DesignerElse": (".Designer.cs", + _disposing("if (disposing) { } else { _f.Dispose(); }"), True), + "D5DesignerFlag": (".Designer.cs", _disposing("if (_enabled) _f.Dispose();"), True), + "D6DesignerLambda": (".Designer.cs", + _disposing("System.Action a = () => _f.Dispose();"), True), + "C1OtherInstance": (".Part.cs", + " public static void Kill({cls} other) => other._f.Dispose();", True), + "C6PartialShadowLocal": (".Part.cs", + " public void Dispose()\n" + " { var _f = new System.IO.MemoryStream(); _f.Dispose(); }", + True), + "C2ShadowLocal": (".Designer.cs", + _disposing("var _f = new System.IO.MemoryStream(); " + "if (disposing) _f.Dispose();"), True), +} + +_UNRELATED = """using System.IO; + +namespace Repro; + +public class C3Unrelated +{ + private readonly MemoryStream _f = new(); + public void Dispose() => _f.Dispose(); +} +""" + +# extra sources: file name -> text +_EXTRA = { + # C3: a same-named field of ANOTHER class, disposed at that class's own teardown + "C3OtherClass.cs": _MAIN.format(cls="C3OtherClass"), + "C3Unrelated.cs": _UNRELATED, + # C4: the designer releases ANOTHER field + "C4WrongField.cs": _MAIN.format(cls="C4WrongField"), + "C4WrongField.Designer.cs": _PART.format( + ns="Repro", cls="C4WrongField", + body=" private readonly System.IO.MemoryStream _other = new();\n" + + _disposing("if (disposing) _other.Dispose();")), + # C5: a designer file of ANOTHER namespace + "C5OtherNs.cs": _MAIN.format(cls="C5OtherNs"), + "C5OtherNs.Designer.cs": _PART.format( + ns="Elsewhere", cls="C5OtherNs", + body=_disposing("if (disposing) _f.Dispose();").replace("override ", "")), + # D: a designer-declared undisposed field must not become a finding + "D7DesignerOnly.cs": _MAIN.format(cls="D7DesignerOnly"), + "D7DesignerOnly.Designer.cs": _PART.format( + ns="Repro", cls="D7DesignerOnly", + body=" private readonly System.IO.MemoryStream _designerLeak = new();\n" + " public void Touch2() => _designerLeak.WriteByte(1);"), +} +_EXPECT_EXTRA = {"C3OtherClass": True, "C4WrongField": True, "C5OtherNs": True} + +_FIELD = re.compile( + r"IDisposable field '(\w+)' \(type '[^']*'\) is never disposed .{0,3}its owner '(\w+)'") + + +def _write(src: str) -> None: + for cls, (suffix, body, _) in _CASES.items(): + with open(os.path.join(src, cls + ".cs"), "w", encoding="utf-8", newline="\n") as f: + f.write(_MAIN.format(cls=cls)) + with open(os.path.join(src, cls + suffix), "w", encoding="utf-8", newline="\n") as f: + f.write(_PART.format(ns="Repro", cls=cls, body=body.replace("{cls}", cls))) + for name, text in _EXTRA.items(): + with open(os.path.join(src, name), "w", encoding="utf-8", newline="\n") as f: + f.write(text) + + +def _run(src: str, work: str, flow: bool, fails: list[str]) -> None: + tag = "--flow-locals" if flow else "plain" + facts = os.path.join(work, f"facts-{tag}.json") + args = ["dotnet", "run", "--project", _EXT, "--", src, "-o", facts] + if flow: + args.append("--flow-locals") + ext = subprocess.run(args, cwd=_REPO, capture_output=True, text=True, encoding="utf-8", + errors="replace") + if ext.returncode != 0 or not os.path.exists(facts): + fails.append(f"[{tag}] extractor rc={ext.returncode}") + return + with open(facts, encoding="utf-8") as f: + doc = json.load(f) + for comp in doc.get("components", []): + from_designer = str(comp.get("file", "")).endswith(".Designer.cs") + for rec in comp.get("subscriptions", []): + if rec.get("event") == "_designerLeak" or from_designer: + fails.append(f"[{tag}] D a designer file became a source of facts: " + f"{comp.get('name')}") + env = dict(os.environ, PYTHONPATH=_REPO) + core = subprocess.run([sys.executable, "-m", "ownlang", "ownir", facts, + "--severity", "warning"], + cwd=_REPO, capture_output=True, text=True, encoding="utf-8", + errors="replace", env=env) + warned = {owner for fld, owner in _FIELD.findall(core.stdout) if fld == "_f"} + expect = {cls: e for cls, (_, _, e) in _CASES.items()} | _EXPECT_EXTRA + for cls, e in expect.items(): + if (cls in warned) != e: + kind = "W lost OWN001 (unproven release credited)" if e else "S still OWN001" + fails.append(f"[{tag}] {cls}: {kind}") + + +def run() -> int: + if hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(errors="backslashreplace") + if not shutil.which("dotnet"): + if os.environ.get("OWN_TIERB_REQUIRED") == "1": + print("extractor partial dispose: FAIL - dotnet is REQUIRED here " + "(OWN_TIERB_REQUIRED=1) and is not on PATH") + return 1 + print("extractor partial dispose: SKIP - dotnet not on PATH") + return 0 + fails: list[str] = [] + work = tempfile.mkdtemp(prefix="own410-") + try: + src = os.path.join(work, "src") + os.makedirs(src) + _write(src) + for flow in (True, False): + _run(src, work, flow, fails) + finally: + shutil.rmtree(work, ignore_errors=True) + if fails: + print("extractor partial dispose: FAIL") + for f in fails: + print(" -", f) + return 1 + n = len(_CASES) + len(_EXPECT_EXTRA) + print(f"extractor partial dispose: OK ({n} cases + designer-evidence check, " + "plain and --flow-locals)") + return 0 + + +if __name__ == "__main__": + raise SystemExit(run()) From 08142f3494bbed52013e0626f429d85db1427d76 Mon Sep 17 00:00:00 2001 From: PhysShell <45852143+PhysShell@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:47:51 +0500 Subject: [PATCH 2/2] fix(p037): the companion Foo.Designer.cs is a semantic side input (#410) The #410 extractor change reads the `Foo.Designer.cs` beside a primary `Foo.cs` as release evidence. P-037's fitness pin refused the two new unclassified read sites, and rightly: classifying them alone would let the designer file change the facts without changing recorded evidence. - p037_evidence.py: a fifth explicit-file mechanism, `sibling-designer`, anchored to the extractor construct; support_manifest carries the companion designer of every primary Foo.cs that is not itself a primary input (when it is, the analysis manifest already pins it and materializing it twice would break population_intact). - test_p037_evidence.py: the two read sites classified under it, and controls on a synthetic history: the designer enters the closure with only Foo.cs as primary input, is not duplicated when it is primary, and a designer edit invalidates the earlier record and breaks the before/after comparison while leaving the primary manifest unchanged. The repository population's support closure is unchanged (exactly three): every designer file under frontend/ and audit/ sits beside its Foo.cs in the same root, so it is a primary input. Refs #410 Co-Authored-By: Claude Opus 5.5 --- scripts/p037_evidence.py | 24 +++++++++-- tests/test_p037_evidence.py | 81 +++++++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/scripts/p037_evidence.py b/scripts/p037_evidence.py index 1160d7c4..9d8069fa 100755 --- a/scripts/p037_evidence.py +++ b/scripts/p037_evidence.py @@ -169,6 +169,14 @@ class EvidenceRefused(RuntimeError): "closure": "support_manifest + external_ancestor_problems", "anchor": r"Path\.Combine\(dir, \"FodyWeavers\.xml\"\)", }, + { + # #410: the companion `Foo.Designer.cs` of a primary `Foo.cs` is read (never compiled) + # as evidence that a field is released at teardown. The extractor reads it only when it + # is not itself a primary input, so the closure carries it only in that case. + "mechanism": "sibling-designer", + "closure": "support_manifest", + "anchor": r"var designer = path\[\.\.\^3\] \+ \"\.Designer\.cs\";", + }, ) @@ -322,15 +330,23 @@ def support_manifest( ) -> list[dict[str, str]]: """Committed side inputs the extractor reads BECAUSE of the primary inputs. - Explicit-file mode only: the sibling ``.xaml`` of every ``*.xaml.cs``, and - every ``FodyWeavers.xml`` on a primary file's ancestor chain up to the repo - root. Project/solution expansion (``WeaverOwnedFiles`` via a ``.csproj``) is - not part of this closure and is not certified by it. + Explicit-file mode only: the sibling ``.xaml`` of every ``*.xaml.cs``, the + companion ``Foo.Designer.cs`` of every primary ``Foo.cs`` that is not itself a + primary input (#410: read as release evidence, never compiled), and every + ``FodyWeavers.xml`` on a primary file's ancestor chain up to the repo root. + Project/solution expansion (``WeaverOwnedFiles`` via a ``.csproj``) is not part + of this closure and is not certified by it. """ tree = _tree_blobs(commit, (), repo=repo) + primary = {entry["path"] for entry in analysis} out: dict[str, dict[str, str]] = {} for entry in analysis: path = entry["path"] + if not path.lower().endswith(".designer.cs"): + designer = path[:-3] + ".Designer.cs" + if designer in tree and designer not in primary: + out[designer] = {"path": designer, "blob": tree[designer], + "mechanism": "sibling-designer"} if path.endswith(".xaml.cs"): sibling = path[:-3] if sibling in tree: diff --git a/tests/test_p037_evidence.py b/tests/test_p037_evidence.py index f9f82a74..e84d6c48 100644 --- a/tests/test_p037_evidence.py +++ b/tests/test_p037_evidence.py @@ -13,7 +13,7 @@ so the real git-backed predicate runs over real commits. Two fitness pins ride along: the extractor's explicit-file filesystem read -sites must all be classified against the four semantic mechanisms the support +sites must all be classified against the five semantic mechanisms the support closure covers, and the repository population's support closure must be exactly the three files it is today. """ @@ -66,7 +66,7 @@ DECL = re.compile(r"^static\s+[\w<>\[\]?,. ]+?\s+(\w+)\s*\(") # Every filesystem read site in the extractor, classified. The explicit-file -# entries are the four semantic mechanisms the evidence closure covers; a new +# entries are the five semantic mechanisms the evidence closure covers; a new # site (or a moved one) fails by NAME until someone classifies it here, and a # classification into explicit-file analysis obliges a support-closure update # in scripts/p037_evidence.py. Counting File.ReadAllText per file would guard @@ -88,6 +88,8 @@ ("", "File.ReadAllText", "path"): ("explicit-file:explicit-cs-contents", 1), ("", "File.Exists", "xamlPath"): ("explicit-file:sibling-xaml", 1), ("", "File.ReadAllText", "xamlPath"): ("explicit-file:sibling-xaml", 1), + ("", "File.Exists", "designer"): ("explicit-file:sibling-designer", 1), + ("", "File.ReadAllText", "designer"): ("explicit-file:sibling-designer", 1), ("", "Directory.Exists", "dir"): ("reference-environment", 2), ("", "Directory.EnumerateFiles", "dir"): ("reference-environment", 2), } @@ -176,6 +178,22 @@ def tree_with(self, path: str, content: bytes) -> str: capture_output=True, env=env) return proc.stdout.decode("ascii").strip() + def tree_with_files(self, files: dict[str, bytes]) -> str: + """HEAD's tree with several paths added or replaced, through a private index.""" + self.index_count += 1 + index = self.tmp / f"index-{self.index_count}" + env = {**self.env, "GIT_INDEX_FILE": str(index)} + subprocess.run(["git", "-C", str(self.repo), "read-tree", self.head_tree], check=True, + capture_output=True, env=env) + for path, content in files.items(): + blob = self.git("hash-object", "-w", "--stdin", stdin=content) + subprocess.run(["git", "-C", str(self.repo), "update-index", "--add", + "--cacheinfo", f"100644,{blob},{path}"], + check=True, capture_output=True, env=env) + proc = subprocess.run(["git", "-C", str(self.repo), "write-tree"], check=True, + capture_output=True, env=env) + return proc.stdout.decode("ascii").strip() + def commit(self, tree: str, parents: list[str], message: str) -> str: args = ["commit-tree", tree, "-m", message] for parent in parents: @@ -598,6 +616,61 @@ def history_controls() -> None: hist.cleanup() +def designer_support_controls() -> None: + """#410: a companion `Foo.Designer.cs` read as release evidence is a semantic side input. + + With only `Foo.cs` as a primary input the extractor reads the designer file beside it, so + the support closure must carry it, and a designer edit must invalidate earlier evidence. + With the designer itself a primary input it is pinned by the analysis manifest instead and + must not be materialized twice.""" + hist = History() + try: + base = "corpus/p037-designer" + main_cs = f"{base}/Foo.cs" + designer = f"{base}/Foo.Designer.cs" + files_v1 = { + main_cs: b"namespace R;\npublic partial class Foo { }\n", + designer: b"namespace R;\npartial class Foo { /* v1 */ }\n", + } + files_v2 = dict(files_v1) + files_v2[designer] = b"namespace R;\npartial class Foo { /* v2 */ }\n" + c1 = hist.commit(hist.tree_with_files(files_v1), [], "designer v1 (root)") + c2 = hist.commit(hist.tree_with_files(files_v2), [c1], "designer v2") + only_main = (main_cs,) + + f1 = ev.population_fields(c1, only_main, repo=hist.repo) + support = {e["path"]: e["mechanism"] for e in f1["support_manifest"]} + check("control-15-sibling-designer-in-support-closure", + support.get(designer) == "sibling-designer", + f"support closure {support!r} does not carry the companion designer file") + whole = ev.population_fields(c1, (base,), repo=hist.repo) + primary = {e["path"] for e in whole["analysis_manifest"]} + check("control-15b-primary-designer-not-in-support", + designer in primary + and all(e["path"] != designer for e in whole["support_manifest"]), + "a designer file that is a primary input must be pinned once, by the analysis " + "manifest") + + before = complete(ev.evidence_fields(only_main, population_commit=c1, source_commit=c1, + repo=hist.repo)) + forged = copy.deepcopy(before) + forged["support_manifest"] = ev.population_fields( + c2, only_main, repo=hist.repo)["support_manifest"] + expect_problem("control-15c-designer-edit-invalidates-record", + ev.record_problems(forged, repo=hist.repo), + "recorded support_manifest does not match") + after = complete(ev.evidence_fields(only_main, population_commit=c2, source_commit=c2, + repo=hist.repo)) + drift = ev.comparison_problems(before, after, against=c2, repo=hist.repo) + expect_problem("control-15d-designer-edit-breaks-comparison", drift, + "semantic support manifests differ") + check("control-15e-designer-edit-moves-only-support", + before["analysis_manifest"] == after["analysis_manifest"], + "the designer edit should move only the support closure") + finally: + hist.cleanup() + + def main() -> int: problems = ev.closure_problems() check("runtime-closure-covered", not problems, "; ".join(problems)) @@ -606,10 +679,12 @@ def main() -> int: fitness_pins() record_level_controls() history_controls() + designer_support_controls() if failures: print(f"RESULT: {failures} P-037 evidence control(s) failed") return 1 - print("RESULT: all P-037 evidence controls passed (14 negative, 1 positive, fitness pins)") + print("RESULT: all P-037 evidence controls passed " + "(14 negative, 1 positive, sibling-designer closure, fitness pins)") return 0