Skip to content

Commit 526a868

Browse files
ZviBaratzclaude
andcommitted
fix(ego-lint): resolve Dash to Panel false positives (-5 FP FAILs, -23 FP WARNs)
Five targeted fixes for false positives found during Dash to Panel field test: 1. R-SEC-03 guard for GPL license headers: `http://www.gnu.org/licenses/` in GPL boilerplate no longer fires (-18 FP WARNs) 2. R-VER48-02 PACKAGE_VERSION guard: `Config.PACKAGE_VERSION >= '48'` recognized as version-compat guard with guard-window: 3 (-2 FP FAILs) 3. skip-comments field for pattern rules: new opt-in that skips matches inside // and /* */ comments. Applied to R-DEPR-05/06 (-3 FP FAILs) 4. guard-window-forward for forward-looking guard: new field that checks N lines after the match. Applied to R-SLOP-24 for multi-line system schema constructors (-5 FP WARNs) 5. Provenance post-filter output fix: R-SLOP-01/02 WARNs deferred until provenance score is known, fixing visual bug where suppressed WARNs still appeared in terminal output Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2b79fce commit 526a868

File tree

19 files changed

+269
-20
lines changed

19 files changed

+269
-20
lines changed

CLAUDE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ bash skills/ego-lint/scripts/ego-lint.sh tests/fixtures/<fixture-name>
4444
`ego-lint.sh` is the main orchestrator. It uses a three-tier rule system (pattern → structural → semantic) and delegates to sub-scripts via `run_subscript`:
4545

4646
- `rules/patterns.yaml` — Tier 1 pattern rules (124 regex-based, declarative rules). Note: at project root, not under `skills/ego-lint/`
47-
- `apply-patterns.py` — Tier 1 pattern engine (inline YAML parser, no PyYAML dependency). Supports `guard-pattern` + `guard-window` (sliding `deque` lookback), `replacement-pattern`, `exclude-dirs`, version-gating, `fix-min-version` (suppresses fix text when extension min shell-version is below threshold)
47+
- `apply-patterns.py` — Tier 1 pattern engine (inline YAML parser, no PyYAML dependency). Supports `guard-pattern` + `guard-window` (sliding `deque` lookback) + `guard-window-forward` (forward peeking), `replacement-pattern`, `exclude-dirs`, version-gating, `fix-min-version` (suppresses fix text when extension min shell-version is below threshold), `skip-comments` (skips matches inside `//` and `/* */` comments)
4848
- `check-quality.py` — Tier 2 heuristic AI slop detection (try-catch density, impossible states, pendulum patterns, empty catches, _destroyed density, mock detection, constructor resources, run_dispose comment, clipboard disclosure, network disclosure, excessive null checks, repeated getSettings, obfuscated names, mixed indentation, excessive logging, code provenance)
4949
- `check-metadata.py` — JSON validity, required fields, UUID format/match, shell-version (with VALID_GNOME_VERSIONS allowlist), session-modes, settings-schema, version-name, donations, gettext-domain consistency
5050
- `check-init.py` — Init-time Shell modification, GObject constructor detection (extension.js only; all GI namespaces, GObject.registerClass exempt, GLib value types exempt), Gio._promisify placement
@@ -67,7 +67,7 @@ Additional tooling:
6767

6868
### Three-tier rule system
6969

70-
- **Tier 1 (patterns.yaml)**: 124 regex rules in YAML, processed by `apply-patterns.py`. Covers web APIs, deprecated APIs, security (telemetry, curl/gsettings spawn, base64), logging, import segregation, AI slop signals, subprocess safety, i18n, GSettings bind flags, GNOME 44-50 migration, code quality advisories. Add new rules by editing `rules/patterns.yaml`. Advanced fields: `min-version`/`max-version` (version-gating), `guard-pattern` + `guard-window` (line-level suppression with configurable lookback), `replacement-pattern` (file-level suppression), `exclude-dirs`.
70+
- **Tier 1 (patterns.yaml)**: 124 regex rules in YAML, processed by `apply-patterns.py`. Covers web APIs, deprecated APIs, security (telemetry, curl/gsettings spawn, base64), logging, import segregation, AI slop signals, subprocess safety, i18n, GSettings bind flags, GNOME 44-50 migration, code quality advisories. Add new rules by editing `rules/patterns.yaml`. Advanced fields: `min-version`/`max-version` (version-gating), `guard-pattern` + `guard-window` (line-level suppression with configurable lookback) + `guard-window-forward` (forward peeking), `replacement-pattern` (file-level suppression), `exclude-dirs`, `skip-comments` (comment-aware matching).
7171
- **Tier 2 (scripts)**: 17 structural heuristic check scripts in Python/bash. `check-quality.py` (AI slop heuristics), `check-init.py` (init-time safety), `check-lifecycle.py` (enable/disable symmetry + timeout verification), `check-resources.py` + `build-resource-graph.py` (cross-file resource tracking), `check-disclosures.py` (clipboard/network disclosure), `check-polkit.py` (polkit policy validation), `check-schema-usage.py` (unused/undefined schema keys), `check-accessibility.py` (a11y checks), plus metadata, prefs, GObject, async, CSS, imports, schema, and package checks. `ego-lint.sh` also has an inline minified JS check, code metrics, and a provenance-gated post-filter that suppresses R-SLOP-01/02 JSDoc warnings when `quality/code-provenance` score >= 4.
7272
- **Tier 3 (checklists)**: 6 semantic review checklists in `skills/ego-review/references/`: lifecycle, security, code-quality (with 10 additional quality items), ai-slop (46-item scoring model), licensing, accessibility (7 items). Applied by Claude during `ego-review` phases.
7373

rules/README.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ if (global.compositor) {
176176
- Only use when the default 1-line lookback is insufficient — most guards are on the same or previous line
177177
- Keep the window as small as practical to avoid false suppressions
178178

179+
### Forward Guard Lookback (`guard-window-forward`)
180+
181+
When the guard evidence appears _after_ the flagged pattern (e.g., in a multi-line constructor), use `guard-window-forward` to look ahead:
182+
183+
```yaml
184+
- id: R-SLOP-24
185+
pattern: "\\bnew\\s+Gio\\.Settings\\s*\\("
186+
guard-pattern: "schema.*org\\.gnome\\.(?!shell\\.extensions\\.)|KEYBINDINGS_SCHEMA"
187+
guard-window-forward: 2
188+
```
189+
190+
This checks the next 2 lines after the matched line for the guard pattern. The typical case is a multi-line constructor:
191+
192+
```js
193+
this._mutter = new Gio.Settings({ // ← pattern fires here
194+
schema_id: 'org.gnome.mutter' // ← guard matches here (forward +1)
195+
});
196+
```
197+
198+
**Guidelines:**
199+
- Set `guard-window-forward` to the maximum expected distance between the flagged pattern and the guard evidence
200+
- Can be combined with `guard-window` for bidirectional lookback
201+
- Keep the window as small as practical to avoid false suppressions
202+
203+
### Comment Skipping (`skip-comments`)
204+
205+
When a rule should not match inside comments (e.g., deprecated API names mentioned in code comments), use `skip-comments: true`:
206+
207+
```yaml
208+
- id: R-DEPR-06
209+
pattern: "\\bTweener\\b"
210+
skip-comments: true
211+
```
212+
213+
This skips matches inside `//` single-line comments and `/* */` block comments (including multi-line). Code on the same line as a comment is still checked — only the portion after `//` or between `/* */` is skipped.
214+
215+
**Use cases:**
216+
- Deprecated API names mentioned in migration comments
217+
- Commented-out legacy code that hasn't been removed
218+
179219
### Fix Version Gating (`fix-min-version`)
180220

181221
When a deprecation rule suggests a fix that only works on newer GNOME versions, use `fix-min-version` to suppress the fix text for extensions whose minimum shell-version is below the threshold. The warning still fires (developers should know about deprecations), but the fix suggestion is omitted when applying it would break backward compatibility.

rules/patterns.yaml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@
147147
message: "ExtensionUtils removed in GNOME 45+; use Extension/ExtensionPreferences class methods"
148148
category: deprecated
149149
fix: "Use this.getSettings(), this.gettext(), this.path, etc. from the Extension base class"
150+
skip-comments: true
150151

151152
- id: R-DEPR-06
152153
pattern: "\\bTweener\\b"
@@ -155,6 +156,7 @@
155156
message: "Tweener is removed; use Clutter animation framework"
156157
category: deprecated
157158
fix: "Use actor.ease({property: value, duration: ms, mode: Clutter.AnimationMode.EASE_OUT_QUAD})."
159+
skip-comments: true
158160

159161
- id: R-DEPR-07
160162
pattern: "imports\\.misc\\.convenience"
@@ -233,6 +235,7 @@
233235
message: "Use HTTPS instead of HTTP for network requests"
234236
category: security
235237
fix: "Change http:// to https://."
238+
guard-pattern: "gnu\\.org/licenses|General Public License"
236239

237240
- id: R-SEC-04
238241
pattern: "\\bsudo\\b"
@@ -671,7 +674,8 @@
671674
message: "new Gio.Settings() is incorrect in GNOME 45+; use this.getSettings() from Extension base class"
672675
category: ai-slop
673676
fix: "Use this.getSettings() in Extension subclass or this.getSettings() in ExtensionPreferences"
674-
guard-pattern: "schema.*org\\.gnome\\.(?!shell\\.extensions\\.)"
677+
guard-pattern: "schema.*org\\.gnome\\.(?!shell\\.extensions\\.)|KEYBINDINGS_SCHEMA"
678+
guard-window-forward: 2
675679
exclude-dirs: ["service"]
676680

677681
- id: R-SLOP-25
@@ -917,7 +921,8 @@
917921
category: version-compat
918922
fix: "Access via global.compositor instead of Meta directly"
919923
min-version: 48
920-
guard-pattern: "if\\s*\\(\\s*Meta\\.disable_unredirect"
924+
guard-pattern: "if\\s*\\(\\s*Meta\\.disable_unredirect|PACKAGE_VERSION.*['\"]4[89]|PACKAGE_VERSION.*['\"][5-9]"
925+
guard-window: 3
921926

922927
- id: R-VER48-03
923928
pattern: "\\bCursorTracker\\.get_for_display\\b"

skills/ego-lint/scripts/apply-patterns.py

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,10 @@ def main():
295295
guard_window = max(1, int(rule.get('guard-window', '1')))
296296
except (ValueError, TypeError):
297297
guard_window = 1
298+
try:
299+
guard_window_fwd = max(0, int(rule.get('guard-window-forward', '0')))
300+
except (ValueError, TypeError):
301+
guard_window_fwd = 0
298302

299303
for scope in scopes:
300304
# Expand glob relative to extension dir
@@ -327,22 +331,69 @@ def main():
327331
if replacement and replacement in file_content:
328332
continue
329333

334+
skip_comments = rule.get('skip-comments', '') == 'true'
335+
in_block_comment = False
336+
lines = file_content.splitlines(True)
330337
line_buffer = deque(maxlen=guard_window)
331-
for lineno, line in enumerate(file_content.splitlines(True), 1):
338+
for line_idx in range(len(lines)):
339+
line = lines[line_idx]
340+
lineno = line_idx + 1
341+
# Track block comment state for skip-comments
342+
if skip_comments:
343+
if in_block_comment:
344+
if '*/' in line:
345+
in_block_comment = False
346+
# Line may have code after */
347+
after_close = line[line.index('*/') + 2:]
348+
if '/*' in after_close:
349+
in_block_comment = True
350+
line_buffer.append(line)
351+
continue
352+
if '/*' in line:
353+
# Check if block comment closes on same line
354+
bc_start = line.index('/*')
355+
rest = line[bc_start + 2:]
356+
if '*/' in rest:
357+
# Single-line block comment — check if match is inside
358+
bc_end = bc_start + 2 + rest.index('*/')
359+
m = compiled.search(line)
360+
if m and bc_start <= m.start() < bc_end:
361+
line_buffer.append(line)
362+
continue
363+
else:
364+
# Multi-line block comment starts here
365+
m = compiled.search(line)
366+
if m and m.start() >= bc_start:
367+
in_block_comment = True
368+
line_buffer.append(line)
369+
continue
370+
in_block_comment = True
332371
if compiled.search(line):
372+
# Skip matches inside single-line comments
373+
if skip_comments and '//' in line:
374+
comment_pos = line.index('//')
375+
m = compiled.search(line)
376+
if m and m.start() >= comment_pos:
377+
line_buffer.append(line)
378+
continue
333379
# Check for inline suppression (prev_line = last item in buffer)
334380
prev_line = line_buffer[-1] if line_buffer else ''
335381
if _is_suppressed(line, prev_line, rid):
336382
line_buffer.append(line)
337383
continue
338-
# Check guard-pattern: if guard matches
339-
# current line or any line in the lookback
340-
# window, suppress (runtime feature detection)
384+
# Check guard-pattern: backward lookback window
341385
if guard_re and (guard_re.search(line) or
342386
any(guard_re.search(bl)
343387
for bl in line_buffer)):
344388
line_buffer.append(line)
345389
continue
390+
# Check guard-pattern: forward look window
391+
if guard_re and guard_window_fwd > 0:
392+
fwd_end = min(line_idx + guard_window_fwd + 1, len(lines))
393+
if any(guard_re.search(lines[j])
394+
for j in range(line_idx + 1, fwd_end)):
395+
line_buffer.append(line)
396+
continue
346397
rel = os.path.relpath(filepath, ext_dir)
347398
if deduplicate:
348399
dedup_files.add(rel)

skills/ego-lint/scripts/ego-lint.sh

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ FAIL_COUNT=0
7676
WARN_COUNT=0
7777
PASS_COUNT=0
7878
SKIP_COUNT=0
79+
DEFERRED_SLOP_JSDOC=() # R-SLOP-01/02 WARNs deferred until provenance score is known
7980

8081
# ---------------------------------------------------------------------------
8182
# Output helpers
@@ -153,7 +154,13 @@ run_pattern_rules() {
153154
check="${check%"${check##*[![:space:]]}"}"
154155
detail="${detail#"${detail%%[![:space:]]*}"}"
155156
detail="${detail%"${detail##*[![:space:]]}"}"
156-
print_result "$status" "$check" "$detail"
157+
# Defer R-SLOP-01/02 WARNs until provenance score is known
158+
if [[ "$status" == "WARN" && "$check" =~ ^R-SLOP-0[12]$ ]]; then
159+
DEFERRED_SLOP_JSDOC+=("${status}|${check}|${detail}")
160+
WARN_COUNT=$((WARN_COUNT + 1))
161+
else
162+
print_result "$status" "$check" "$detail"
163+
fi
157164
done <<< "$output"
158165
}
159166

@@ -570,17 +577,24 @@ if provenance_line=$(grep 'quality/code-provenance' "$RESULTS_FILE" 2>/dev/null)
570577
fi
571578
fi
572579

573-
if [[ "$provenance_score" -ge 4 ]]; then
574-
slop_jsdoc_count=$(grep -cE '^WARN\|R-SLOP-0[12]\|' "$RESULTS_FILE" 2>/dev/null || echo 0)
575-
if [[ "$slop_jsdoc_count" -gt 0 ]]; then
576-
tmp_results="$(mktemp)"
577-
grep -vE '^WARN\|R-SLOP-0[12]\|' "$RESULTS_FILE" > "$tmp_results"
578-
mv "$tmp_results" "$RESULTS_FILE"
579-
WARN_COUNT=$((WARN_COUNT - slop_jsdoc_count))
580-
PASS_COUNT=$((PASS_COUNT + 1))
581-
print_result "PASS" "provenance/jsdoc-suppressed" \
582-
"Suppressed $slop_jsdoc_count JSDoc warnings (R-SLOP-01/02) — provenance score $provenance_score indicates hand-written code"
583-
fi
580+
deferred_count=${#DEFERRED_SLOP_JSDOC[@]}
581+
if [[ "$provenance_score" -ge 4 && "$deferred_count" -gt 0 ]]; then
582+
# Suppress deferred R-SLOP-01/02 WARNs (high provenance = intentional JSDoc)
583+
WARN_COUNT=$((WARN_COUNT - deferred_count))
584+
PASS_COUNT=$((PASS_COUNT + 1))
585+
print_result "PASS" "provenance/jsdoc-suppressed" \
586+
"Suppressed $deferred_count JSDoc warnings (R-SLOP-01/02) — provenance score $provenance_score indicates hand-written code"
587+
else
588+
# Flush deferred entries (low provenance or no deferred entries)
589+
for entry in "${DEFERRED_SLOP_JSDOC[@]}"; do
590+
_df_status="${entry%%|*}"
591+
_df_rest="${entry#*|}"
592+
_df_check="${_df_rest%%|*}"
593+
_df_detail="${_df_rest#*|}"
594+
_df_display="${_df_detail%%|fix:*}"
595+
printf "[%-4s] %-38s %s\n" "$_df_status" "$_df_check" "$_df_display"
596+
echo "${entry}" >> "$RESULTS_FILE"
597+
done
584598
fi
585599

586600
# ---------------------------------------------------------------------------
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Dash to Panel field test — FP fixes
2+
# Sourced by run-tests.sh — uses run_lint, assert_output_contains, assert_exit_code, etc.
3+
4+
# --- gpl-license-header (R-SEC-03 guard for GPL boilerplate URLs) ---
5+
echo "=== gpl-license-header ==="
6+
run_lint "gpl-license-header@test"
7+
assert_exit_code "exits with 0 (advisory only)" 0
8+
assert_output_not_contains "R-SEC-03 suppressed for GPL license URL" "\[WARN\].*R-SEC-03.*gnu\.org"
9+
assert_output_contains "R-SEC-03 still fires on real HTTP URL" "\[WARN\].*R-SEC-03"
10+
echo ""
11+
12+
# --- version-guard-pkgver (R-VER48-02 guard for PACKAGE_VERSION) ---
13+
echo "=== version-guard-pkgver ==="
14+
run_lint "version-guard-pkgver@test"
15+
assert_exit_code "exits with 0 (version guard suppresses R-VER48-02)" 0
16+
assert_output_not_contains "no FAIL for R-VER48-02 with PACKAGE_VERSION guard" "\[FAIL\].*R-VER48-02"
17+
echo ""
18+
19+
# --- deprecated-in-comments (skip-comments for R-DEPR-05/06) ---
20+
echo "=== deprecated-in-comments ==="
21+
run_lint "deprecated-in-comments@test"
22+
assert_exit_code "exits with 0 (comments skipped)" 0
23+
assert_output_not_contains "R-DEPR-06 not fired in comment" "\[FAIL\].*R-DEPR-06"
24+
assert_output_not_contains "R-DEPR-05 not fired in comment" "\[FAIL\].*R-DEPR-05"
25+
echo ""
26+
27+
# --- system-gsettings-multiline (R-SLOP-24 guard-window-forward) ---
28+
echo "=== system-gsettings-multiline ==="
29+
run_lint "system-gsettings-multiline@test"
30+
assert_output_not_contains "R-SLOP-24 suppressed for multiline system schema" "\[WARN\].*R-SLOP-24"
31+
echo ""

tests/assertions/provenance-jsdoc.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ run_lint "high-provenance-jsdoc@test"
77
assert_exit_code "exits with 0 (no failures)" 0
88
assert_output_contains "provenance suppresses JSDoc" "provenance/jsdoc-suppressed"
99
assert_output_contains "provenance score 4+" "provenance-score=[4-9]"
10+
assert_output_not_contains "JSDoc WARNs suppressed from output" "\[WARN\].*R-SLOP-0[12]"
1011
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: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
2+
3+
// The original animations used Tweener instead of Clutter animations
4+
// this._settings = ExtensionUtils.getSettings('org.test.extension');
5+
/* ExtensionUtils was the old way */
6+
7+
export default class CommentTestExtension extends Extension {
8+
enable() {}
9+
disable() {}
10+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"uuid": "deprecated-in-comments@test",
3+
"name": "Deprecated in Comments Test",
4+
"description": "Tests skip-comments for R-DEPR-05/06",
5+
"shell-version": ["48"],
6+
"url": "https://example.com"
7+
}

0 commit comments

Comments
 (0)