diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e78134..bebe6e5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -207,6 +207,10 @@ jobs: frontend/roslyn/samples/ReturnedPublisherSample.cs \ frontend/roslyn/samples/OwnIgnoreSample.cs \ frontend/roslyn/samples/DpRotationSample.cs \ + frontend/roslyn/samples/RequerySuggestedAllowlistSample.cs \ + frontend/roslyn/samples/SelfDetachingHandlerSample.cs \ + frontend/roslyn/samples/UsingFieldAcquisitionSample.cs \ + frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -772,6 +776,65 @@ jobs: || { echo "FAIL: a -=/+= pair on differently-typed params must stay flagged (not old/new halves)"; exit 1; } echo "$out" | grep -qE "\[OWN001\].*'TwoFieldsRotation'" \ || { echo "FAIL: a -=/+= pair across two class fields must stay flagged (not a rotation)"; exit 1; } + # issue #223 — curated allowlist: CommandManager.RequerySuggested is implemented + # over weak references (see docs/notes/field-notes-patterns.md entry 17), so an + # ordinary instance-bound handler that never `-=`s it must NOT raise OWN014. + if echo "$out" | grep -q "ImeSupportLike"; then + echo "FAIL: an allowlisted CommandManager.RequerySuggested subscription was wrongly reported"; exit 1 + fi + # control: an ORDINARY (non-allowlisted) static event, same never-detached + # instance-handler shape, must STILL raise OWN014 — the allowlist must not + # weaken the general static-source tier. + echo "$out" | grep -qE "RequerySuggestedAllowlistSample\.cs:[0-9]+: error: \[OWN014\].*'OrdinaryStaticSubscriber'" \ + || { echo "FAIL: expected OWN014 on the non-allowlisted static-event subscriber"; exit 1; } + # issue #224 — a handler that unsubscribes ITSELF inside its own body (a + # self-detaching one-shot handler) is bounded -> must be SILENT. + if echo "$out" | grep -q "DropDownButtonLike"; then + echo "FAIL: a self-detaching handler subscription was wrongly reported"; exit 1 + fi + # control 1: the SAME shape but the handler does NOT self-detach -> must STILL warn. + echo "$out" | grep -qE "SelfDetachingHandlerSample\.cs:[0-9]+: warning: \[OWN001\].*'NonDetachingSubscriber'" \ + || { echo "FAIL: expected OWN001 on the non-self-detaching subscriber"; exit 1; } + # control 2: the handler detaches a DIFFERENT event name -> must NOT be credited + # as releasing the subscribed event -> must STILL warn. + echo "$out" | grep -qE "SelfDetachingHandlerSample\.cs:[0-9]+: warning: \[OWN001\].*'WrongEventDetachSubscriber'" \ + || { echo "FAIL: expected OWN001 when the self-detach targets the wrong event name"; exit 1; } + # control 3 (Codex P2 on PR #231): the handler detaches the CORRECT event name + # but off an UNRELATED receiver, not its own `sender` parameter -> the actual + # subscribed source is never released -> must STILL warn. + echo "$out" | grep -qE "SelfDetachingHandlerSample\.cs:[0-9]+: warning: \[OWN001\].*'WrongReceiverDetachSubscriber'" \ + || { echo "FAIL: expected OWN001 when the self-detach targets the wrong receiver (not sender)"; exit 1; } + # issue #220 — `using (field = new T())`: the field IS the `using` acquisition + # target, disposed at scope exit -> must be SILENT. + if echo "$out" | grep -q "HashCheckerLike"; then + echo "FAIL: a field disposed via using (field = new T()) was wrongly reported"; exit 1 + fi + # control: the SAME field, constructed the same way, but OUTSIDE any `using` and + # never disposed -> must STILL warn (the recognition is using-scoped, not "any + # field assignment from new is a release"). + echo "$out" | grep -qE "UsingFieldAcquisitionSample\.cs:[0-9]+: error: \[OWN001\].*'LeakyAssignerLike'" \ + || { echo "FAIL: expected OWN001 on the field assigned outside any using block"; exit 1; } + # issue #222 — a template part captured as a LOCAL (plain variable, via + # Template.FindName) or an `is T x` PATTERN variable (via GetTemplateChild), not + # only a field, is self-owned -> both must be SILENT. + if echo "$out" | grep -qE "MetroWindowLike|OverloadViewerLike"; then + echo "FAIL: a template part captured as a local/pattern variable was wrongly reported"; exit 1 + fi + # control: a local variable that merely ALIASES an INJECTED field (not a + # GetTemplateChild/FindName fetch) must STILL warn — the exemption is scoped to + # an actual template-part fetch, not "any local-variable subscription is self-owned." + echo "$out" | grep -qE "TemplatePartLocalCaptureSample\.cs:[0-9]+: warning: \[OWN001\].*'InjectedLocalSubscriber'" \ + || { echo "FAIL: expected OWN001 on the injected-local (non-template-part) subscriber"; exit 1; } + # control (Codex P2 on PR #231): a template-part local in one method must NOT + # exempt an UNRELATED same-named local (aliasing an injected source) in a + # DIFFERENT method of the same class — locals are self-owned by SYMBOL, not + # name. The template-part method's own subscription must stay silent... + if echo "$out" | grep -q "OnTemplateClick"; then + echo "FAIL: the legitimate template-part-local subscription was wrongly reported"; exit 1 + fi + # ...while the same-named local in the OTHER method must still warn. + echo "$out" | grep -qE "TemplatePartLocalCaptureSample\.cs:[0-9]+: warning: \[OWN001\].*'SameNameDifferentScopeSubscriber'" \ + || { echo "FAIL: expected OWN001 on the same-named-but-unrelated local in a different method"; exit 1; } echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) at the C# location" - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2) run: | diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 505aae9..7cf7db2 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -597,14 +597,23 @@ when s.IsKind(SyntaxKind.SuppressNullableWarningExpression): // — a *running* timer is rooted by the dispatcher regardless of who owns the field. static bool IsSelfOwnedSource(ExpressionSyntax left, IEventSymbol ev, SemanticModel model, HashSet owned, - ISymbol? cls) + HashSet ownedLocals, ISymbol? cls) { if (left is not MemberAccessExpressionSyntax m) return !ev.IsStatic; // bare event => an instance event on `this` if (m.Expression is ThisExpressionSyntax) return true; var recv = model.GetSymbolInfo(m.Expression).Symbol; - if ((recv is IFieldSymbol or ILocalSymbol) && owned.Contains(recv.Name)) + // FIELD names are unique per class (two same-named fields wouldn't compile), so a + // name-based lookup is sound; a LOCAL's name is only unique within its own + // declaration space — an unrelated method can freely reuse the same identifier for a + // completely different (possibly injected) local. So locals are matched by SYMBOL + // (`ownedLocals`, keyed by SymbolEqualityComparer), never by name, else a same-named + // local aliasing an injected source in one method could wrongly borrow another + // method's template-part local's exemption (Codex P2). + if (recv is IFieldSymbol && owned.Contains(recv.Name)) + return true; + if (recv is ILocalSymbol && ownedLocals.Contains(recv)) return true; // A get-only PROPERTY over a member the class owns (`this.Child.Event += h`, Child a // `=> _owned` / `{ get; } = new()` property) is the SAME collectable self-cycle as the @@ -719,6 +728,24 @@ ev.Name is "ProcessExit" or "DomainUnload" or "UnhandledException" or "FirstChan && ev.ContainingType is { Name: "AppDomain" } ct && IsInNamespace(ct, "System"); +// P-004 (issue #223): a curated allowlist of BCL/WPF static events KNOWN to be +// implemented internally over weak references, so a subscriber is never pinned +// alive by the subscription — unsubscribing is optional, not merely undetected. +// `System.Windows.Input.CommandManager.RequerySuggested` is the motivating (and, +// deliberately, the ONLY) entry: mined from AvalonEdit's `ImeSupport.cs`, whose own +// comment explains the field exists to keep the HANDLER alive (weak refs can collect +// it), the opposite of the usual leak concern — see +// docs/notes/field-notes-patterns.md entry 17. This is NOT a general "static sources +// are fine" relaxation (every other static-source subscription still raises OWN014 +// via the `capture` lowering below) — extend this list only when another sibling +// event's weak-reference implementation is independently confirmed, never by +// widening the predicate's shape (e.g. matching on name alone, or on "any +// System.Windows.Input member"). +static bool IsWeakReferencedStaticEvent(IEventSymbol ev) => + ev.Name == "RequerySuggested" + && ev.ContainingType is { Name: "CommandManager" } ct + && IsInNamespace(ct, "System", "Windows", "Input"); + // Does this handler retain NO subscriber instance? A static method group has a null delegate // target; a lambda / anonymous method retains nothing only when it captures neither `this` // (explicit, or implicit via an instance member) nor an enclosing local/parameter. Keeps the @@ -761,6 +788,82 @@ static bool DeclaredWithin(ISymbol sym, SyntaxNode scope) return true; } +// Strip parens/cast/`as`/null-forgiving (`!`) wrappers down to the base expression — +// e.g. `((Popup)sender!)` -> `sender`. Shared by HandlerSelfDetaches to resolve a +// `-=`'s receiver to the underlying symbol regardless of how it is cast back. +static ExpressionSyntax UnwrapToBase(ExpressionSyntax e) +{ + while (true) + { + var next = e switch + { + ParenthesizedExpressionSyntax p => p.Expression, + CastExpressionSyntax c => c.Expression, + BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left, + PostfixUnaryExpressionSyntax u + when u.IsKind(SyntaxKind.SuppressNullableWarningExpression) => u.Operand, + _ => e, + }; + if (ReferenceEquals(next, e)) + return e; + e = next; + } +} + +// P-004 (issue #224): a subscribed handler that unsubscribes ITSELF, inside its own +// body, the first time it runs ("do this once, then stop listening") needs no +// external `-=` — by the time the event could fire again, the subscription is +// already gone. The ONLY expression inside the handler provably referring to the +// actual runtime event source is its own `sender` parameter (whatever field/local/ +// property the ORIGINAL `+=` reached the source through, `sender` at invocation time +// IS that same source) — so this requires the `-=`'s receiver, once a cast/`as`/`!`/ +// parens are stripped, to resolve to the handler's own first parameter. Matching only +// the event NAME and handler METHOD (without checking the receiver at all) would +// wrongly credit a self-detach that targets some OTHER, unrelated object exposing an +// event of the same name (Codex P2 on PR #231) — the original subscription's source +// would never actually be released. Scoped to a named METHOD-GROUP handler only: a +// lambda has no name to self-reference by, so it is left exactly as before (still +// gets the "no '-=' handle" note). Mined: AvalonEdit `Search/DropDownButton.cs` +// (`((Popup)sender).Closed -= DropDownContent_Closed;` inside the very handler +// subscribed as `DropDownContent.Closed += DropDownContent_Closed`). +static bool HandlerSelfDetaches(ExpressionSyntax right, IEventSymbol ev, + SemanticModel model, CSharpCompilation compilation) +{ + right = NormalizeHandler(right); + var info = model.GetSymbolInfo(right); + var handlerMethod = info.Symbol as IMethodSymbol + ?? info.CandidateSymbols.OfType().FirstOrDefault(); + if (handlerMethod is null) + return false; + foreach (var sref in handlerMethod.DeclaringSyntaxReferences) + { + if (sref.GetSyntax() is not MethodDeclarationSyntax { ParameterList.Parameters: { Count: > 0 } ps } mdecl) + continue; + SyntaxNode? body = (SyntaxNode?)mdecl.Body ?? mdecl.ExpressionBody; + if (body is null) + continue; + var bodyModel = compilation.GetSemanticModel(mdecl.SyntaxTree); + if (bodyModel.GetDeclaredSymbol(ps[0]) is not { } senderParam) + continue; + foreach (var inner in body.DescendantNodes().OfType()) + { + if (!inner.IsKind(SyntaxKind.SubtractAssignmentExpression) + || inner.Left is not MemberAccessExpressionSyntax innerLeft + || innerLeft.Name.Identifier.Text != ev.Name) + continue; + // the receiver must resolve to the handler's OWN sender parameter — the + // only provable reference to the actual firing source. + var recvSym = bodyModel.GetSymbolInfo(UnwrapToBase(innerLeft.Expression)).Symbol; + if (!SymbolEqualityComparer.Default.Equals(recvSym, senderParam)) + continue; + var innerHandlerSym = bodyModel.GetSymbolInfo(NormalizeHandler(inner.Right)).Symbol; + if (SymbolEqualityComparer.Default.Equals(innerHandlerSym, handlerMethod)) + return true; + } + } + return false; +} + // P-004 process-lived-subscriber exemption: the WPF application object (`App`) is a // process-lived singleton — exactly one instance, created at startup, alive until // the process exits. Subscribing it to a process-lived static event @@ -3822,6 +3925,27 @@ or ImplicitObjectCreationExpressionSyntax && model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol tf && IsTemplatePartFetch(a.Right)) selfOwned.Add(tf.Name); + // P-004 (issue #222): a template part is EQUALLY self-owned when it is captured + // as a plain LOCAL variable (`Button upButton = (Button)this.Template.FindName( + // "PART_UP", this);`) or an `is T x` PATTERN variable (`if (this.GetTemplateChild( + // ...) is T x)`), not only a field — same fetch, same template-owned lifetime, + // just not stored in a field. Tracked by SYMBOL (`selfOwnedLocals`), not name — a + // local's identifier is scoped to its own method, so two unrelated methods can + // reuse the same variable name for two entirely different locals (Codex P2). + // Mined: MahApps.Metro `Controls/MetroWindow.cs` (pattern-variable form), + // AvalonEdit `CodeCompletion/OverloadViewer.cs` (plain local-variable form). + var selfOwnedLocals = new HashSet(SymbolEqualityComparer.Default); + foreach (var declr in cls.DescendantNodes().OfType()) + if (declr.Initializer?.Value is { } declInit + && IsTemplatePartFetch(declInit) + && model.GetDeclaredSymbol(declr) is ILocalSymbol declLocal) + selfOwnedLocals.Add(declLocal); + foreach (var isPat in cls.DescendantNodes().OfType()) + if (IsTemplatePartFetch(isPat.Expression) + && isPat.Pattern is DeclarationPatternSyntax + { Designation: SingleVariableDesignationSyntax svd } + && model.GetDeclaredSymbol(svd) is ILocalSymbol patLocal) + selfOwnedLocals.Add(patLocal); // * WPF MVVM view-model — `_vm = DataContext as VM`: when THIS view's own // XAML constructs its DataContext (recorded in viewsOwningDataContext from // the sibling `.xaml`), the view owns that VM, so the view<->VM cycle is @@ -3865,11 +3989,17 @@ or ImplicitObjectCreationExpressionSyntax // intent, not a leak (mined: Npgsql PoolManager's `AppDomain.CurrentDomain. // ProcessExit += (_,_) => ClearAll()` shutdown hook). A handler that captures // instance state still pins it to the process, so it stays OWN014 (Codex). - if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned, clsSymbol) + if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned, selfOwnedLocals, clsSymbol) || IsStaticHandler(a.Right, model) || (IsProcessLifetimeAppDomainEvent(ev) && HandlerRetainsNoInstance(a.Right, model)))) continue; + // P-004 (issue #223): the curated weak-referenced-static-event allowlist — + // unconditional (unlike the AppDomain exemption above, this does NOT gate on + // HandlerRetainsNoInstance: the whole point of a weak-referenced source is + // that even a normally-retaining handler cannot be pinned by it). + if (IsWeakReferencedStaticEvent(ev)) + continue; // P-004 tiering: a local-variable source is method-bounded — it // cannot outlive `this`, so it is not a heap leak; drop it (the same // spirit as the self-owned drop above). "static"/"injected" ride @@ -3920,6 +4050,10 @@ or ImplicitObjectCreationExpressionSyntax continue; var released = unsub.Contains($"{a.Left}|{NormalizeHandler(a.Right)}") || (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv)) + // P-004 (issue #224): the handler unsubscribes ITSELF inside its own body + // (a self-detaching one-shot handler) — bounded, so no external `-=` is + // needed. Timers already have their own Stop()-based release check above. + || (!isTimer && HandlerSelfDetaches(a.Right, ev, model, compilation)) // #218: the `+=` on the NEW half of a property-changed old->new rotation whose // `-=` is on the OLD half (same event member, same handler, same method) — a // single paired lifecycle, not an unpaired leak. @@ -4061,6 +4195,22 @@ or ImplicitObjectCreationExpressionSyntax disposed.Add(model.GetSymbolInfo(cae.Expression).Symbol is ILocalSymbol lc && aliasToField.TryGetValue(lc, out var fc) ? fc : cdf); + // 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` + // evaluates to at scope exit, regardless of whether `expr` is a bare `new T()`, an + // already-tracked LOCAL (the `using (existingLocal)` shape the flow-locals engine + // lowers elsewhere in this file), or, as here, a field assignment — the field is + // disposed exactly once, deterministically, when the block exits. This-instance + // only (`ThisFieldName`), matching every other release recognition above. Mined: + // ShareX `HashChecker.cs`/`TaskEx.cs` (`using (cts = new CancellationTokenSource())`), + // `IndexerJson.cs` (`using (jsonWriter = new JsonTextWriter(sw))`). + foreach (var us in cls.DescendantNodes().OfType()) + if (us.Expression is AssignmentExpressionSyntax { } uae + && uae.IsKind(SyntaxKind.SimpleAssignmentExpression) + && ThisFieldName(uae.Left) is { } uf) + disposed.Add(uf); + // P-004 EventSource counter exemption: inside an EventSource, a DiagnosticCounter field // constructed with `this` is registered to (and lifetime-owned by) the source — a // process-lived diagnostic the source never field-disposes (see IsEventSourceOwnedCounter). diff --git a/frontend/roslyn/samples/RequerySuggestedAllowlistSample.cs b/frontend/roslyn/samples/RequerySuggestedAllowlistSample.cs new file mode 100644 index 0000000..51825da --- /dev/null +++ b/frontend/roslyn/samples/RequerySuggestedAllowlistSample.cs @@ -0,0 +1,67 @@ +// P-004 (issue #223): a curated allowlist of ONE BCL/WPF static event — +// System.Windows.Input.CommandManager.RequerySuggested — known to be implemented +// internally over weak references, so a subscriber is never pinned alive by it. +// Mined from AvalonEdit's Editing/ImeSupport.cs (docs/notes/field-notes-patterns.md +// entry 17). No real WPF reference assembly is available on the Linux CI runner, so +// this file declares a self-contained stand-in for System.Windows.Input.CommandManager +// — the same technique WinFormsModelessSample.cs already uses for +// System.Windows.Forms. The extractor resolves the event against THIS declaration +// (all sample files are compiled together), so the allowlist predicate — which keys +// on the event's name + containing type + namespace, not on assembly identity — +// exercises exactly the same code path it would against the real WPF type. +using System; + +namespace System.Windows.Input +{ + public static class CommandManager + { + public static event EventHandler? RequerySuggested; + } +} + +namespace Own.Samples.WeakStaticEvent +{ + using System.Windows.Input; + + // Positive: allowlisted weak-referenced static event, ordinary instance-bound + // handler stored in a field, never `-=`'d — mirrors ImeSupport.cs exactly. Must + // be SILENT (no OWN014 region escape). + public sealed class ImeSupportLike + { + // "we need to keep the event handler instance alive because + // CommandManager.RequerySuggested uses weak references" — same comment as the + // real AvalonEdit source; the field exists to keep the HANDLER alive, not to + // prevent a leak of the subscriber. + private EventHandler? requerySuggestedHandler; + + public ImeSupportLike() + { + requerySuggestedHandler = OnRequerySuggested; + CommandManager.RequerySuggested += requerySuggestedHandler; + } + + private void OnRequerySuggested(object? sender, EventArgs e) { } + } + + // Negative control: an ORDINARY process-lived static event (NOT on the + // allowlist), same instance-handler-stored-in-a-field shape, never detached. The + // fix must NOT weaken the general static-source tier — this must STILL raise + // OWN014, proving the exemption is scoped to the one named CommandManager event. + public static class OtherStaticSource + { + public static event EventHandler? SomethingChanged; + } + + public sealed class OrdinaryStaticSubscriber + { + private EventHandler? handler; + + public OrdinaryStaticSubscriber() + { + handler = OnSomethingChanged; + OtherStaticSource.SomethingChanged += handler; + } + + private void OnSomethingChanged(object? sender, EventArgs e) { } + } +} diff --git a/frontend/roslyn/samples/SelfDetachingHandlerSample.cs b/frontend/roslyn/samples/SelfDetachingHandlerSample.cs new file mode 100644 index 0000000..43775ca --- /dev/null +++ b/frontend/roslyn/samples/SelfDetachingHandlerSample.cs @@ -0,0 +1,95 @@ +// P-004 (issue #224): a subscribed handler that unsubscribes ITSELF, inside its own +// body, the first time it fires — a common one-shot idiom ("do this once, then stop +// listening") that needs no external `-=`. Mined from AvalonEdit's +// Search/DropDownButton.cs (docs/notes/field-notes-patterns.md entry 18). +using System; + +namespace Own.Samples.SelfDetachingHandler +{ + public sealed class PopupLike + { + public event EventHandler? Closed; + public event EventHandler? Opened; + public void FireClosed() => Closed?.Invoke(this, EventArgs.Empty); + } + + // Positive: the handler removes itself from the SAME event, off the `sender` + // parameter cast back to the source type, the first time it runs — bounded by + // construction. Must be SILENT (no OWN001 warning). + public sealed class DropDownButtonLike + { + private readonly PopupLike content; + + public DropDownButtonLike(PopupLike content) + { + this.content = content; + content.Closed += DropDownContent_Closed; + } + + private void DropDownContent_Closed(object? sender, EventArgs e) + { + ((PopupLike)sender!).Closed -= DropDownContent_Closed; + } + } + + // Negative control 1: the SAME shape, but the handler does NOT self-detach (it + // just runs and returns) — must STILL warn (OWN001). Proves the new recognition + // requires an ACTUAL matching self-`-=` in the handler body, not just "this is a + // named handler on an injected source." + public sealed class NonDetachingSubscriber + { + private readonly PopupLike content; + + public NonDetachingSubscriber(PopupLike content) + { + this.content = content; + content.Closed += OnClosed; + } + + private void OnClosed(object? sender, EventArgs e) { /* never detaches */ } + } + + // Negative control 2: the handler DOES contain a `-=`, but against a DIFFERENT + // event name (`Opened`, not the subscribed `Closed`) — a wrong-event detach must + // NOT be credited as releasing the `Closed` subscription. Must STILL warn + // (OWN001), proving the match requires the inner `-=`'s member name to equal the + // SUBSCRIBED event's name. + public sealed class WrongEventDetachSubscriber + { + private readonly PopupLike content; + + public WrongEventDetachSubscriber(PopupLike content) + { + this.content = content; + content.Closed += OnClosed; + } + + private void OnClosed(object? sender, EventArgs e) + { + ((PopupLike)sender!).Opened -= OnClosed; + } + } + + // Negative control 3 (Codex P2 on PR #231): the handler detaches the CORRECT + // event name, but off an UNRELATED object (a different field) instead of the + // handler's own `sender` parameter — the actual object that raised the event. A + // same-named-event detach on the wrong receiver must NOT be credited as + // releasing the original subscription's source. Must STILL warn (OWN001). + public sealed class WrongReceiverDetachSubscriber + { + private readonly PopupLike content; + private readonly PopupLike other; + + public WrongReceiverDetachSubscriber(PopupLike content, PopupLike other) + { + this.content = content; + this.other = other; + content.Closed += OnClosed; + } + + private void OnClosed(object? sender, EventArgs e) + { + other.Closed -= OnClosed; // detaches a DIFFERENT PopupLike, not `sender` + } + } +} diff --git a/frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs b/frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs new file mode 100644 index 0000000..8d7ffe2 --- /dev/null +++ b/frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs @@ -0,0 +1,114 @@ +using System; + +// P-004 (issue #222): the self-owned-template-part exemption (SelfOwnedControlParts.cs) +// only credited a FIELD assignment (`_field = GetTemplateChild(...) as T`). A template +// part is EQUALLY self-owned when captured as a plain LOCAL variable or an `is T x` +// PATTERN variable — same fetch, same template-owned lifetime, just not stored in a +// field. Mined: MahApps.Metro Controls/MetroWindow.cs (pattern-variable form, via +// GetTemplateChild), AvalonEdit CodeCompletion/OverloadViewer.cs (plain local-variable +// form, via Template.FindName). Stand-in types mirror SelfOwnedControlParts.cs's +// technique (Tier A, no WPF reference set needed), namespaced separately so they +// cannot collide with the other samples compiled alongside this file. +namespace OwnSamples.TemplatePartLocals +{ + // Positive (pattern variable): `GetTemplateChild(...) is T x`, captured as an `is` + // pattern-match local, then subscribed. Must be SILENT (no OWN001 warning). + public sealed class MetroWindowLike : TemplatedControlStub + { + public override void OnApplyTemplate() + { + if (GetTemplateChild("PART_Content") is ContentControlStub metroContentControl) + { + metroContentControl.TransitionCompleted += OnTransitionCompleted; + } + } + + private void OnTransitionCompleted(object? sender, EventArgs e) { } + } + + // Positive (plain local via FindName): the template part is stored in an ordinary + // local variable (not a field, not a pattern variable), then subscribed. Must be + // SILENT (no OWN001 warning). + public sealed class OverloadViewerLike : TemplatedControlStub + { + public override void OnApplyTemplate() + { + ButtonStub upButton = (ButtonStub)Template.FindName("PART_UP", this); + upButton.Click += OnUpClick; + } + + private void OnUpClick(object? sender, EventArgs e) { } + } + + // Negative control: a local variable holding an INJECTED object (aliasing a + // constructor-supplied field) — NOT a GetTemplateChild/FindName fetch — subscribed + // the same way. Must STILL warn (OWN001), proving the exemption is scoped to an + // actual template-part fetch, not "any local-variable subscription is self-owned." + public sealed class InjectedLocalSubscriber + { + private readonly ButtonStub externalButton; + + public InjectedLocalSubscriber(ButtonStub externalButton) + { + this.externalButton = externalButton; + } + + public void Wire() + { + ButtonStub local = externalButton; // aliases an INJECTED field, not a template fetch + local.Click += OnClick; + } + + private void OnClick(object? sender, EventArgs e) { } + } + + // Precision regression control (Codex P2 on PR #231): a template-part LOCAL in one + // method must NOT exempt an UNRELATED same-named local (aliasing an injected source) + // in a DIFFERENT method of the same class — locals are self-owned by SYMBOL, never + // by name (a local's identifier is scoped to its own method; two methods can freely + // reuse the same name for two entirely different locals). + public sealed class SameNameDifferentScopeSubscriber : TemplatedControlStub + { + private readonly ButtonStub injectedButton; + + public SameNameDifferentScopeSubscriber(ButtonStub injectedButton) + { + this.injectedButton = injectedButton; + } + + // A template-part local named "sameName" -> legitimately self-owned. Must be SILENT. + public void WireTemplatePart() + { + ButtonStub sameName = (ButtonStub)Template.FindName("PART_X", this); + sameName.Click += OnTemplateClick; + } + + // An UNRELATED local, ALSO named "sameName", aliasing an INJECTED field — must + // STILL warn (OWN001). A name-based (rather than symbol-based) exemption would + // wrongly treat this as self-owned because "sameName" is in the set from + // WireTemplatePart above. + public void WireInjected() + { + ButtonStub sameName = injectedButton; + sameName.Click += OnInjectedClick; + } + + private void OnTemplateClick(object? sender, EventArgs e) { } + private void OnInjectedClick(object? sender, EventArgs e) { } + } + + public sealed class ButtonStub { public event EventHandler? Click; } + public sealed class ContentControlStub { public event EventHandler? TransitionCompleted; } + + public abstract class TemplatedControlStub + { + protected object? GetTemplateChild(string name) => null; + protected TemplateStub Template { get; } = new TemplateStub(); + public virtual void OnApplyTemplate() { } + } + + public sealed class TemplateStub + { + public object? FindName(string name, object scope) => null; + } +} diff --git a/frontend/roslyn/samples/UsingFieldAcquisitionSample.cs b/frontend/roslyn/samples/UsingFieldAcquisitionSample.cs new file mode 100644 index 0000000..56c7601 --- /dev/null +++ b/frontend/roslyn/samples/UsingFieldAcquisitionSample.cs @@ -0,0 +1,44 @@ +// P-004 (issue #220): `using (field = new T()) { ... }` disposes the FIELD at the end +// of the using block, exactly like `using (var local = new T())` disposes the local — +// the flow-locals engine already threads a release for `using (existingLocal)`; this +// closes the sibling gap where the acquisition expression assigns a FIELD. Mined from +// ShareX's HashChecker.cs / TaskEx.cs / IndexerJson.cs +// (docs/notes/field-notes-patterns.md entry 14). +using System.Threading; + +namespace Own.Samples.UsingFieldAcquisition +{ + // Positive: the field IS the `using` acquisition target — disposed at the end of + // the using block. Must be SILENT (no OWN001 disposable-field leak). + public sealed class HashCheckerLike + { + private CancellationTokenSource? cts; + + public void Check() + { + using (cts = new CancellationTokenSource()) + { + cts.Token.ThrowIfCancellationRequested(); + } + } + + public void Cancel() => cts?.Cancel(); + } + + // Negative control: the SAME field, constructed the SAME way (a `new + // CancellationTokenSource()` assignment), but OUTSIDE any `using` — never + // disposed anywhere. Must STILL warn (OWN001 disposable-field leak), proving the + // new recognition is scoped to the `using (field = ...)` acquisition shape, not + // "any field assignment from `new` is a release." + public sealed class LeakyAssignerLike + { + private CancellationTokenSource? cts; + + public void Start() + { + cts = new CancellationTokenSource(); + } + + public void Cancel() => cts?.Cancel(); + } +}