Skip to content

Commit 30bbeac

Browse files
ZviBaratzclaude
andauthored
fix(ego-lint): reduce resource-tracking/no-destroy-method false positives (#86)
## Summary - Expand instantiation cleanup tracking in `build-resource-graph.py` to recognize custom cleanup method calls (e.g., `disconnect_all()`, `remove()`) on child references inside parent cleanup method bodies — not just `.destroy()` and `.disable()` - Add `parent_calls_destroy` guard to Case 1 in `detect_orphans()` so utility classes without a standard cleanup method are not flagged when their parent manages cleanup via a custom method call - Fixes 10 FPs in blur-my-shell (utility classes with `disconnect_all`/`remove`) Closes #77 ## Test plan - [x] New test fixture `parent-managed-cleanup@test` verifies custom cleanup suppression - [x] All 607 test assertions pass - [ ] Field test: `bash scripts/field-test-runner.sh --no-fetch --extension blur-my-shell` shows FP reduction 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ff05efb commit 30bbeac

File tree

7 files changed

+70
-0
lines changed

7 files changed

+70
-0
lines changed

skills/ego-lint/references/rules-reference.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2019,6 +2019,7 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
20192019
- **Checked by**: check-resources.py → build-resource-graph.py
20202020
- **Rule**: Module creates resources but has no cleanup method (`destroy()`, `disable()`, or `onDestroy()`) for cleanup.
20212021
- **Fix**: Add a `destroy()` method that cleans up all resources, or use `onDestroy()` connected via `this.connect('destroy', this.onDestroy.bind(this))`.
2022+
- **Note**: Suppressed when the parent calls any method on the child ref (e.g., `this._helper.disconnect_all()`) or nulls it inside a cleanup method body. This covers utility classes with custom cleanup methods managed by their parent.
20222023
20232024
### resource-tracking/destroy-not-called
20242025
- **Severity**: advisory

skills/ego-lint/scripts/build-resource-graph.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,28 @@ def scan_file(file_path, ext_dir):
316316
inst['destroy_line'] = lineno
317317
break
318318

319+
# Second pass: for instantiations still unresolved, check if ANY method
320+
# call or null assignment on the ref appears inside a cleanup method body.
321+
# This catches custom cleanup patterns like ref.disconnect_all(),
322+
# ref.remove(), ref.cleanup(), etc.
323+
for inst in instantiates:
324+
if inst['has_destroy_call']:
325+
continue
326+
ref = inst['stored_as']
327+
if not ref:
328+
continue
329+
any_cleanup_pat = re.compile(
330+
re.escape(ref) + r'(?:\??\.\w+\s*\(|\s*=\s*null\b)'
331+
)
332+
for method_name in ('destroy', 'disable', '_destroy', 'onDestroy'):
333+
mb = find_method_body(content, method_name)
334+
if mb:
335+
_, _, body = mb
336+
if any_cleanup_pat.search(body):
337+
inst['has_destroy_call'] = True
338+
inst['destroy_line'] = mb[0]
339+
break
340+
319341
# Track widget refs that are added as children (auto-cleanup on parent destroy)
320342
child_refs = set()
321343
child_add_pat = re.compile(
@@ -499,6 +521,8 @@ def detect_orphans(file_scans, ownership):
499521

500522
# Case 1: Module creates resources but has no cleanup method
501523
if not has_cleanup_method:
524+
if parent_calls_destroy:
525+
continue # Parent manages this child via custom cleanup method
502526
for c in creates:
503527
orphans.append({
504528
'file': rel,
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Parent-managed custom cleanup assertions
2+
# Sourced by run-tests.sh — uses run_lint, assert_output_contains, assert_exit_code, etc.
3+
4+
# --- parent-managed-cleanup (custom cleanup method recognized) ---
5+
echo "=== parent-managed-cleanup ==="
6+
run_lint "parent-managed-cleanup@test"
7+
assert_exit_code "exits with 0 (no blocking issues)" 0
8+
assert_output_not_contains "no no-destroy-method for parent-managed child" "\[WARN\].*resource-tracking/no-destroy-method"
9+
assert_output_not_contains "no destroy-not-called for parent-managed child" "\[WARN\].*resource-tracking/destroy-not-called"
10+
assert_output_contains "resource tracking ran" "(PASS|WARN|FAIL).*resource-tracking"
11+
echo ""
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
SPDX-License-Identifier: GPL-2.0-or-later
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
2+
import {SignalHelper} from './helper.js';
3+
4+
export default class TestExtension extends Extension {
5+
enable() {
6+
this._signals = new SignalHelper();
7+
this._signals.connect_all();
8+
}
9+
10+
disable() {
11+
this._signals.disconnect_all();
12+
this._signals = null;
13+
}
14+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export class SignalHelper {
2+
connect_all() {
3+
this._handlerId = global.display.connect('window-created', () => {});
4+
}
5+
6+
disconnect_all() {
7+
if (this._handlerId) {
8+
global.display.disconnect(this._handlerId);
9+
this._handlerId = null;
10+
}
11+
}
12+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"uuid": "parent-managed-cleanup@test",
3+
"name": "Parent Managed Cleanup Test",
4+
"description": "Tests custom cleanup method recognition",
5+
"shell-version": ["48"],
6+
"url": "https://example.com"
7+
}

0 commit comments

Comments
 (0)