feat(ego-lint): add .destroy without parentheses detection (R-LIFE-23) (#50)

## Summary

- Add Tier 2 check in `check-lifecycle.py` detecting `.destroy` property
access without `()` — a silent no-op bug where objects are never
destroyed
- Suppress false positives for callback references
(`connect`/`connectSmart`/`bind`), existence checks (`if`/`typeof`),
property assignments, and destructuring
- Add test fixture with TP (forEach arrow body) and FP patterns (connect
callback, existence check, typeof check)

Field test validation:
- **blur-my-shell**: correctly detects `coverflow_alt_tab.js:69`
(`actors.forEach(a => a.destroy)`)
- **appindicator**: correctly suppresses `connectSmart(menu, 'destroy',
this, this.destroy)` callback reference

Closes #38

## Test plan

- [x] `bash tests/run-tests.sh` — 562 passed, 0 failed
- [x] Field test: blur-my-shell TP detected, appindicator FP suppressed
- [x] Fixture covers: forEach arrow body (TP), connectSmart callback
(FP), if-check (FP), typeof-check (FP)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: ZviBaratz

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

@@ -2084,6 +2084,22 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
 - **Known limitations**: Multi-line `.connect(\n'changed::...'` calls are not detected (per-line scanning). Signal IDs stored via `.push()` rather than direct assignment may false-positive.
 - **Tested by**: `tests/fixtures/gsettings-bare-connect@test/`, `tests/fixtures/gsettings-auto-cleanup@test/`
 
+### R-LIFE-23: .destroy without parentheses
+- **Severity**: advisory
+- **Checked by**: check-lifecycle.py
+- **Rule**: `.destroy` must be called as a method (`.destroy()`), not accessed as a property (`.destroy`).
+- **Rationale**: In JavaScript, `obj.destroy` without parentheses is a property access that evaluates to the function object but does not call it. The object is never destroyed, causing a silent resource leak. This is a common typo, especially in `forEach` callbacks: `actors.forEach(a => a.destroy)` does nothing.
+- **Fix**: Add parentheses: `actors.forEach(a => a.destroy())`.
+- **Not matched**: `.destroy()` and `.destroy?.()` (actual calls), `.destroyAll()` etc. (different methods) are excluded by the primary regex pattern.
+- **Suppressed when**:
+  - `connect`/`connectSmart`/`connectObject(` before `.destroy` (callback reference, but not `disconnect()`)
+  - `.bind(`/`.call(`/`.apply(` before or after `.destroy` (Function.prototype reference)
+  - `.destroy` inside an `if()` condition or preceded by `typeof` (existence check)
+  - Followed by `=` but not `==` (property assignment)
+  - `{ destroy }` on the same line (destructuring)
+  - Ternary `?` after `.destroy` but not `?.` (existence check)
+- **Tested by**: `tests/fixtures/destroy-no-call@test/`
+
 ---
 
 ## Security (R-SEC) — continued

skills/ego-lint/scripts/check-lifecycle.py

@@ -25,6 +25,7 @@
   - R-LIFE-20: Bus name ownership without release
   - R-LIFE-21: GSettings signal leak (bare connect without disconnect)
   - R-LIFE-22: Stage actor leak (add_child/addChrome without remove)
+  - R-LIFE-23: .destroy without parentheses (property access, not method call)
   - R-LIFE-24: MessageTray source leak (add without destroy)
   - R-SEC-16: Clipboard + keybinding cross-reference
   - R-FILE-07: Missing export default class
@@ -947,6 +948,79 @@ def check_bus_name_lifecycle(ext_dir):
     # If no bus name ownership, skip silently
 
 
+def check_destroy_without_call(ext_dir):
+    """R-LIFE-23: .destroy without () is property access, not method call."""
+    js_files = find_js_files(ext_dir, exclude_prefs=True)
+    if not js_files:
+        return
+
+    violations = []
+    for filepath in js_files:
+        content = strip_comments(read_file(filepath))
+        rel = os.path.relpath(filepath, ext_dir)
+
+        for lineno, line in enumerate(content.splitlines(), 1):
+            # Match .destroy at word boundary (excludes .destroyAll etc.), not followed by () or ?.() (excludes actual calls)
+            for m in re.finditer(r'\.destroy\b(?!\s*(\?\.)?\s*\()', line):
+                before = line[:m.start()]
+                after = line[m.end():]
+
+                # Skip: callback reference in signal connection
+                if re.search(r'(?<!dis)connect\w*\s*\(', before):
+                    continue
+                # Skip: Function.prototype reference (.bind/.call/.apply)
+                if re.search(r'\.(bind|call|apply)\s*\(', before):
+                    continue
+                if re.search(r'\.(bind|call|apply)\s*\(', after):
+                    continue
+
+                # Skip: property existence check (only when .destroy is inside the condition)
+                if re.search(r'\btypeof\b', before):
+                    continue
+                if_match = re.search(r'\bif\s*\(', before)
+                if if_match:
+                    # Check .destroy is inside the condition, not in the body
+                    after_if_open = before[if_match.end():]
+                    depth = 1
+                    for ch in after_if_open:
+                        if ch == '(':
+                            depth += 1
+                        elif ch == ')':
+                            depth -= 1
+                            if depth == 0:
+                                break
+                    if depth > 0:
+                        # Condition still open — .destroy is being tested
+                        continue
+
+                # Skip: ternary existence check (e.g., actor.destroy ? actor.destroy() : null)
+                after_stripped = after.lstrip()
+                if after_stripped.startswith('?') and not after_stripped.startswith('?.'):
+                    continue
+
+                # Skip: property assignment (not comparison)
+                if after_stripped.startswith('=') and not after_stripped.startswith('=='):
+                    continue
+
+                # Skip: destructuring
+                if re.search(r'\{\s*destroy\b', line):
+                    continue
+
+                violations.append(f"{rel}:{lineno}")
+
+    if violations:
+        locs = violations[:5]
+        for loc in locs:
+            result("WARN", "lifecycle/destroy-no-call",
+                   f"{loc}: .destroy without () — property access, not method call")
+        if len(violations) > 5:
+            result("WARN", "lifecycle/destroy-no-call",
+                   f"(and {len(violations) - 5} more)")
+    else:
+        result("PASS", "lifecycle/destroy-no-call",
+               "All .destroy references include parentheses")
+
+
 def check_widget_lifecycle(ext_dir):
     """Detect widgets created in enable() but not destroyed in disable()."""
     ext_file = os.path.join(ext_dir, 'extension.js')
@@ -1239,6 +1313,7 @@ def main():
     check_clipboard_network(ext_dir)
     check_soup_session_abort(ext_dir)
     check_destroy_then_null(ext_dir)
+    check_destroy_without_call(ext_dir)
     check_widget_lifecycle(ext_dir)
     check_stage_actor_lifecycle(ext_dir)
     check_message_tray_lifecycle(ext_dir)

tests/fixtures/destroy-no-call@test/LICENSE

@@ -0,0 +1 @@
+SPDX-License-Identifier: GPL-2.0-or-later

tests/fixtures/destroy-no-call@test/extension.js

@@ -0,0 +1,54 @@
+import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
+import St from 'gi://St';
+import * as Util from './util.js';
+
+export default class TestExtension extends Extension {
+    enable() {
+        this._actors = [new St.Label(), new St.Button()];
+        this._widget = new St.Label({text: 'test'});
+    }
+
+    disable() {
+        // BAD: .destroy without () — property access, not method call
+        this._actors.forEach((actor) => actor.destroy);
+
+        // BAD: .destroy in if-body, not condition (should still flag)
+        if (this._shouldCleanup) this._widget.destroy;
+
+        // GOOD: .destroy() with parens
+        this._widget.destroy();
+
+        // GOOD: optional chaining call (FP suppression)
+        this._widget.destroy?.();
+        this._widget = null;
+
+        // GOOD: callback reference in connectSmart (FP suppression)
+        Util.connectSmart(this._widget, 'destroy', this, this.destroy);
+
+        // GOOD: existence check (FP suppression)
+        if (this._widget.destroy) {
+            this._widget.destroy();
+        }
+
+        // GOOD: typeof check (FP suppression)
+        if (typeof this._widget.destroy === 'function') {
+            this._widget.destroy();
+        }
+
+        // GOOD: bound method reference (FP suppression)
+        const cleanup = this._widget.destroy.bind(this._widget);
+
+        // GOOD: .call/.apply invocation (FP suppression)
+        St.Widget.prototype.destroy.call(this._widget);
+        St.Widget.prototype.destroy.apply(this._widget, []);
+
+        // GOOD: property assignment (FP suppression)
+        this._widget.destroy = null;
+
+        // GOOD: ternary existence check (FP suppression)
+        this._widget.destroy ? this._widget.destroy() : null;
+
+        // GOOD: destructuring (FP suppression)
+        const {destroy} = this._widget;
+    }
+}

tests/fixtures/destroy-no-call@test/metadata.json

@@ -0,0 +1 @@
+{"uuid": "destroy-no-call@test", "name": "Destroy No Call Test", "description": "Tests .destroy without parentheses detection", "shell-version": ["48"], "url": "https://github.com/test/destroy-no-call"}

tests/run-tests.sh

@@ -634,6 +634,14 @@ assert_output_not_contains "no no-destroy-method for widget with onDestroy" "\[W
 assert_output_contains "resource tracking ran" "(PASS|WARN|FAIL).*resource-tracking"
 echo ""
 
+# --- destroy-no-call ---
+echo "=== destroy-no-call ==="
+run_lint "destroy-no-call@test"
+assert_exit_code "exits with 0 (advisory only)" 0
+assert_output_contains "detects .destroy without parens" "\[WARN\].*lifecycle/destroy-no-call.*without \(\)"
+assert_output_count "exactly 2 destroy-no-call warnings" "\[WARN\].*lifecycle/destroy-no-call" 2
+echo ""
+
 # Extended assertion files (auto-sourced from assertions/ directory)
 ASSERTIONS_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/assertions"
 for assertion_file in "$ASSERTIONS_DIR"/*.sh; do