Merge branch 'main' into fix/r-slop-38-identifier-fps

Author: ZviBaratz

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

@@ -1318,10 +1318,10 @@ Rules for code that runs at module load or constructor time, before `enable()` i
 
 - **Severity**: blocking
 - **Checked by**: check-init.py
-- **Rule**: Must not create GObjects from any GI namespace (St, Clutter, Gio, GLib, GObject, Meta, Shell, Pango, Soup, Cogl, Atk, GdkPixbuf) at module scope or in `extension.js` constructors. GObject constructors in helper file constructors are not flagged (runtime-only, instantiated from `enable()`). Shell globals are flagged everywhere. **Exemptions**: `GObject.registerClass()` (class registration, not instantiation), arrow function definitions (lazy evaluation), value types (`GLib.Bytes`, `GLib.Variant`, `GLib.DateTime`, `GLib.TimeZone`, `GLib.Regex`, `GLib.Uri`, `Cogl.Color`, `Clutter.Color`).
+- **Rule**: Must not create GObjects from any GI namespace (St, Clutter, Gio, GLib, GObject, Meta, Shell, Pango, Soup, Cogl, Atk, GdkPixbuf) at module scope or in `extension.js` constructors. GObject constructors in helper class constructors are not flagged — whether in separate files or in `extension.js` alongside the Extension class (runtime-only, instantiated from `enable()`). Constructor checks in `extension.js` only apply to the Extension class (identified by `extends Extension` or `export default class`). Shell globals are flagged everywhere. **Exemptions**: `GObject.registerClass()` (class registration, not instantiation), arrow function definitions (lazy evaluation), value types (`GLib.Bytes`, `GLib.Variant`, `GLib.DateTime`, `GLib.TimeZone`, `GLib.Regex`, `GLib.Uri`, `Cogl.Color`, `Clutter.Color`).
 - **Rationale**: GObjects created before enable() cannot be properly cleaned up; #1 rejection cause
 - **Fix**: Move all GObject creation to enable()
-- **Tested by**: `tests/fixtures/init-modification@test/`, `tests/fixtures/init-helper-constructor@test/`, `tests/fixtures/constructor-lifecycle-expanded@test/`
+- **Tested by**: `tests/fixtures/init-modification@test/`, `tests/fixtures/init-helper-constructor@test/`, `tests/fixtures/constructor-lifecycle-expanded@test/`, `tests/fixtures/init-helper-in-extension@test/`
 
 #### Example
 

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

@@ -94,6 +94,17 @@ def is_skip_line(line):
     r'|Cogl\.Color|Clutter\.Color)\b'
 )
 
+# Extension class identification for scoping constructor checks.
+# Only the Extension class constructor runs at instantiation time —
+# helper class constructors only run when called from enable().
+EXTENSION_CLASS_DEF = re.compile(
+    r'class\s+\w+\s+extends\s+(?:\w+\.)*Extension\s*\{'
+)
+# Fallback: export default class without extends
+DEFAULT_EXPORT_CLASS_DEF = re.compile(
+    r'export\s+default\s+class\s+\w+\s*\{'
+)
+
 
 def extract_module_scope_lines(content_lines):
     """Extract lines that are at module scope (outside any class/function body).
@@ -114,12 +125,42 @@ def extract_module_scope_lines(content_lines):
     return module_lines
 
 
-def extract_constructor_lines(content_lines):
+def find_extension_class_range(content_lines):
+    """Find the line range of the Extension class body.
+
+    Returns (start_line, end_line) or None if not found.
+    Priority: extends Extension > export default class.
+    """
+    for lineno, line in enumerate(content_lines, 1):
+        if EXTENSION_CLASS_DEF.search(line) or \
+                DEFAULT_EXPORT_CLASS_DEF.search(line):
+            depth = line.count('{') - line.count('}')
+            if depth <= 0 and '{' in line:
+                # Single-line class body, e.g., `class Foo extends Extension { }` —
+                # unlikely but handled for completeness
+                return (lineno, lineno)
+            start = lineno
+            for end_lineno, end_line in enumerate(
+                    content_lines[lineno:], lineno + 1):
+                depth += end_line.count('{') - end_line.count('}')
+                if depth <= 0:
+                    return (start, end_lineno)
+            return (start, len(content_lines))
+    # No match found. This includes multiline class definitions where
+    # `class` and `{` are on different lines. Returning None causes all
+    # constructors to be checked (conservative — no false negatives).
+    return None
+
+
+def extract_constructor_lines(content_lines, class_range=None):
     """Extract lines inside constructor() method bodies.
 
     Returns a list of (original_lineno, line_text) tuples.
     Skips constructors inside GObject.registerClass() class bodies, since those
     only run when explicitly instantiated (not at module init time).
+
+    If class_range is provided as (start, end), only returns constructors
+    whose definition line falls within that range.
     """
     constructor_lines = []
     in_constructor = False
@@ -147,6 +188,9 @@ def extract_constructor_lines(content_lines):
         if not in_constructor and re.search(r'\bconstructor\s*\(', line):
             if in_register_class:
                 continue
+            # Only include constructors within the Extension class range
+            if class_range and not (class_range[0] <= lineno <= class_range[1]):
+                continue
             in_constructor = True
             open_braces = line.count('{')
             close_braces = line.count('}')
@@ -248,11 +292,12 @@ def check_init_modifications(ext_dir):
                     violations.append(f"{rel}:{lineno}")
 
         # Check constructor() lines
-        # Helper file constructors are runtime-only (only run when
-        # instantiated from enable()), so only flag in extension.js
+        # Helper class constructors are runtime-only (only run when
+        # instantiated from enable()), so only flag the Extension class
         is_extension_js = os.path.basename(filepath) == 'extension.js'
         if is_extension_js:
-            ctor_lines = extract_constructor_lines(lines)
+            ext_range = find_extension_class_range(lines)
+            ctor_lines = extract_constructor_lines(lines, class_range=ext_range)
             ctor_lines = filter_signal_callbacks(ctor_lines)
             for lineno, line in ctor_lines:
                 if is_skip_line(line):

tests/assertions/init-helper-in-extension.sh

@@ -0,0 +1,10 @@
+# Tests that helper class constructors in extension.js are not flagged
+# while Extension class constructors still are
+# Sourced by run-tests.sh
+
+echo "=== init-helper-in-extension ==="
+run_lint "init-helper-in-extension@test"
+assert_exit_code "exits with 1 (Extension constructor violation)" 1
+assert_output_contains "flags Extension constructor Shell global" "\[FAIL\].*init/shell-modification.*extension.js:20"
+assert_output_not_contains "no FP on helper class constructor" "init/shell-modification.*extension.js:8"
+echo ""

tests/fixtures/init-helper-in-extension@test/LICENSE

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

tests/fixtures/init-helper-in-extension@test/extension.js

@@ -0,0 +1,31 @@
+import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
+import * as Main from 'resource:///org/gnome/shell/ui/main.js';
+import Gio from 'gi://Gio';
+
+// Helper class — instantiated at runtime from enable()
+class Helper {
+    constructor() {
+        this._cancellable = new Gio.Cancellable();
+    }
+    destroy() {
+        this._cancellable.cancel();
+        this._cancellable = null;
+    }
+}
+
+export default class TestExtension extends Extension {
+    constructor(metadata) {
+        super(metadata);
+        // VIOLATION: Shell global in Extension constructor
+        this._hasOverview = Main.sessionMode.hasOverview;
+    }
+
+    enable() {
+        this._helper = new Helper();
+    }
+
+    disable() {
+        this._helper.destroy();
+        this._helper = null;
+    }
+}

tests/fixtures/init-helper-in-extension@test/metadata.json

@@ -0,0 +1,7 @@
+{
+    "uuid": "init-helper-in-extension@test",
+    "name": "Init Helper In Extension Test",
+    "description": "Tests that helper class constructors in extension.js are not flagged",
+    "shell-version": ["48"],
+    "url": "https://example.com"
+}