feat(extractor): recognise Behavior.AssociatedObject as a self-owned source (#227)#237
Conversation
…source (#227) A `Behavior`-derived class subscribing to (an element reached from) its own base-class `AssociatedObject` accessor was tiered "injected" and warned OWN001 (the MahApps TiltBehavior FP from the issue #201 sweep). A behavior is attached to and detached from exactly one element and cannot outlive being attached, so that element is co-lifetimed with the subscriber — the same collectable source<->this cycle the shipped self-owned-source exemption already encodes for a constructed field, just reached through a base-class accessor. The exemption is deliberately narrow, within the issue's fixed guardrails: - subscriber gate: the class must derive from `Behavior` (`IsBehaviorSubscriber`, direct-base simple-name match, mirroring `IsProcessLivedApplication`) — the attach/detach pairing is what guarantees co-lifetime; - source gate: the `+=` receiver must resolve, same-class and assignment-chain- local, to `this.AssociatedObject` (`ResolvesToAssociatedObject`) — directly, via a `var x = ...`/`is`-pattern local (only when never reassigned), or via a field every assignment to which resolves to `AssociatedObject` (a single injected write denies the proof). No interprocedural guessing. Unlike #228 a lambda handler is fine here: the source is co-lifetimed with the behavior, so a capture just closes the collectable cycle rather than pinning a process-lived source to a shorter-lived local. Pinned by AssociatedObjectSourceSample.cs: three silent positives (is-pattern field, direct `this.AssociatedObject.Event`, bare-identifier local) and the four controls the guardrails require — the unrelated injected source in the same OnAttached, a non-Behavior subscriber, a field also assigned an injected value, and a resolver-bound local reassigned before the `+=` — wired into the wpf-extractor CI job with assertions both ways. Verified locally with the real extractor (.NET 8): the sample yields exactly the four control warnings, all three positives silent; 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 #227 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2AWb9N2VUsby2XpdftBcu
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://cdn.github.com/in/57789?s=60&v=4){kind=small}
| if (_attached is Panel panel) | ||
| panel.Loaded += (s, e) => Handle(); // silent (#227): panel IS AssociatedObject | ||
|
|
||
| _bus.Changed += (s, e) => Handle(); // OWN001: unrelated injected source |
|
|
||
| public void Wire() | ||
| { | ||
| this.AssociatedObject.Loaded += OnLoaded; // OWN001: subscriber is not a Behavior |
| { | ||
| _el = this.AssociatedObject; | ||
| if (_el is Panel panel) | ||
| panel.Loaded += OnLoaded; // OWN001: _el is also injected below |
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa1af42e7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| MemberAccessExpressionSyntax m => m.Name.Identifier.Text == "AssociatedObject" | ||
| && m.Expression is ThisExpressionSyntax, | ||
| IdentifierNameSyntax id => id.Identifier.Text == "AssociatedObject", |
There was a problem hiding this comment.
In a Behavior subclass, a local/parameter or a hidden member named AssociatedObject can refer to an injected publisher rather than the inherited behavior accessor (for example void Wire(UiElement AssociatedObject) { AssociatedObject.Loaded += OnLoaded; }). Because this branch accepts the identifier purely by text before checking GetSymbolInfo, IsAssociatedObjectSource will suppress the new subscription warning even though the source is not co-lifetimed with the behavior.
Useful? React with 👍 / 👎.
#227, Codex P2) IsAssociatedObjectAccess matched the identifier `AssociatedObject` purely by text, so a local, a parameter, or a hidden member DECLARED in the Behavior subclass named `AssociatedObject` — holding an injected publisher, not the inherited accessor — would wrongly pass the self-owned-source gate (`void Wire(UiElement AssociatedObject) { AssociatedObject.Loaded += H; }`). That source is not co-lifetimed with the behavior, so the subscription must stay flagged. The name match now consults the symbol: the genuine base accessor is either UNRESOLVED (null — the Interactivity assembly is absent on the runner, the normal WPF case) or an INHERITED member (containing type is a base, not this class); a local/parameter binding, or a member declared on this class, is a shadow and denies the exemption. Conservative — an unresolvable symbol keeps today's behaviour. New flagged control ShadowParamBehavior (a parameter named AssociatedObject) + a CI assertion. Verified with the real extractor: all five controls warn, the three positives stay silent; full-sample-set output still byte-identical to main. Refs #227 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2AWb9N2VUsby2XpdftBcu
| var src = this.AssociatedObject; | ||
| src = _injected; // rebind -> declaration binding stale | ||
| if (src is { } s0) | ||
| s0.Loaded += OnLoaded; // OWN001: reassigned local |
| { | ||
| public void Wire(UiElement AssociatedObject) | ||
| { | ||
| AssociatedObject.Loaded += OnLoaded; // OWN001: shadowing parameter |
…ct field proof (#227) FieldAssignedOnlyFromAssociatedObject walked only the one class declaration holding the `+=`, so for a `partial` Behavior a disqualifying injected write to the field in a sibling partial FILE was invisible — the field could pass as an AssociatedObject alias while actually being injected elsewhere (same class of hole CodeRabbit flagged on the #229 sibling). It now scans every partial declaration of the field's containing type via the symbol's DeclaringSyntaxReferences (the merged compilation makes them all reachable), each with its own tree's semantic model. New flagged control PartialFieldBehavior (field assigned from AssociatedObject in one partial, from an injected value in the other) + a CI assertion. Verified with the real extractor: all six controls warn, the three positives stay silent; full-sample-set output byte-identical to main. Gates: run_tests 276/276, ruff, mypy, yaml all green. Refs #227 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U2AWb9N2VUsby2XpdftBcu
| { | ||
| _el = this.AssociatedObject; | ||
| if (_el is Panel panel) | ||
| panel.Loaded += OnLoaded; // OWN001: _el also injected in the sibling partial |
|
@coderabbitai review |
3 participants
Что и зачем
Расширяет self-owned-source исключение на sub-issue #227 (часть a родительского #221):
Behavior-класс, подписывающийся на (элемент, достижимый из) собственного базового аксессораAssociatedObject, больше не помечается OWN001. Поведение не может пережить прикрепление, поэтому источник co-lifetimed с подписчиком — тот же собираемый GC циклsource<->this, что уже кодирует исключение для сконструированного поля, только достигнутый через аксессор базового класса (FP из MahAppsTiltBehavior.cs, найденный oracle-sweep по #201).Реализовано строго в зафиксированных «Design guardrails» issue, без расширения: гейт подписчика — база
Behavior(IsBehaviorSubscriber, синтаксическое совпадение прямой базы по простому имени, какIsProcessLivedApplication); гейт источника — приёмник+=разрешается, в пределах класса и по цепочке присваиваний, вthis.AssociatedObject(ResolvesToAssociatedObject): напрямую, черезvar x = .../is-паттерн локал (только если не переприсвоен), либо через поле, каждое присваивание которому разрешается вAssociatedObject(единственная инъекция отменяет доказательство). Без межпроцедурных догадок. Lambda-обработчик здесь допустим (в отличие от #228): источник co-lifetimed с поведением.Тип изменения
Как проверено
python tests/run_tests.py(276/276 ownir, все зелёные)ruff check .иmypy(чисто)AssociatedObjectSourceSample.cs: 3 тихих позитива (is-паттерн поле, прямойthis.AssociatedObject.Event, bare-identifier локал), 4 контроля помечены (неродственный инъектированный источник в том жеOnAttached; не-Behaviorподписчик; поле, также получающее инъекцию; локал, переприсвоенный до+=)Связанные issue
Closes #227. Refs #221 (родитель, контекст). Sub-issue #229 (часть c) идёт отдельным PR.
Чеклист
AssociatedObjectSourceSample.cs+ wpf-extractor job, обе стороны) и задокументировано вdocs/notes/field-notes-patterns.md(entry 15, shape a)feat:)🤖 Generated with Claude Code
Generated by Claude Code