From 2fd252910057a2a137f25f80c0bfe4a8d18d8e63 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 16:43:33 +0000 Subject: [PATCH 1/4] feat(own): fold cleanup into an existing OnClosed override Quality improvement for the subscription/disposable-field shapes: when the owner class already has a 'protected override void OnClosed(...)' (which runs exactly when the Window's Closed event would), insert the detach/dispose statement at the top of that method body instead of stacking a fresh 'this.Closed += (s,e) => ...' lambda. Cleaner, more idiomatic patches. Conservative: only folds into a clean BLOCK-body OnClosed (its '{' ends a line); one-liner/expression bodies fall back to the lambda. Other owners (Unloaded / non-Window) keep the lambda. detail reported as '/fold'. Fixture: FoldWindow (Window with an existing OnClosed). +1 test. 21/21 + 7/7, -O. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa --- fix/README.md | 8 +++- fix/fixarm/own_fix.py | 44 ++++++++++++++++++- .../own001-sub-fold/after.findings.json | 3 ++ .../own001-sub-fold/before.findings.json | 12 +++++ .../before/Broker/FoldWindow.xaml.cs | 25 +++++++++++ fix/tests/test_own_fix.py | 15 +++++++ 6 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 fix/fixtures/own001-sub-fold/after.findings.json create mode 100644 fix/fixtures/own001-sub-fold/before.findings.json create mode 100644 fix/fixtures/own001-sub-fold/before/Broker/FoldWindow.xaml.cs diff --git a/fix/README.md b/fix/README.md index a24cf63..dc0454a 100644 --- a/fix/README.md +++ b/fix/README.md @@ -84,9 +84,13 @@ that escapes its block (return/out/ref/store) → `local-escapes`; a block-body unknown-delegate lambda → `lambda-shape-unsupported` / `unknown-event-delegate`; an unbraced guard → `unbraced-control-flow`; no safe teardown → `no-safe-teardown`. +When the owner already has a `protected override void OnClosed(...)`, the cleanup is +**folded into it** (a statement at the top of the body) instead of stacking another +`this.Closed += …` lambda — a cleaner, more idiomatic patch. + ## Next - Promote proven-mechanical rules into `tiers._T1_RULES` (auto-commit) from real diffs. -- OWN fixer: fold into an existing `OnClosed`/`Dispose` override when one is present; - widen lambda extraction to more event delegates. +- OWN fixer: fold into an existing `Dispose()`/`Unloaded` handler too; widen lambda + extraction to more event delegates (`EventHandler`, custom `*Changed`). - Windows-bound fix-spike: does `roslynator fix` load `Broker.sln` (docs/fix-arm.md §6). diff --git a/fix/fixarm/own_fix.py b/fix/fixarm/own_fix.py index aa31367..8b15a95 100644 --- a/fix/fixarm/own_fix.py +++ b/fix/fixarm/own_fix.py @@ -213,8 +213,45 @@ def _class_close(lines: list[str], cls_idx: int): # An edit is (start, end, repl_lines): `lines[start:end] = repl_lines` (insert when # start == end). plan_file applies them bottom-up so indices stay valid. +def _enclosing_class_idx(lines, idx): + """Line index of the nearest `class X` declaration at or above idx.""" + for i in range(idx, -1, -1): + if re.search(r"\bclass\s+\w+", lines[i]): + return i + return None + + +def _fold_after_open_brace(lines, decl_idx, end): + """For a method whose declaration is at decl_idx, return (brace_line, body_indent) + if it has a clean BLOCK body (its `{` ends a line) — so a statement can be folded + in right after it. Returns None for one-liner/expression bodies (don't fold).""" + for i in range(decl_idx, end): + sk = _code_skeleton(lines[i]).rstrip() + if sk.endswith("{"): + return i, re.match(r"\s*", lines[i]).group(0) + " " + if "{" in sk or sk.endswith(";") or "=>" in sk: # one-liner / expr body -> skip + return None + return None + + +def _fold_target(lines, cls_idx, ev): + """If the class already has a teardown method to fold cleanup into, return its + block anchor. Today: a `protected override void OnClosed(...)` for ev == Closed — + it runs exactly when the Window's Closed event would, so folding is equivalent and + cleaner than stacking another lambda.""" + if ev != "Closed": + return None + end = _class_close(lines, cls_idx) + end = end if end is not None else len(lines) + for i in range(cls_idx, end): + if re.search(r"\boverride\s+void\s+OnClosed\b", lines[i]): + return _fold_after_open_brace(lines, i, end) + return None + + def _plan_teardown(lines, f, idx, stmt, anchor_missing="site-not-found"): - """Subscription / disposable-field: hang `stmt` on the owner's teardown event.""" + """Subscription / disposable-field: run `stmt` on the owner's teardown. Folds into + an existing OnClosed override when present; otherwise adds a fresh teardown lambda.""" if idx is None: return None, anchor_missing if _in_unbraced_control_flow(lines, idx): @@ -223,6 +260,11 @@ def _plan_teardown(lines, f, idx, stmt, anchor_missing="site-not-found"): ev = _teardown_event(decl) if ev is None: return None, "no-safe-teardown" + cls_idx = _enclosing_class_idx(lines, idx) + fold = _fold_target(lines, cls_idx, ev) if cls_idx is not None else None + if fold is not None: + brace_idx, body_indent = fold + return [(brace_idx + 1, brace_idx + 1, [f"{body_indent}{stmt};\n"])], f"{ev}/fold" indent = re.match(r"\s*", lines[idx]).group(0) hook = f"{indent}this.{ev} += (s, e) => {stmt};\n" return [(idx + 1, idx + 1, [hook])], ev diff --git a/fix/fixtures/own001-sub-fold/after.findings.json b/fix/fixtures/own001-sub-fold/after.findings.json new file mode 100644 index 0000000..2ef5648 --- /dev/null +++ b/fix/fixtures/own001-sub-fold/after.findings.json @@ -0,0 +1,3 @@ +{ + "findings": [] +} diff --git a/fix/fixtures/own001-sub-fold/before.findings.json b/fix/fixtures/own001-sub-fold/before.findings.json new file mode 100644 index 0000000..5be6ea8 --- /dev/null +++ b/fix/fixtures/own001-sub-fold/before.findings.json @@ -0,0 +1,12 @@ +{ + "findings": [ + { + "tool": "own-check", + "path": "Broker/FoldWindow.xaml.cs", + "line": 14, + "rule": "OWN001", + "category_name": "subscription-leak", + "message": "event 'fGoods.PropertyChanged' is subscribed (handler 'new PropertyChangedEventHandler(GoodsPropertyChanged)') but never unsubscribed; its source is an injected dependency whose lifetime is unknown, so it may outlive and keep 'FoldWindow' alive (possible leak) [resource: subscription token]" + } + ] +} diff --git a/fix/fixtures/own001-sub-fold/before/Broker/FoldWindow.xaml.cs b/fix/fixtures/own001-sub-fold/before/Broker/FoldWindow.xaml.cs new file mode 100644 index 0000000..ec222f7 --- /dev/null +++ b/fix/fixtures/own001-sub-fold/before/Broker/FoldWindow.xaml.cs @@ -0,0 +1,25 @@ +using System; +using System.ComponentModel; +using System.Windows; + +namespace Sts.Broker +{ + public partial class FoldWindow : Window + { + private readonly Goods fGoods; + + public FoldWindow(Goods goods) + { + fGoods = goods; + InitializeComponent(); + fGoods.PropertyChanged += new PropertyChangedEventHandler(GoodsPropertyChanged); + } + + protected override void OnClosed(EventArgs e) + { + base.OnClosed(e); + } + + private void GoodsPropertyChanged(object sender, PropertyChangedEventArgs e) { } + } +} diff --git a/fix/tests/test_own_fix.py b/fix/tests/test_own_fix.py index 88ffe5a..dd06338 100644 --- a/fix/tests/test_own_fix.py +++ b/fix/tests/test_own_fix.py @@ -113,6 +113,21 @@ def test_own014_usercontrol_inserts_unloaded_detach(): "this.Unloaded += (s, e) => fThis.PropertyChanged -= data_PropertyChanged;") +# ---- OWN001 subscription folds into an existing OnClosed override ----------- + +def test_own001_subscription_folds_into_onclosed(): + rel = "Broker/FoldWindow.xaml.cs" + with _wrapped("own001-sub-fold", "OWN001") as (res, wd, _applier): + assert res.status == OK, res.ledger() + lines = _read(wd, rel) + # detach is folded into OnClosed (right after its '{'), NOT a fresh lambda + oc = next(i for i, line in enumerate(lines) if "override void OnClosed" in line) + assert lines[oc + 1].strip() == "{" + assert lines[oc + 2].strip() == ( + "fGoods.PropertyChanged -= new PropertyChangedEventHandler(GoodsPropertyChanged);") + assert "this.Closed += (s, e) =>" not in "".join(lines) # no stacked lambda + + # ---- OWN001 disposable field on a Window -> dispose on Closed --------------- def test_own001_disposable_field_disposes_on_closed(): From 1747ac37a5b425386717dbf5bd2291748d49b2c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 16:45:07 +0000 Subject: [PATCH 2/4] feat(own): widen lambda extraction to PropertyChanging + ErrorsChanged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two more unambiguous INotify-family event delegates to the lambda-extraction table (PropertyChangingEventArgs, DataErrorsChangedEventArgs). Names whose delegate differs across frameworks (Click, TextChanged -> RoutedEventArgs vs EventArgs) are deliberately NOT added — extracting them blindly could emit a wrong signature, so they stay suggest-only. +1 test. 22/22 + 7/7, -O. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa --- fix/fixarm/own_fix.py | 7 ++++++- fix/tests/test_own_fix.py | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/fix/fixarm/own_fix.py b/fix/fixarm/own_fix.py index 8b15a95..4426132 100644 --- a/fix/fixarm/own_fix.py +++ b/fix/fixarm/own_fix.py @@ -307,11 +307,16 @@ def _plan_local(lines, f, name): return edits, "using" -# well-known events whose delegate's EventArgs type we can name without a compiler +# Events whose delegate's EventArgs type is UNAMBIGUOUS without a compiler — the +# INotify* family. Names like Click/TextChanged are deliberately excluded: the same +# name maps to different delegates across frameworks (RoutedEventArgs vs EventArgs), +# so extracting them blindly could emit a wrong signature → they stay suggest-only. _EVENT_ARGS = { "PropertyChanged": "PropertyChangedEventArgs", + "PropertyChanging": "PropertyChangingEventArgs", "ListChanged": "ListChangedEventArgs", "CollectionChanged": "NotifyCollectionChangedEventArgs", + "ErrorsChanged": "DataErrorsChangedEventArgs", } _LAMBDA2 = re.compile(r"^\(\s*(\w+)\s*,\s*(\w+)\s*\)\s*=>\s*(.+?)\s*$") diff --git a/fix/tests/test_own_fix.py b/fix/tests/test_own_fix.py index dd06338..7131199 100644 --- a/fix/tests/test_own_fix.py +++ b/fix/tests/test_own_fix.py @@ -175,6 +175,29 @@ def test_own001_inline_lambda_extracted_and_detached(): # ---- refused shapes stay suggest-only: NOT patched ------------------------- +def test_lambda_extraction_more_delegates(): + # PropertyChanging is unambiguous (INotify family) -> extractable with the right args + src = ("public partial class W : Window\n" + "{\n" + " public W(Model m)\n" + " {\n" + " InitializeComponent();\n" + " m.PropertyChanging += (s, e) => Refresh();\n" + " }\n" + " private void Refresh() { }\n" + "}\n") + d, path = _tmp_cs(src) + try: + f = Finding("OWN001", "W.cs", 6, tool="own-check", + message="event 'm.PropertyChanging' is subscribed (handler '(s, e) => Refresh()') ...") + new, applied, skipped = plan_file(path, [f]) + assert [d for _, d in applied] == ["extract+detach"], skipped + assert "m.PropertyChanging += OnMPropertyChanging;" in new + assert "private void OnMPropertyChanging(object s, PropertyChangingEventArgs e) => Refresh();" in new + finally: + shutil.rmtree(d, ignore_errors=True) + + def test_block_lambda_is_not_patched(): # a block-body lambda can't be a clean expression method -> suggest-only, untouched rel = "Broker/DatabaseOptimizationWindow.xaml.cs" From d840115ceb111aa558419e543d640b7e19c81305 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 16:54:36 +0000 Subject: [PATCH 3/4] fix(own): sound fold + qualified extracted args (Codex + CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fold only when the source is a class MEMBER (Codex P2): folding a raw detach into OnClosed is out of scope if the source is a ctor param/local. _is_class_member gates it (members carry a modifier; params/locals don't); non-members keep the capturing lambda at the call site. (+test: ctor-local source -> lambda) - fully-qualify extracted event-args types (Codex P2): the generated method names the type explicitly, so PropertyChangingEventArgs etc. are now System.ComponentModel.* — compiles even if the file lacked the using. - restrict OnClosed fold to member depth (CodeRabbit Major): depth-gated scan so a nested type's OnClosed can't be folded into. (+test: nested OnClosed -> lambda) - tests: assert the 'Closed/fold' applied-detail; cover ErrorsChanged extraction. 24/24 own + 7/7 wrapper, normal and -O. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa --- fix/fixarm/own_fix.py | 51 ++++++++++++---- fix/tests/test_own_fix.py | 120 +++++++++++++++++++++++++++++--------- 2 files changed, 133 insertions(+), 38 deletions(-) diff --git a/fix/fixarm/own_fix.py b/fix/fixarm/own_fix.py index 4426132..ee63497 100644 --- a/fix/fixarm/own_fix.py +++ b/fix/fixarm/own_fix.py @@ -238,20 +238,46 @@ def _fold_target(lines, cls_idx, ev): """If the class already has a teardown method to fold cleanup into, return its block anchor. Today: a `protected override void OnClosed(...)` for ev == Closed — it runs exactly when the Window's Closed event would, so folding is equivalent and - cleaner than stacking another lambda.""" + cleaner than stacking another lambda. Only a member-depth OnClosed of THIS class + counts — a nested type's override (deeper brace depth) must not be folded into.""" if ev != "Closed": return None end = _class_close(lines, cls_idx) end = end if end is not None else len(lines) + depth, started = 0, False for i in range(cls_idx, end): - if re.search(r"\boverride\s+void\s+OnClosed\b", lines[i]): + if started and depth == 1 and re.search(r"\boverride\s+void\s+OnClosed\b", lines[i]): return _fold_after_open_brace(lines, i, end) + for ch in _code_skeleton(lines[i]): + if ch == "{": + depth, started = depth + 1, True + elif ch == "}": + depth -= 1 return None -def _plan_teardown(lines, f, idx, stmt, anchor_missing="site-not-found"): +_MODIFIER = r"(?:public|private|protected|internal|static|readonly|const|volatile|virtual|override|sealed|new|required)" + + +def _is_class_member(lines, cls_idx, root): + """Is `root` a member (field/property) of the class — i.e. in scope inside a method + body like OnClosed? `this` always is. Members carry an access/field modifier; + constructor parameters and locals do not, so requiring a modifier separates them. + Used to keep the fold sound: a ctor-local source must stay on the captured lambda.""" + if root == "this": + return True + end = _class_close(lines, cls_idx) + end = end if end is not None else len(lines) + decl = re.compile(rf"^\s*(\[[^\]]*\]\s*)?({_MODIFIER}\s+)+[\w<>\[\].,\s]*?\b{re.escape(root)}\b\s*(=|;|\{{|=>)") + return any(decl.match(_code_skeleton(lines[i])) for i in range(cls_idx, end)) + + +def _plan_teardown(lines, f, idx, stmt, anchor_missing="site-not-found", fold_root=None): """Subscription / disposable-field: run `stmt` on the owner's teardown. Folds into - an existing OnClosed override when present; otherwise adds a fresh teardown lambda.""" + an existing OnClosed override when present AND the source is a class member (so it + stays in scope there); otherwise adds a fresh teardown lambda at the call site, + which captures whatever is in scope (incl. ctor locals). fold_root=None ⇒ always + foldable (a disposable field is a member by construction).""" if idx is None: return None, anchor_missing if _in_unbraced_control_flow(lines, idx): @@ -262,7 +288,7 @@ def _plan_teardown(lines, f, idx, stmt, anchor_missing="site-not-found"): return None, "no-safe-teardown" cls_idx = _enclosing_class_idx(lines, idx) fold = _fold_target(lines, cls_idx, ev) if cls_idx is not None else None - if fold is not None: + if fold is not None and (fold_root is None or _is_class_member(lines, cls_idx, fold_root)): brace_idx, body_indent = fold return [(brace_idx + 1, brace_idx + 1, [f"{body_indent}{stmt};\n"])], f"{ev}/fold" indent = re.match(r"\s*", lines[idx]).group(0) @@ -312,11 +338,13 @@ def _plan_local(lines, f, name): # name maps to different delegates across frameworks (RoutedEventArgs vs EventArgs), # so extracting them blindly could emit a wrong signature → they stay suggest-only. _EVENT_ARGS = { - "PropertyChanged": "PropertyChangedEventArgs", - "PropertyChanging": "PropertyChangingEventArgs", - "ListChanged": "ListChangedEventArgs", - "CollectionChanged": "NotifyCollectionChangedEventArgs", - "ErrorsChanged": "DataErrorsChangedEventArgs", + # fully qualified: the extracted method names the type explicitly, so it must + # compile even if the file's lambda relied on type inference without the using. + "PropertyChanged": "System.ComponentModel.PropertyChangedEventArgs", + "PropertyChanging": "System.ComponentModel.PropertyChangingEventArgs", + "ListChanged": "System.ComponentModel.ListChangedEventArgs", + "CollectionChanged": "System.Collections.Specialized.NotifyCollectionChangedEventArgs", + "ErrorsChanged": "System.ComponentModel.DataErrorsChangedEventArgs", } _LAMBDA2 = re.compile(r"^\(\s*(\w+)\s*,\s*(\w+)\s*\)\s*=>\s*(.+?)\s*$") @@ -375,7 +403,8 @@ def _plan_one(lines, f): """Dispatch a finding to its shape's planner.""" shape, a, b = classify(f.message) if shape == NAMED_HANDLER_SUB: - return _plan_teardown(lines, f, _find_sub_line(lines, f.line, a), f"{a} -= {b}") + return _plan_teardown(lines, f, _find_sub_line(lines, f.line, a), f"{a} -= {b}", + fold_root=a.split(".")[0]) # source's root must be in scope to fold if shape == DISPOSABLE_FIELD: return _plan_teardown(lines, f, _find_ctor_anchor(lines, f.line), f"{a}?.Dispose()", anchor_missing="no-ctor-anchor") diff --git a/fix/tests/test_own_fix.py b/fix/tests/test_own_fix.py index 7131199..7f09426 100644 --- a/fix/tests/test_own_fix.py +++ b/fix/tests/test_own_fix.py @@ -117,15 +117,74 @@ def test_own014_usercontrol_inserts_unloaded_detach(): def test_own001_subscription_folds_into_onclosed(): rel = "Broker/FoldWindow.xaml.cs" - with _wrapped("own001-sub-fold", "OWN001") as (res, wd, _applier): - assert res.status == OK, res.ledger() - lines = _read(wd, rel) - # detach is folded into OnClosed (right after its '{'), NOT a fresh lambda + fdir = os.path.join(FIX, "own001-sub-fold") + before = load_findings(os.path.join(fdir, "before.findings.json")) + wd = _seed("own001-sub-fold") + try: + new, applied, skipped = plan_file(os.path.join(wd, rel), before) + assert [d for _, d in applied] == ["Closed/fold"], (applied, skipped) # fold, not plain Closed + lines = new.splitlines(keepends=True) oc = next(i for i, line in enumerate(lines) if "override void OnClosed" in line) assert lines[oc + 1].strip() == "{" assert lines[oc + 2].strip() == ( "fGoods.PropertyChanged -= new PropertyChangedEventHandler(GoodsPropertyChanged);") - assert "this.Closed += (s, e) =>" not in "".join(lines) # no stacked lambda + assert "this.Closed += (s, e) =>" not in new # no stacked lambda + finally: + shutil.rmtree(wd, ignore_errors=True) + + +def test_fold_skips_ctor_local_source(): + # source `goods` is a ctor parameter (not a member) -> folding the raw detach into + # OnClosed would be out of scope; must keep the capturing lambda at the call site. + src = ("public partial class W : Window\n" + "{\n" + " public W(Goods goods)\n" + " {\n" + " InitializeComponent();\n" + " goods.PropertyChanged += new PropertyChangedEventHandler(H);\n" + " }\n" + " protected override void OnClosed(EventArgs e)\n" + " {\n" + " base.OnClosed(e);\n" + " }\n" + " private void H(object s, PropertyChangedEventArgs e) { }\n" + "}\n") + d, path = _tmp_cs(src) + try: + f = Finding("OWN001", "W.cs", 6, tool="own-check", + message="event 'goods.PropertyChanged' is subscribed (handler 'new PropertyChangedEventHandler(H)') ...") + new, applied, skipped = plan_file(path, [f]) + assert [d for _, d in applied] == ["Closed"], (applied, skipped) # lambda, not fold + assert "this.Closed += (s, e) => goods.PropertyChanged -= new PropertyChangedEventHandler(H);" in new + # OnClosed body must be untouched (no out-of-scope detach folded in) + assert "goods.PropertyChanged -=" in new and new.count("goods.PropertyChanged -=") == 1 + finally: + shutil.rmtree(d, ignore_errors=True) + + +def test_fold_ignores_nested_type_onclosed(): + # the field's class has no OnClosed; a NESTED type does. Must not fold into it. + src = ("public partial class Outer : Window\n" + "{\n" + " private readonly Timer _t;\n" + " public Outer()\n" + " {\n" + " InitializeComponent();\n" + " }\n" + " private class Inner\n" + " {\n" + " protected override void OnClosed(EventArgs e) { }\n" + " }\n" + "}\n") + d, path = _tmp_cs(src) + try: + f = Finding("OWN001", "Outer.cs", 3, tool="own-check", + message="IDisposable field '_t' (type 'Timer') is never disposed — its owner 'Outer' leaks it") + new, applied, skipped = plan_file(path, [f]) + assert [d for _, d in applied] == ["Closed"], (applied, skipped) # lambda, not folded into Inner + assert "this.Closed += (s, e) => _t?.Dispose();" in new + finally: + shutil.rmtree(d, ignore_errors=True) # ---- OWN001 disposable field on a Window -> dispose on Closed --------------- @@ -168,34 +227,41 @@ def test_own001_inline_lambda_extracted_and_detached(): text = "".join(_read(wd, rel)) assert "stage.PropertyChanged += OnStagePropertyChanged;" in text # method group assert "this.Closed += (s, e) => stage.PropertyChanged -= OnStagePropertyChanged;" in text - assert ('private void OnStagePropertyChanged(object s2, PropertyChangedEventArgs e2) ' - '=> OnPropertyChanged("Stages");') in text # extracted method + assert ('private void OnStagePropertyChanged(object s2, ' + 'System.ComponentModel.PropertyChangedEventArgs e2) ' + '=> OnPropertyChanged("Stages");') in text # extracted, qualified args assert "+= (s2, e2) =>" not in text # the lambda is gone # ---- refused shapes stay suggest-only: NOT patched ------------------------- def test_lambda_extraction_more_delegates(): - # PropertyChanging is unambiguous (INotify family) -> extractable with the right args - src = ("public partial class W : Window\n" - "{\n" - " public W(Model m)\n" - " {\n" - " InitializeComponent();\n" - " m.PropertyChanging += (s, e) => Refresh();\n" - " }\n" - " private void Refresh() { }\n" - "}\n") - d, path = _tmp_cs(src) - try: - f = Finding("OWN001", "W.cs", 6, tool="own-check", - message="event 'm.PropertyChanging' is subscribed (handler '(s, e) => Refresh()') ...") - new, applied, skipped = plan_file(path, [f]) - assert [d for _, d in applied] == ["extract+detach"], skipped - assert "m.PropertyChanging += OnMPropertyChanging;" in new - assert "private void OnMPropertyChanging(object s, PropertyChangingEventArgs e) => Refresh();" in new - finally: - shutil.rmtree(d, ignore_errors=True) + # both newly-added INotify-family events are unambiguous -> extractable with the + # right (fully-qualified) args type. + cases = [ + ("PropertyChanging", "System.ComponentModel.PropertyChangingEventArgs", "OnMPropertyChanging"), + ("ErrorsChanged", "System.ComponentModel.DataErrorsChangedEventArgs", "OnMErrorsChanged"), + ] + for ev, args_type, method in cases: + src = ("public partial class W : Window\n" + "{\n" + " public W(Model m)\n" + " {\n" + " InitializeComponent();\n" + f" m.{ev} += (s, e) => Refresh();\n" + " }\n" + " private void Refresh() { }\n" + "}\n") + d, path = _tmp_cs(src) + try: + f = Finding("OWN001", "W.cs", 6, tool="own-check", + message=f"event 'm.{ev}' is subscribed (handler '(s, e) => Refresh()') ...") + new, applied, skipped = plan_file(path, [f]) + assert [dt for _, dt in applied] == ["extract+detach"], (ev, skipped) + assert f"m.{ev} += {method};" in new + assert f"private void {method}(object s, {args_type} e) => Refresh();" in new + finally: + shutil.rmtree(d, ignore_errors=True) def test_block_lambda_is_not_patched(): From 02396b70851358a16d426ba6a0e31f01d90187d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 16:59:37 +0000 Subject: [PATCH 4/4] fix(own): match OnClosed on skeleton + tidy test divider (CodeRabbit) - _fold_target now matches the OnClosed override against _code_skeleton(line) instead of the raw line, so a commented-out/string '//... OnClosed ...' at member depth can't be picked as a bogus fold anchor. (+test) - move the 'refused shapes' test divider below the extraction test it was mistakenly heading (cosmetic). 25/25 own + 7/7 wrapper, normal and -O. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa --- fix/fixarm/own_fix.py | 2 +- fix/tests/test_own_fix.py | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/fix/fixarm/own_fix.py b/fix/fixarm/own_fix.py index ee63497..60f9b60 100644 --- a/fix/fixarm/own_fix.py +++ b/fix/fixarm/own_fix.py @@ -246,7 +246,7 @@ def _fold_target(lines, cls_idx, ev): end = end if end is not None else len(lines) depth, started = 0, False for i in range(cls_idx, end): - if started and depth == 1 and re.search(r"\boverride\s+void\s+OnClosed\b", lines[i]): + if started and depth == 1 and re.search(r"\boverride\s+void\s+OnClosed\b", _code_skeleton(lines[i])): return _fold_after_open_brace(lines, i, end) for ch in _code_skeleton(lines[i]): if ch == "{": diff --git a/fix/tests/test_own_fix.py b/fix/tests/test_own_fix.py index 7f09426..eab3e25 100644 --- a/fix/tests/test_own_fix.py +++ b/fix/tests/test_own_fix.py @@ -162,6 +162,28 @@ def test_fold_skips_ctor_local_source(): shutil.rmtree(d, ignore_errors=True) +def test_fold_ignores_commented_onclosed(): + # a commented-out OnClosed must not be matched as a fold anchor (skeleton-based) + src = ("public partial class W : Window\n" + "{\n" + " private readonly Timer _t;\n" + " public W()\n" + " {\n" + " InitializeComponent();\n" + " }\n" + " // protected override void OnClosed(EventArgs e) { }\n" + "}\n") + d, path = _tmp_cs(src) + try: + f = Finding("OWN001", "W.cs", 3, tool="own-check", + message="IDisposable field '_t' (type 'Timer') is never disposed — its owner 'W' leaks it") + new, applied, skipped = plan_file(path, [f]) + assert [dt for _, dt in applied] == ["Closed"], (applied, skipped) # lambda, not a bogus fold + assert "this.Closed += (s, e) => _t?.Dispose();" in new + finally: + shutil.rmtree(d, ignore_errors=True) + + def test_fold_ignores_nested_type_onclosed(): # the field's class has no OnClosed; a NESTED type does. Must not fold into it. src = ("public partial class Outer : Window\n" @@ -233,7 +255,7 @@ def test_own001_inline_lambda_extracted_and_detached(): assert "+= (s2, e2) =>" not in text # the lambda is gone -# ---- refused shapes stay suggest-only: NOT patched ------------------------- +# ---- wider lambda-extraction delegates (PropertyChanging / ErrorsChanged) -- def test_lambda_extraction_more_delegates(): # both newly-added INotify-family events are unambiguous -> extractable with the @@ -264,6 +286,8 @@ def test_lambda_extraction_more_delegates(): shutil.rmtree(d, ignore_errors=True) +# ---- refused shapes stay suggest-only: NOT patched ------------------------- + def test_block_lambda_is_not_patched(): # a block-body lambda can't be a clean expression method -> suggest-only, untouched rel = "Broker/DatabaseOptimizationWindow.xaml.cs"