fix(extractor): recognize DP old→new subscription rotation as paired (Closes #218)

.github/workflows/ci.yml

@@ -206,6 +206,7 @@ jobs:
             frontend/roslyn/samples/VoidSubscribeSample.cs \
             frontend/roslyn/samples/ReturnedPublisherSample.cs \
             frontend/roslyn/samples/OwnIgnoreSample.cs \
+            frontend/roslyn/samples/DpRotationSample.cs \
             -o "$RUNNER_TEMP/facts.json"
           cat "$RUNNER_TEMP/facts.json"
       - name: Check facts through the core
@@ -753,7 +754,25 @@ jobs:
             "$RUNNER_TEMP/own.sarif" >/dev/null \
             || { echo "FAIL: SARIF must carry SuppressedLeak once, with an inSource suppressions justification"; \
                  jq '.runs[0].results[]|{ruleId,component:.properties.component,suppressions}' "$RUNNER_TEMP/own.sarif"; 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) at the C# location"
+          # issue #218 — DP/property-changed old->new subscription ROTATION (unsub OLD, sub NEW, same
+          # handler = one paired lifecycle) must be SILENT. Two recognised forms: a DP callback reading
+          # e.OldValue/e.NewValue (CommandTriggerAction), and a plain virtual OnXChanged(old, new)
+          # override with two same-type params (AbstractMargin). Confirmed FP in 3 real repos, 6+ sites.
+          # InlineCastRotation covers the direct inline-cast receiver `((ICommand)e.NewValue!).Event +=`
+          # (OldValue/NewValue are object-typed, so the cast is mandatory) — Codex review catch.
+          if echo "$out" | grep -qE "'CommandTriggerAction'|'AbstractMargin'|'InlineCastRotation'"; then
+            echo "FAIL: a DP old->new subscription rotation was wrongly flagged as a leak (#218)"; exit 1
+          fi
+          # ...and it must NOT over-widen — three controls STAY flagged: (a) the += uses a DIFFERENT
+          # handler than the -= (a genuine unpaired subscription); (b) two UNRELATED, differently-typed
+          # params (not the old/new halves of one change); (c) `-=` on one class FIELD, `+=` on another.
+          echo "$out" | grep -qE "\[OWN001\].*'MismatchedHandlerRotation'" \
+            || { echo "FAIL: a rotation with a DIFFERENT += handler must stay flagged (no over-widen)"; exit 1; }
+          echo "$out" | grep -qE "\[OWN001\].*'UnrelatedPairRotation'" \
+            || { 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; }
+          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: |
           # Path-sensitive flow analysis of local IDisposables — bugs the flat D1

docs/notes/field-notes-patterns.md

@@ -382,6 +382,22 @@ call's value. **Rule of thumb: when a `+=`/`-=` pair straddles the old/new halve
 a property-changed callback (or an equivalent `On<X>Changed(old, new)` override),
 treat them as one paired lifecycle, not two independent, unpaired subscriptions.**
 
+**Shipped (issue #218).** The extractor now recognises the rotation and stamps the
+`+=` on the new half as `released` — so the core sees a balanced acquire/release and
+stays silent (no new OwnIR fact, no `OWNIR_VERSION` bump; it reuses the existing
+`released` field). A `+=` on `<newRecv>.<Event>` with handler `H` is paired when the
+same method holds a `-=` on `<oldRecv>.<Event>` with the same `H`, and `(oldRecv,
+newRecv)` are the **old/new halves of one change**: either bound from a
+`DependencyPropertyChangedEventArgs` `e.OldValue`/`e.NewValue` (directly, or via
+`e.OldValue is T old` / `(T)e.OldValue` / `var old = e.OldValue`), or two **same-type
+parameters** of the enclosing method with old positioned before new (the
+`OnXChanged(T old, T new)` override — matched by the param-pair shape, not the method
+name). It deliberately does **not** widen: a `+=` with a *different* handler than the
+`-=`, a pair on two class **fields**, or on **differently-typed** params are not a
+rotation and stay flagged. Pinned by `frontend/roslyn/samples/DpRotationSample.cs`
+(`CommandTriggerAction`/`AbstractMargin` silent; `MismatchedHandlerRotation`/
+`UnrelatedPairRotation`/`TwoFieldsRotation` flagged) in the `wpf-extractor` CI job.
+
 ## 12. WinForms `Controls`/`ToolStripItemCollection` membership as a disposal channel
 
 **Seen in:** ShareX `ShareX.HelpersLib/Forms/ImageViewer.cs` (`pbPreview`,

frontend/roslyn/OwnSharp.Extractor/Program.cs

@@ -486,6 +486,106 @@
     left is MemberAccessExpressionSyntax m
         && (m.Name.Identifier.Text == "Tick" || m.Name.Identifier.Text == "Elapsed");
 
+// Issue #218 — DP/property-changed old->new subscription ROTATION. A property-changed callback (a
+// `PropertyChangedCallback` reading `e.OldValue`/`e.NewValue`, or a virtual `OnXChanged(T old, T new)`
+// override) detaches the SAME handler from the OLD value and re-attaches it to the NEW one across a
+// value change. The extractor pairs `-=`/`+=` per source VARIABLE, so the `+=` on the new half looks
+// unpaired and false-flags OWN001 — yet at most one subscription is ever live. This treats the `+=`
+// on the new half as RELEASED when its matching `-=` sits on the OLD half of the SAME change: same
+// event member, same handler, same method. Precise by construction (see IsOldNewPair) — it never
+// pairs a `-=`/`+=` on unrelated sources (two fields, differently-typed params), which stay flagged.
+static bool IsRotationPairedRelease(AssignmentExpressionSyntax add, SemanticModel model)
+{
+    if (add.Left is not MemberAccessExpressionSyntax addLhs)
+        return false;                                        // only `receiver.EventMember += H`
+    var eventMember = addLhs.Name.Identifier.Text;           // e.g. CanExecuteChanged
+    var newRecv = addLhs.Expression;                         // the NEW half's receiver
+    var handler = NormalizeHandler(add.Right).ToString();
+    if (add.FirstAncestorOrSelf<BaseMethodDeclarationSyntax>() is not { } method)
+        return false;                                        // a rotation lives in one callback
+    foreach (var node in method.DescendantNodes().OfType<AssignmentExpressionSyntax>())
+        if (node.IsKind(SyntaxKind.SubtractAssignmentExpression)
+            && node.Left is MemberAccessExpressionSyntax subLhs
+            && subLhs.Name.Identifier.Text == eventMember               // same event member
+            && NormalizeHandler(node.Right).ToString() == handler        // same handler
+            && IsOldNewPair(subLhs.Expression, newRecv, method, model))  // old/new halves of one change
+            return true;
+    return false;
+}
+
+// Are (oldRecv, newRecv) — the receivers of a `-=`/`+=` on the same event+handler — the OLD/NEW
+// halves of ONE property change? Two provable forms, nothing wider:
+//   (1) DependencyPropertyChangedEventArgs: `e.OldValue` / `e.NewValue` — used directly, or bound to
+//       a local (`e.OldValue is T old`, `(T)e.OldValue`, `var old = e.OldValue`).
+//   (2) two parameters of the enclosing method of the SAME type, OLD positioned before NEW — the
+//       `OnXChanged(T oldValue, T newValue)` override, matched by the param-pair shape, not by name.
+// Two class fields, differently-typed parameters, or unrelated locals are NOT a pair -> stay flagged.
+static bool IsOldNewPair(ExpressionSyntax oldRecv, ExpressionSyntax newRecv,
+                         BaseMethodDeclarationSyntax method, SemanticModel model)
+{
+    // (1) direct e.OldValue / e.NewValue receivers — including the inline CAST form
+    // `((ICommand)e.NewValue).CanExecuteChanged += H` (OldValue/NewValue are object-typed, so a
+    // cast is mandatory to reach the event), which is the common way to write this rotation.
+    if (IsDpValueAccess(StripCasts(oldRecv), "OldValue") && IsDpValueAccess(StripCasts(newRecv), "NewValue"))
+        return true;
+    var oldSym = model.GetSymbolInfo(oldRecv).Symbol;
+    var newSym = model.GetSymbolInfo(newRecv).Symbol;
+    if (oldSym is null || newSym is null
+        || SymbolEqualityComparer.Default.Equals(oldSym, newSym))
+        return false;   // unresolved, or the SAME source (that is the existing per-variable pairing)
+    // (1') locals bound from e.OldValue / e.NewValue.
+    if (BoundFromDpValue(oldSym, "OldValue", method, model)
+        && BoundFromDpValue(newSym, "NewValue", method, model))
+        return true;
+    // (2) two SAME-TYPE parameters, old before new.
+    return oldSym is IParameterSymbol op && newSym is IParameterSymbol np
+        && SymbolEqualityComparer.Default.Equals(op.Type, np.Type)
+        && op.Ordinal < np.Ordinal;
+}
+
+// `<recv>.OldValue` / `<recv>.NewValue` member access — the DependencyPropertyChangedEventArgs
+// halves. Matched by member name (those two members ARE the canonical DP-changed shape); the
+// extractor is syntactic by design, so this stays robust when the WPF ref is absent.
+static bool IsDpValueAccess(ExpressionSyntax expr, string member) =>
+    expr is MemberAccessExpressionSyntax ma && ma.Name.Identifier.Text == member;
+
+// Was the local/pattern variable `sym` bound from `e.<member>` (OldValue/NewValue) inside `method`?
+// Recognises a declaration pattern (`e.OldValue is T sym`) and a declarator initializer
+// (`var sym = e.OldValue`, `T sym = (T)e.OldValue`) — the ways a callback names its old/new half.
+static bool BoundFromDpValue(ISymbol sym, string member,
+                             BaseMethodDeclarationSyntax method, SemanticModel model)
+{
+    foreach (var node in method.DescendantNodes())
+    {
+        if (node is SingleVariableDesignationSyntax svd
+            && svd.Parent is DeclarationPatternSyntax { Parent: IsPatternExpressionSyntax ip }
+            && IsDpValueAccess(StripCasts(ip.Expression), member)
+            && SymbolEqualityComparer.Default.Equals(model.GetDeclaredSymbol(svd), sym))
+            return true;
+        if (node is VariableDeclaratorSyntax { Initializer: { } init } vd
+            && IsDpValueAccess(StripCasts(init.Value), member)
+            && SymbolEqualityComparer.Default.Equals(model.GetDeclaredSymbol(vd), sym))
+            return true;
+    }
+    return false;
+}
+
+// Peel enclosing casts / parentheses / the null-forgiving `!` so `(ICommand)e.OldValue`,
+// `(e.OldValue)` and `((ICommand)e.NewValue!)` all reach the underlying `e.OldValue`/`e.NewValue`.
+static ExpressionSyntax StripCasts(ExpressionSyntax e)
+{
+    while (true)
+        switch (e)
+        {
+            case CastExpressionSyntax c: e = c.Expression; continue;
+            case ParenthesizedExpressionSyntax p: e = p.Expression; continue;
+            case PostfixUnaryExpressionSyntax s
+                when s.IsKind(SyntaxKind.SuppressNullableWarningExpression):
+                e = s.Operand; continue;
+            default: return e;
+        }
+}
+
 // P-004 self-owned exemption: is the event SOURCE owned by (and so never longer-
 // lived than) the subscriber? True for a bare instance event on `this`, or a
 // receiver that resolves to a field/local the class OWNS. "Owns" is the `owned`
@@ -3582,7 +3682,7 @@
     .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
     .Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
     .ToList();
 var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);
 var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
 // P-004 WPF profile: widen the reference set with assemblies named by the
 // OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
@@ -3819,7 +3919,11 @@
                 if (!isTimer && source == "static" && HandlerRetainsNoInstance(a.Right, model))
                     continue;
                 var released = unsub.Contains($"{a.Left}|{NormalizeHandler(a.Right)}")
-                    || (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv));
+                    || (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv))
+                    // #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.
+                    || IsRotationPairedRelease(a, model);
                 // #146 — interprocedural publisher provenance: a `+=` on a PARAMETER
                 // publisher stays `injected` (locally honest — param→param syntax is
                 // identical for a DI singleton bus, a real leak, and for the

frontend/roslyn/samples/DpRotationSample.cs

@@ -0,0 +1,126 @@
+using System;
+
+namespace Own.Samples;
+
+// Issue #218 — DP/property-changed old->new subscription ROTATION must not be flagged as a leak:
+// a callback detaches the SAME handler from the OLD value and re-attaches it to the NEW one across
+// a value change, so at most one subscription is ever live (no leak). Two recognised forms plus
+// three negative controls that must STAY flagged (proving the fix does not over-widen). Stand-in
+// types (the extractor build has no WPF ref) mirror the real cases: MahApps CommandTriggerAction,
+// MaterialDesign SmartHint, AvalonEdit margins.
+
+public interface ICommand
+{
+    event EventHandler CanExecuteChanged;
+}
+
+// A different type that also exposes CanExecuteChanged — used by a negative control so the
+// discriminator is the parameter *types*, not the event/handler names.
+public interface IOtherCommand
+{
+    event EventHandler CanExecuteChanged;
+}
+
+// Stand-in for System.Windows.DependencyPropertyChangedEventArgs (matched syntactically by its
+// OldValue/NewValue members, the canonical DP-changed shape).
+public sealed class DependencyPropertyChangedEventArgs
+{
+    public object? OldValue { get; init; }
+    public object? NewValue { get; init; }
+}
+
+public class TextView
+{
+    public event EventHandler VisualLinesChanged = delegate { };
+}
+
+// FORM (1) — DependencyProperty callback, pattern-cast old/new (the MahApps CommandTriggerAction
+// shape). unsub OLD, sub NEW, same handler -> one paired lifecycle -> SILENT.
+public sealed class CommandTriggerAction
+{
+    private void OnCommandCanExecuteChanged(object? sender, EventArgs e) { }
+
+    private static void OnCommandChanged(CommandTriggerAction action, DependencyPropertyChangedEventArgs e)
+    {
+        if (e.OldValue is ICommand oldCommand)
+            oldCommand.CanExecuteChanged -= action.OnCommandCanExecuteChanged;   // unsub OLD
+        if (e.NewValue is ICommand newCommand)
+            newCommand.CanExecuteChanged += action.OnCommandCanExecuteChanged;   // sub NEW -> paired (SILENT)
+    }
+}
+
+// FORM (1, inline cast) — DP callback that subscribes DIRECTLY on the cast DP value, no intermediate
+// local: `((ICommand)e.NewValue!).CanExecuteChanged += H` paired with the OldValue `-=`. Since
+// OldValue/NewValue are object-typed the cast is mandatory, so this direct-cast form is a common way
+// to write the rotation -> SILENT. (Codex review: strip casts / `!` before the direct DP-receiver check.)
+public sealed class InlineCastRotation
+{
+    private void OnCanExecuteChanged(object? sender, EventArgs e) { }
+
+    private void OnCommandChanged(DependencyPropertyChangedEventArgs e)
+    {
+        ((ICommand)e.OldValue!).CanExecuteChanged -= OnCanExecuteChanged;   // unsub OLD (inline cast)
+        ((ICommand)e.NewValue!).CanExecuteChanged += OnCanExecuteChanged;   // sub NEW (inline cast) -> paired (SILENT)
+    }
+}
+
+// FORM (2) — plain virtual OnXChanged(old, new) override, two SAME-type parameters (the AvalonEdit
+// AbstractMargin shape — not even a DP callback). unsub param0, sub param1, same handler -> SILENT.
+public class AbstractMargin
+{
+    private void OnVisualLinesChanged(object? sender, EventArgs e) { }
+
+    protected virtual void OnTextViewChanged(TextView oldTextView, TextView newTextView)
+    {
+        if (oldTextView != null)
+            oldTextView.VisualLinesChanged -= OnVisualLinesChanged;   // unsub OLD (param 0)
+        if (newTextView != null)
+            newTextView.VisualLinesChanged += OnVisualLinesChanged;   // sub NEW (param 1) -> paired (SILENT)
+    }
+}
+
+// NEGATIVE CONTROL (a) — the rotation shape but a DIFFERENT handler on the += than the -=: the new
+// subscription is genuinely unpaired (the -= detached a different delegate) -> STAYS FLAGGED.
+public sealed class MismatchedHandlerRotation
+{
+    private void HandlerA(object? sender, EventArgs e) { }
+    private void HandlerB(object? sender, EventArgs e) { }
+
+    private static void OnChanged(MismatchedHandlerRotation self, DependencyPropertyChangedEventArgs e)
+    {
+        if (e.OldValue is ICommand oldCommand)
+            oldCommand.CanExecuteChanged -= self.HandlerA;   // unsub OLD with HandlerA
+        if (e.NewValue is ICommand newCommand)
+            newCommand.CanExecuteChanged += self.HandlerB;   // sub NEW with HandlerB (different!) -> LEAK
+    }
+}
+
+// NEGATIVE CONTROL (b) — a rotation-LIKE pair outside a property-changed context: two UNRELATED
+// parameters of DIFFERENT types (same event/handler names, but not the old/new halves of one value
+// change) -> the += is a real unpaired subscription -> STAYS FLAGGED.
+public sealed class UnrelatedPairRotation
+{
+    private void Handler(object? sender, EventArgs e) { }
+
+    private void Wire(ICommand primary, IOtherCommand secondary)
+    {
+        primary.CanExecuteChanged -= Handler;     // unsub on primary (ICommand)
+        secondary.CanExecuteChanged += Handler;   // sub on secondary (IOtherCommand — different type) -> LEAK
+    }
+}
+
+// NEGATIVE CONTROL (c) — `-=` on one class FIELD, `+=` on another (same event + handler). Fields are
+// not the old/new halves of a change; this is two independent subscriptions -> STAYS FLAGGED.
+public sealed class TwoFieldsRotation
+{
+    private ICommand _a = default!;
+    private ICommand _b = default!;
+
+    private void Handler(object? sender, EventArgs e) { }
+
+    private void Swap()
+    {
+        _a.CanExecuteChanged -= Handler;   // unsub on field _a
+        _b.CanExecuteChanged += Handler;   // sub on field _b (not an old/new pair) -> LEAK
+    }
+}