Skip to content

Commit d1fa661

Browse files
ZviBaratzclaude
andauthored
fix(ego-lint): skip signal callback bodies in constructor init check (#129)
## Summary - Add `filter_signal_callbacks()` to `check-init.py` that excludes signal callback body lines (`.connect()`, `.connectObject()`) from the constructor init-time check, while still checking the `.connect()` line itself (the receiver may be a Shell global accessed at construction time) - Add test fixture `init-constructor-signal@test` with both `.connect()` and `.connectObject()` callbacks containing Shell globals, plus a direct Shell global access that should still be flagged - Mark tiling-shell annotation for constructor signal callback FP as fixed ## Context Issue #118 identified 8 FPs for `init/shell-modification` across tiling-shell and v-shell. Research showed that 6 of 8 were already resolved by prior PRs (#21 scoped constructor checks to extension.js, #88 added arrow fn exemption). The remaining 2 (tiling-shell `globalState.js` constructor signal callbacks) are addressed by this PR. Signal callbacks are deferred — they don't execute during construction, only when the signal fires at runtime. The `.connect()` line itself is kept because the receiver object (e.g., `Main.panel.connect(...)`) IS accessed immediately during construction. ## Test plan - [x] New fixture `init-constructor-signal@test` verifies: - Direct Shell global in constructor is flagged (line 10) - `.connect()` callback body is NOT flagged (lines 15-17) - `.connectObject()` callback body is NOT flagged (lines 21-24) - [x] All 653 existing test assertions pass (0 regressions) - [x] Existing init fixtures unchanged: `init-time-safety`, `init-helper-constructor`, `init-modification`, `constructor-gobject` Closes #118 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 38cbb59 commit d1fa661

File tree

6 files changed

+91
-1
lines changed

6 files changed

+91
-1
lines changed

field-tests/annotations/tiling-shell.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ findings:
2323
notes: "get_maximized ? .unmaximize(MaximizeFlags) : .unmaximize() — same fix as R-VER49-08."
2424
- id: "init/shell-modification::constructor signal callback"
2525
classification: fp
26-
notes: "globalState.js — inside constructor signal callback, not module scope. 2 instances."
26+
notes: "globalState.js — inside constructor signal callback, not module scope. 2 instances. Fixed: signal callback filter in constructor check."
2727
- id: "init/shell-modification::module-scope arrow fn definition"
2828
classification: fp
2929
notes: "ui.js — const fn = () => Main.layoutManager.monitors is lazy, not executed at load time. Fixed: arrow fn skip."

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,44 @@ def extract_constructor_lines(content_lines):
170170
return constructor_lines
171171

172172

173+
# Signal connection patterns — callbacks are deferred, not executed during
174+
# construction. Shell globals inside these callbacks are not init-time
175+
# violations.
176+
SIGNAL_CONNECT = re.compile(r'\.connect(?:Object)?\s*\(')
177+
178+
179+
def filter_signal_callbacks(ctor_lines):
180+
"""Remove lines inside signal callback bodies from constructor lines.
181+
182+
Signal callbacks (.connect(), .connectObject()) are deferred — they don't
183+
execute during construction, so Shell globals inside them are not init-time
184+
violations. The .connect() line itself is kept (the receiver may be a Shell
185+
global that IS accessed at construction time).
186+
"""
187+
filtered = []
188+
callback_depth = 0
189+
190+
for lineno, line in ctor_lines:
191+
if callback_depth > 0:
192+
# Inside a callback body — track depth and skip
193+
callback_depth += line.count('{') - line.count('}')
194+
if callback_depth < 0:
195+
callback_depth = 0
196+
continue
197+
198+
# Include this line (even .connect() lines — the receiver is
199+
# accessed at construction time)
200+
filtered.append((lineno, line))
201+
202+
# If this line starts a signal callback, begin tracking its body
203+
if SIGNAL_CONNECT.search(line):
204+
net = line.count('{') - line.count('}')
205+
if net > 0:
206+
callback_depth = net
207+
208+
return filtered
209+
210+
173211
def check_init_modifications(ext_dir):
174212
"""R-INIT-01: Detect Shell modifications outside enable()/disable()."""
175213
js_files = find_js_files(ext_dir)
@@ -215,6 +253,7 @@ def check_init_modifications(ext_dir):
215253
is_extension_js = os.path.basename(filepath) == 'extension.js'
216254
if is_extension_js:
217255
ctor_lines = extract_constructor_lines(lines)
256+
ctor_lines = filter_signal_callbacks(ctor_lines)
218257
for lineno, line in ctor_lines:
219258
if is_skip_line(line):
220259
continue
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: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import GObject from 'gi://GObject';
2+
import * as Main from 'resource:///org/gnome/shell/ui/main.js';
3+
import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
4+
5+
export default class SignalCallbackExtension extends Extension {
6+
constructor(metadata) {
7+
super(metadata);
8+
9+
// VIOLATION: Shell global accessed directly in constructor
10+
this._hasOverview = Main.sessionMode.hasOverview;
11+
12+
// OK: Shell global inside signal callback — deferred, not executed
13+
// during construction
14+
this._signalId = this.connect('notify', () => {
15+
const monitors = Main.layoutManager.monitors;
16+
Main.panel.addToStatusArea('test', null);
17+
});
18+
19+
// OK: multi-line connectObject callback
20+
this.connectObject('notify::visible', () => {
21+
if (Main.overview.visible) {
22+
Main.messageTray.close();
23+
}
24+
}, this);
25+
}
26+
27+
enable() {
28+
Main.panel.addToStatusArea('test', this);
29+
}
30+
31+
disable() {
32+
this.disconnect(this._signalId);
33+
}
34+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"uuid": "init-constructor-signal@test",
3+
"name": "Init Constructor Signal Test",
4+
"description": "Tests that signal callbacks in constructors are not flagged",
5+
"shell-version": ["48"],
6+
"url": "https://github.com/test/init-constructor-signal"
7+
}

tests/run-tests.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,15 @@ assert_output_not_contains "no FP on async arrow function" "init/shell-modificat
231231
assert_output_not_contains "no FP on arrow GObject constructor" "init/shell-modification.*extension.js:14"
232232
echo ""
233233

234+
# --- init-constructor-signal ---
235+
echo "=== init-constructor-signal ==="
236+
run_lint "init-constructor-signal@test"
237+
assert_exit_code "exits with 1 (has constructor violation)" 1
238+
assert_output_contains "flags direct Shell global in constructor" "\[FAIL\].*init/shell-modification.*extension.js:10"
239+
assert_output_not_contains "no FP on .connect() callback body" "init/shell-modification.*extension.js:1[5-7]"
240+
assert_output_not_contains "no FP on .connectObject() callback body" "init/shell-modification.*extension.js:2[1-4]"
241+
echo ""
242+
234243
# --- lifecycle-imbalance ---
235244
echo "=== lifecycle-imbalance ==="
236245
run_lint "lifecycle-imbalance@test"

0 commit comments

Comments
 (0)