diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56a91db2..5474ee1b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -141,6 +141,7 @@ jobs: frontend/roslyn/samples/InjectedDcViewSample.xaml.cs \ frontend/roslyn/samples/ResolvedDisposableSample.cs \ frontend/roslyn/samples/FieldReleaseSample.cs \ + frontend/roslyn/samples/StaticClassEscapeSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -321,6 +322,14 @@ jobs: if echo "$out" | grep -q "CleanStaticEventViewModel"; then echo "FAIL: an unsubscribed (released) static-event subscription was wrongly reported"; exit 1 fi + # P-004 robust static-handler exemption (mined: ImageSharp MemoryAllocatorValidator): a + # static-METHOD handler on a static event stores a null-target delegate -> no instance is + # retained -> OWN014 must NOT fire, even when the method-group symbol surfaces as a member + # group (now resolved via CandidateSymbols). (StaticEventEscapeViewModel above proves an + # INSTANCE handler on the same static event still escapes, so this stays scoped.) + if echo "$out" | grep -q "StaticAllocationCounter"; then + echo "FAIL: a static-method handler on a static event was wrongly reported as a region escape"; exit 1 + fi # P-004 process-lived-subscriber exemption (mined: ScreenToGif App + # Translator): the WPF `App` singleton hooking the process-lived # AppDomain.UnhandledException promotes nothing, so the static-source region diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index a94e544e..51fb0c5a 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -185,10 +185,23 @@ BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left, // Target is null, so no instance is retained — the subscription cannot leak a // subscriber, however long-lived the source. Only method-group handlers // (identifier / member access) are judged; lambdas and delegate-typed values may -// capture state and are left as leak candidates. -static bool IsStaticHandler(ExpressionSyntax right, SemanticModel model) => - IsHandler(right) - && model.GetSymbolInfo(right).Symbol is IMethodSymbol { IsStatic: true }; +// capture state and are left as leak candidates. A method group's symbol can surface +// as a MEMBER GROUP (Symbol == null, CandidateSymbols populated) instead of the bound +// method, so fall back to the candidates — else a genuinely static-method handler is +// missed and a static-source subscription that retains NO instance is mis-reported as +// a region escape (mined: ImageSharp MemoryAllocatorValidator's static MemoryDiagnostics +// handlers). When falling back, require ALL candidates static so an overload set that +// mixes a static and an instance method is not wrongly exempted. +static bool IsStaticHandler(ExpressionSyntax right, SemanticModel model) +{ + if (!IsHandler(right)) + return false; + var info = model.GetSymbolInfo(right); + if (info.Symbol is { } s) + return s is IMethodSymbol { IsStatic: true }; + var cands = info.CandidateSymbols; + return cands.Length > 0 && cands.All(c => c is IMethodSymbol { IsStatic: true }); +} // P-004 process-lived-subscriber exemption: the WPF application object (`App`) is a // process-lived singleton — exactly one instance, created at startup, alive until diff --git a/frontend/roslyn/samples/StaticClassEscapeSample.cs b/frontend/roslyn/samples/StaticClassEscapeSample.cs new file mode 100644 index 00000000..e31cab4f --- /dev/null +++ b/frontend/roslyn/samples/StaticClassEscapeSample.cs @@ -0,0 +1,29 @@ +using System; + +namespace Own.Samples; + +// P-004 static-handler exemption, robustly (mined: ImageSharp MemoryAllocatorValidator). +// +// A static class whose static ctor hooks a process-lived STATIC event with a STATIC METHOD +// handler stores a delegate whose Target is null — no instance is retained, so OWN014 must NOT +// fire. The mined case slipped through because the method-group symbol can surface as a member +// group (Symbol == null), which IsStaticHandler now resolves via CandidateSymbols. Contrast: +// StaticEventEscapeViewModel — an INSTANCE handler on a static event — must still raise OWN014 +// (capturing lambdas in a static class likewise still escape: the closure is retained). + +public static class StaticDiagnosticsBus +{ + public static event EventHandler? Allocated; + + public static void Raise() => Allocated?.Invoke(null, EventArgs.Empty); +} + +public static class StaticAllocationCounter +{ + static StaticAllocationCounter() + { + StaticDiagnosticsBus.Allocated += OnAllocated; // static-method handler -> null target -> SILENT + } + + private static void OnAllocated(object? sender, EventArgs e) { } +}