Skip to content

Commit 3a84524

Browse files
ZviBaratzclaude
andcommitted
refactor(ego-lint): unify service/ path filtering and document exclusions
Replace 6 inline string-based service/ filters in check-lifecycle.py with a shared _skip_non_lifecycle() helper using component-based path matching (consistent with check-quality.py). The string approach ('/service/' in path) could false-match paths like myservice-backup/. Add scope exclusion notes to R-LIFE-02, R-LIFE-06, and R-QUAL-04 in rules-reference.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 12fe1d9 commit 3a84524

File tree

2 files changed

+20
-16
lines changed

2 files changed

+20
-16
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,7 @@ Heuristic rules that detect code patterns commonly seen in AI-generated or over-
941941
- **Rule**: Extension code should avoid declaring mutable variables (`let` or `var`) at module scope (outside any class or function).
942942
- **Rationale**: Module-level mutable state persists across enable/disable cycles, leading to subtle bugs. GNOME Shell extensions should keep all mutable state inside the `Extension` class instance, which is created fresh on each enable cycle.
943943
- **Fix**: Move mutable state into the Extension class as instance properties. Use `const` for module-level declarations that are truly constant (imports, enums, configuration).
944+
- **Scope exclusion**: Files in `service/` are excluded (daemon processes have different lifecycle — no enable/disable).
944945

945946
### R-QUAL-05: Empty catch blocks
946947
- **Severity**: advisory
@@ -1187,6 +1188,7 @@ disable() {
11871188
- **Rule**: `timeout_add` or `idle_add` call whose return value is not stored.
11881189
- **Rationale**: Without the source ID, the timeout cannot be removed in disable(), causing callbacks after extension teardown.
11891190
- **Fix**: Store the return value: `this._timeoutId = GLib.timeout_add(...)` and call `GLib.Source.remove(this._timeoutId)` in disable().
1191+
- **Scope exclusion**: Files in `service/` are excluded (daemon processes have different lifecycle — no enable/disable).
11901192
- **Tested by**: `tests/fixtures/lifecycle-basic@test/`
11911193

11921194
#### Example
@@ -1280,6 +1282,7 @@ export default class MyExtension extends Extension {
12801282
- **Rule**: Callbacks passed to `GLib.timeout_add()` or `GLib.idle_add()` should explicitly return `GLib.SOURCE_REMOVE` or `GLib.SOURCE_CONTINUE`.
12811283
- **Rationale**: If a timeout callback does not return `GLib.SOURCE_REMOVE` (or `false`), the default return value of `undefined` is falsy and the timeout is removed — but this is implicit and confusing. If the intent is to repeat, forgetting `SOURCE_CONTINUE` silently breaks the timer. Explicit return values make the intent clear and are expected by EGO reviewers.
12821284
- **Fix**: Add `return GLib.SOURCE_REMOVE;` for one-shot timeouts or `return GLib.SOURCE_CONTINUE;` for repeating timeouts at the end of the callback.
1285+
- **Scope exclusion**: Files in `service/` are excluded (daemon processes have different lifecycle — no enable/disable).
12831286
12841287
### R-LIFE-07: D-Bus proxy without signal disconnect
12851288
- **Severity**: advisory

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

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,17 @@
4242
import sys
4343

4444

45+
# Directories without enable()/disable() lifecycle
46+
_NON_LIFECYCLE_DIRS = frozenset({'service'})
47+
48+
49+
def _skip_non_lifecycle(filepath, ext_dir):
50+
"""Return True if filepath is inside a non-lifecycle directory."""
51+
rel = os.path.relpath(filepath, ext_dir)
52+
parts = rel.replace(os.sep, '/').split('/')
53+
return any(p in _NON_LIFECYCLE_DIRS for p in parts)
54+
55+
4556
def result(status, check, detail):
4657
print(f"{status}|{check}|{detail}")
4758

@@ -171,9 +182,7 @@ def check_untracked_timeouts(ext_dir):
171182
if not js_files:
172183
return
173184

174-
# Skip service/ directory (different lifecycle — no enable/disable)
175-
js_files = [f for f in js_files
176-
if '/service/' not in f.replace(os.sep, '/')]
185+
js_files = [f for f in js_files if not _skip_non_lifecycle(f, ext_dir)]
177186

178187
untracked = []
179188
for filepath in js_files:
@@ -258,9 +267,7 @@ def check_timeout_return_value(ext_dir):
258267
if not js_files:
259268
return
260269

261-
# Skip service/ directory (different lifecycle)
262-
js_files = [f for f in js_files
263-
if '/service/' not in f.replace(os.sep, '/')]
270+
js_files = [f for f in js_files if not _skip_non_lifecycle(f, ext_dir)]
264271

265272
missing = []
266273
for filepath in js_files:
@@ -1233,9 +1240,7 @@ def check_gsettings_signal_leak(ext_dir):
12331240
if not js_files:
12341241
return
12351242

1236-
# Skip service/ directory (different lifecycle)
1237-
js_files = [f for f in js_files
1238-
if '/service/' not in f.replace(os.sep, '/')]
1243+
js_files = [f for f in js_files if not _skip_non_lifecycle(f, ext_dir)]
12391244
if not js_files:
12401245
return
12411246

@@ -1325,9 +1330,7 @@ def check_dbus_signal_leak(ext_dir):
13251330
if not js_files:
13261331
return
13271332

1328-
# Skip service/ directory (different lifecycle)
1329-
js_files = [f for f in js_files
1330-
if '/service/' not in f.replace(os.sep, '/')]
1333+
js_files = [f for f in js_files if not _skip_non_lifecycle(f, ext_dir)]
13311334
if not js_files:
13321335
return
13331336

@@ -1436,8 +1439,7 @@ def check_module_scope_state(ext_dir):
14361439
if not files:
14371440
return
14381441

1439-
# Skip service/ directory (different lifecycle model)
1440-
files = [f for f in files if '/service/' not in f.replace(os.sep, '/')]
1442+
files = [f for f in files if not _skip_non_lifecycle(f, ext_dir)]
14411443

14421444
MODULE_STATE_RE = re.compile(
14431445
r'(?:const|let|var)\s+(\w+)\s*=\s*new\s+(Map|Set)\s*\(')
@@ -1504,8 +1506,7 @@ def check_module_scope_prototype(ext_dir):
15041506
if not files:
15051507
return
15061508

1507-
# Skip service/ directory (different lifecycle model)
1508-
files = [f for f in files if '/service/' not in f.replace(os.sep, '/')]
1509+
files = [f for f in files if not _skip_non_lifecycle(f, ext_dir)]
15091510

15101511
PROTO_ASSIGN = re.compile(r'([\w.]+\.prototype\.\w+)\s*=(?!=)')
15111512
PROTO_OBJ_ASSIGN = re.compile(

0 commit comments

Comments
 (0)