Skip to content

Commit f403bdb

Browse files
committed
fix(extractor): curated app-scoped source exemption for the Application subscriber (#228)
An App : Application subscribing to PaletteHelper().GetThemeManager() — an app-scoped, process-lived instance reached through a resolver call instead of a literal static member — was tiered "injected" and warned OWN001 (the MaterialDesign MahMaterialDragablzMashUp App.xaml.cs FP from the issue #201 sweep). The subscription promotes nothing: the subscriber IS the process-lived object and the source is bound to the app's own state. The narrowing inverts every property that made the rejected clsIsStatic broadening unsound (oracle-known-fps.md "Rejected approaches"): - subscriber gate unchanged — byte-for-byte the existing clsIsApp; - source loosens only to a CURATED resolver allowlist (exactly PaletteHelper.GetThemeManager for now; grown one confirmed sibling at a time, like the #223 weak-event list), resolved semantically through the receiver's local binding (var-initializer or is-pattern designation, casts/! peeled) — any unprovable step keeps today's honest warning; - handler must be a METHOD GROUP of the App class itself — a lambda is rejected outright, closing the captures-a-local hole that sank clsIsStatic. Pinned by AppScopedSourceSample.cs: two silent positives (is-pattern local + direct-invocation receiver) and three flagged controls (non-App subscriber, lambda handler, non-curated resolver), each edge of the gate, wired into the wpf-extractor CI job. The rejected-approaches note gains a "what DID ship next to it" section distinguishing this narrowing. Verified locally with the real extractor (.NET 8): the sample yields exactly the three control warnings; a full-sample-set diff of old vs new extractor output is byte-identical (zero regression). Gates: run_tests 276/276, ruff, mypy, yaml all green. Closes #228 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U
1 parent 3ca4bde commit f403bdb

4 files changed

Lines changed: 230 additions & 1 deletion

File tree

.github/workflows/ci.yml

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ jobs:
211211
frontend/roslyn/samples/SelfDetachingHandlerSample.cs \
212212
frontend/roslyn/samples/UsingFieldAcquisitionSample.cs \
213213
frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs \
214+
frontend/roslyn/samples/AppScopedSourceSample.cs \
214215
-o "$RUNNER_TEMP/facts.json"
215216
cat "$RUNNER_TEMP/facts.json"
216217
- name: Check facts through the core
@@ -835,7 +836,27 @@ jobs:
835836
# ...while the same-named local in the OTHER method must still warn.
836837
echo "$out" | grep -qE "TemplatePartLocalCaptureSample\.cs:[0-9]+: warning: \[OWN001\].*'SameNameDifferentScopeSubscriber'" \
837838
|| { echo "FAIL: expected OWN001 on the same-named-but-unrelated local in a different method"; exit 1; }
838-
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"
839+
# issue #228 — an Application-derived subscriber on a CURATED app-scoped
840+
# resolver result (PaletteHelper.GetThemeManager) with a method-group handler
841+
# of the App class itself must be SILENT: the source is process-lived (bound
842+
# to the app's own state), so nothing is promoted. Both receiver forms — an
843+
# `is`-pattern local (App) and the direct invocation (DirectApp).
844+
if echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+:.*('App'|'DirectApp')"; then
845+
echo "FAIL: a curated app-scoped subscription inside the Application was wrongly reported (#228)"; exit 1
846+
fi
847+
# ...and the exemption must NOT over-widen — three controls STAY flagged:
848+
# (1) the SAME curated shape from a NON-Application class (the subscriber
849+
# gate stays clsIsApp, byte-for-byte);
850+
echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'NotAnApp'" \
851+
|| { echo "FAIL: expected OWN001 on the non-Application subscriber (curated source alone must not exempt)"; exit 1; }
852+
# (2) App + curated source, but a LAMBDA handler capturing a local — the
853+
# exact hole that sank the rejected clsIsStatic broadening;
854+
echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'LambdaApp'" \
855+
|| { echo "FAIL: expected OWN001 on the lambda-handler subscription inside the App (capture hole)"; exit 1; }
856+
# (3) App + method-group handler, but a NON-curated resolver.
857+
echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'CuratedOnlyApp'" \
858+
|| { echo "FAIL: expected OWN001 on the non-curated resolver source inside the App"; exit 1; }
859+
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) + #228 curated app-scoped source in App (silent; controls flagged) at the C# location"
839860
- name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2)
840861
run: |
841862
# Path-sensitive flow analysis of local IDisposables — bugs the flat D1

docs/notes/oracle-known-fps.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,27 @@ which we have no reliable signal for. So those two findings stay in
308308
extractor rule. An in-code `ANTI-PATTERN` comment at the exemption site warns against
309309
re-adding `|| clsIsStatic`.
310310

311+
### What DID ship next to it (issue #228) — and why it is not the same thing
312+
313+
The #201 sweep's MaterialDesign `App.xaml.cs` FP (an `App : Application` subscribing
314+
to `PaletteHelper().GetThemeManager()`, an app-scoped instance that is process-lived
315+
without being a literal `static` member) is cleared by a narrowing that inverts every
316+
property that made `clsIsStatic` unsound:
317+
318+
- the **subscriber** gate is unchanged — byte-for-byte the existing `clsIsApp`
319+
("the subscriber IS the process-lived object", the property `clsIsStatic` lacked);
320+
- the **source** check loosens only to a **curated allowlist** of resolver methods
321+
whose result is verified app-scoped (`PaletteHelper.GetThemeManager`, verified at
322+
`PaletteHelper.cs:22-26`) — grown one confirmed sibling at a time, like the #223
323+
weak-event list, never inferred;
324+
- the **handler** must be a method group of the App class itself — a lambda is
325+
rejected outright, so the "captures a shorter-lived local and pins it" hole that
326+
sank `clsIsStatic` (and would NOT have been closed by it anyway) cannot pass.
327+
328+
Pinned by `AppScopedSourceSample.cs`: two silent positives (pattern-local and direct
329+
receiver) and three flagged controls (non-App subscriber, lambda handler, non-curated
330+
resolver), each edge of the gate.
331+
311332
## How the baseline stays honest
312333

313334
- **Matched by name, not line** — `(repo, file-basename, OWN code,

frontend/roslyn/OwnSharp.Extractor/Program.cs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,75 @@ static bool IsProcessLivedApplication(TypeDeclarationSyntax cls)
895895
&& cls.Modifiers.Any(m => m.IsKind(SyntaxKind.PartialKeyword));
896896
}
897897

898+
// P-004 / issue #228: the CURATED resolver allowlist whose call RESULT is app-scoped
899+
// (bound to Application-owned state, hence process-lived) without being a literal
900+
// `static` member. One entry per CONFIRMED real-world sibling — same policy as the
901+
// #223 weak-event allowlist, never an inference. Matched by (containing-type simple
902+
// name, method name), mirroring the [OwnIgnore] simple-name precedent: the declaring
903+
// package usually does not resolve on the Linux runner.
904+
// - MaterialDesign PaletteHelper.GetThemeManager(): returns the IThemeManager bound
905+
// to the app's own merged ResourceDictionary (PaletteHelper.cs:22-26, verified in
906+
// the issue #201 sweep — MahMaterialDragablzMashUp App.xaml.cs FP).
907+
static bool IsAppScopedResolver(IMethodSymbol sym) =>
908+
(sym.ContainingType?.Name, sym.Name) is ("PaletteHelper", "GetThemeManager");
909+
910+
// #228: does this `+=` receiver resolve (directly, or through a local bound by a
911+
// `var x = ...` initializer or an `is`-pattern designation) to the RESULT of a curated
912+
// app-scoped resolver call? Any unprovable step returns false — the subscription then
913+
// keeps today's honest "injected" warning, never the other way around.
914+
static bool IsCuratedAppScopedSource(ExpressionSyntax left, SemanticModel model)
915+
=> left is MemberAccessExpressionSyntax m
916+
&& ResolvesToAppScopedCall(m.Expression, model, depth: 0);
917+
918+
static bool ResolvesToAppScopedCall(ExpressionSyntax expr, SemanticModel model, int depth)
919+
{
920+
if (depth > 3)
921+
return false;
922+
expr = StripCasts(expr);
923+
if (expr is InvocationExpressionSyntax inv)
924+
{
925+
var info = model.GetSymbolInfo(inv);
926+
var sym = info.Symbol as IMethodSymbol
927+
?? info.CandidateSymbols.OfType<IMethodSymbol>().FirstOrDefault();
928+
return sym is not null && IsAppScopedResolver(sym);
929+
}
930+
if (model.GetSymbolInfo(expr).Symbol is not ILocalSymbol local)
931+
return false;
932+
foreach (var r in local.DeclaringSyntaxReferences)
933+
switch (r.GetSyntax())
934+
{
935+
// var tm = helper.GetThemeManager();
936+
case VariableDeclaratorSyntax { Initializer.Value: { } init }
937+
when ResolvesToAppScopedCall(init, model, depth + 1):
938+
return true;
939+
// helper.GetThemeManager() is { } tm / is ThemeManagerLike tm
940+
case SingleVariableDesignationSyntax des
941+
when des.Ancestors().OfType<IsPatternExpressionSyntax>().FirstOrDefault()
942+
is { Expression: { } scrutinee }
943+
&& ResolvesToAppScopedCall(scrutinee, model, depth + 1):
944+
return true;
945+
}
946+
return false;
947+
}
948+
949+
// #228: the handler must be a METHOD GROUP declared on the subscribing class itself —
950+
// its delegate target is then the App singleton (already process-lived, nothing to
951+
// promote). A lambda/anonymous method is rejected outright: it may capture an
952+
// enclosing LOCAL, and pinning that local to the app's lifetime is exactly the leak
953+
// the rejected `clsIsStatic` broadening would have swallowed (oracle-known-fps.md,
954+
// "Rejected approaches").
955+
static bool IsOwnMethodGroupHandler(ExpressionSyntax right, SemanticModel model,
956+
INamedTypeSymbol? cls)
957+
{
958+
if (cls is null || right is AnonymousFunctionExpressionSyntax)
959+
return false;
960+
var info = model.GetSymbolInfo(right);
961+
var sym = info.Symbol as IMethodSymbol
962+
?? info.CandidateSymbols.OfType<IMethodSymbol>().FirstOrDefault();
963+
return sym is not null
964+
&& SymbolEqualityComparer.Default.Equals(sym.ContainingType, cls);
965+
}
966+
898967
// P-004 WPF MVVM ownership: a field read from `this.DataContext`, optionally through
899968
// an `as`/cast (`DataContext as VM`, `(VM)DataContext`). Combined with a view whose
900969
// own XAML CONSTRUCTS its DataContext, such a field is the view's owned view-model.
@@ -4031,6 +4100,19 @@ or ImplicitObjectCreationExpressionSyntax
40314100
// write-up: docs/notes/oracle-known-fps.md → "Rejected approaches".
40324101
if (!isTimer && source == "static" && clsIsApp)
40334102
continue;
4103+
// P-004 / issue #228: the same `clsIsApp` subscriber, but the source is an
4104+
// APP-SCOPED RESOLVER RESULT (curated: PaletteHelper.GetThemeManager) rather
4105+
// than a literal static member — genuinely process-lived, just reached through
4106+
// a call, so SubscriptionSourceKind honestly says "injected". This loosens
4107+
// ONLY the source check; the subscriber gate stays byte-for-byte `clsIsApp`,
4108+
// and the handler must be a METHOD GROUP of the App class itself — a lambda
4109+
// is rejected because it may capture an enclosing local, the exact hole that
4110+
// sank the `clsIsStatic` broadening above (see that comment + the "Rejected
4111+
// approaches" write-up; this narrowing is documented alongside it).
4112+
if (!isTimer && source == "injected" && clsIsApp
4113+
&& IsOwnMethodGroupHandler(a.Right, model, clsSymbol)
4114+
&& IsCuratedAppScopedSource(a.Left, model))
4115+
continue;
40344116
// P-004 / issue #199: a NON-RETAINING handler on a static (process-lived)
40354117
// source promotes nothing, so OWN014's premise ("the subscriber is pinned
40364118
// for the source's life") does not hold -> silent. A static METHOD handler
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// issue #228 — an Application-derived subscriber whose event source is an APP-SCOPED
2+
// RESOLVER RESULT (curated: PaletteHelper.GetThemeManager), not a literal static member.
3+
// The source is process-lived (bound to the app's own state), so the subscription
4+
// promotes nothing — but SubscriptionSourceKind honestly tiers a call-result receiver
5+
// "injected", which used to warn. Real-world shape: MaterialDesignInXamlToolkit
6+
// MahMaterialDragablzMashUp App.xaml.cs:10,22 (found by the issue #201 oracle sweep).
7+
//
8+
// The exemption is deliberately NARROW (see the rejected `clsIsStatic` broadening in
9+
// docs/notes/oracle-known-fps.md): subscriber must be the Application class itself,
10+
// the resolver must be on the curated list, and the handler must be a METHOD GROUP of
11+
// that class — three negative controls below pin each edge.
12+
using System;
13+
14+
namespace OwnSamples.AppScoped
15+
{
16+
// Stand-in for System.Windows.Application: IsProcessLivedApplication matches the
17+
// base NAME syntactically (WPF assemblies do not resolve on the Linux runner).
18+
public class Application { }
19+
20+
public class ThemeManagerLike
21+
{
22+
public event EventHandler? ThemeChanged;
23+
public void Raise() => ThemeChanged?.Invoke(this, EventArgs.Empty);
24+
}
25+
26+
// Stand-in for MaterialDesignThemes.Wpf.PaletteHelper — the curated resolver,
27+
// matched by (containing-type, method) = (PaletteHelper, GetThemeManager).
28+
public class PaletteHelper
29+
{
30+
static readonly ThemeManagerLike Manager = new();
31+
public ThemeManagerLike? GetThemeManager() => Manager;
32+
}
33+
34+
// Same shape, NON-curated name -> never exempted.
35+
public class OtherHelper
36+
{
37+
static readonly ThemeManagerLike Manager = new();
38+
public ThemeManagerLike? GetOtherService() => Manager;
39+
}
40+
41+
// POSITIVE (silent): App + curated resolver via `is`-pattern local + method-group
42+
// handler of the App class — the exact MaterialDesign shape.
43+
public class App : Application
44+
{
45+
public void OnStartup()
46+
{
47+
var helper = new PaletteHelper();
48+
if (helper.GetThemeManager() is { } themeManager)
49+
themeManager.ThemeChanged += ThemeManager_ThemeChanged; // silent (#228)
50+
}
51+
52+
void ThemeManager_ThemeChanged(object? sender, EventArgs e) { }
53+
}
54+
55+
// POSITIVE (silent): the direct-invocation receiver form, no intermediate local.
56+
public class DirectApp : Application
57+
{
58+
public void OnStartup()
59+
{
60+
new PaletteHelper().GetThemeManager()!.ThemeChanged += OnTheme; // silent (#228)
61+
}
62+
63+
void OnTheme(object? sender, EventArgs e) { }
64+
}
65+
66+
// CONTROL 1 (flagged): the SAME curated shape from a class that is NOT the
67+
// Application — the subscriber gate must stay `clsIsApp`, byte-for-byte.
68+
public class NotAnApp
69+
{
70+
public void Wire()
71+
{
72+
var helper = new PaletteHelper();
73+
if (helper.GetThemeManager() is { } themeManager)
74+
themeManager.ThemeChanged += OnTheme; // OWN001 warning
75+
}
76+
77+
void OnTheme(object? sender, EventArgs e) { }
78+
}
79+
80+
// CONTROL 2 (flagged): App + curated source, but the handler is a LAMBDA capturing
81+
// an enclosing local — pinning that local to app lifetime is the exact hole that
82+
// sank the `clsIsStatic` broadening; a lambda must never pass the handler gate.
83+
public class LambdaApp : Application
84+
{
85+
public void OnStartup()
86+
{
87+
var counter = new int[1];
88+
if (new PaletteHelper().GetThemeManager() is { } themeManager)
89+
themeManager.ThemeChanged += (s, e) => counter[0]++; // OWN001 warning
90+
}
91+
}
92+
93+
// CONTROL 3 (flagged): App + method-group handler, but a NON-curated resolver —
94+
// membership is a curated allowlist, not "any call result inside App".
95+
public class CuratedOnlyApp : Application
96+
{
97+
public void OnStartup()
98+
{
99+
if (new OtherHelper().GetOtherService() is { } service)
100+
service.ThemeChanged += OnTheme; // OWN001 warning
101+
}
102+
103+
void OnTheme(object? sender, EventArgs e) { }
104+
}
105+
}

0 commit comments

Comments
 (0)