From a005e4840598fc1198c01da62b59202ec28ff367 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 27 Jul 2026 00:32:42 +0200 Subject: [PATCH] =?UTF-8?q?feat(config):=20parse=20sweep=20=E2=80=94=20ver?= =?UTF-8?q?ify=20every=20project=20script=20parses,=20not=20just=20the=20s?= =?UTF-8?q?tartup=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit godot-cold-parse only ever sees scripts on the STARTUP path: autoloads and the main scene chain. That is the correct scope for the job it was built for (Sprint 36's `Could not find base class "MetaScreen"`, a registration-ORDER bug), but it is far narrower than the name suggests, and most of the codebase is invisible to it. Verified by deliberately breaking a non-startup UI script and a test file in turn: cold-parse reported "clean", exit 0, for both. That is the second half of today's false green. A parse error in test_step_canvas_annotation_layer.gd survived cold-parse AND survived gdUnit4, which reports the suites that DID load as a clean pass. Two gates, one blind spot: neither verified that a file it never opened was openable. godot-parse-sweep opens every .gd in the project (226 today, addons and .godot excluded) and fails on any that will not parse. The split between the two halves is forced, not stylistic. No Godot API reports GDScript parse failure reliably: - ResourceLoader.load(path, "GDScript", CACHE_MODE_IGNORE) SEGFAULTS the engine on a script that fails to parse — it dies on exactly the input the tool exists to find. - GDScript.new() + source_code + reload() returns a clean error code but detaches the script from its resource_path, so class_name, preload() and relative extends stop resolving: it reported 150 of 226 healthy scripts as broken. - Plain ResourceLoader.load() neither crashes nor false-positives, but returns a NON-null object for a broken script, so its return value is useless. The engine's own stderr is the only honest signal. So the GDScript half just opens files and makes no verdict; the wrapper scrapes the diagnosis. The wrapper also refuses to pass unless the sweep reported completion, so a future break in the walk cannot itself become a false green. Unlike cold-parse, "Cannot infer the type" is NOT filtered. That filter is precisely why cold-parse stayed silent about the file below. First run found a real one: client/tests/util/scene_helper.gd has not parsed since 2026-02-25 — five months — because `func(a := null, ...)` cannot infer a type from null. Fixed with explicit `: Variant` params. Blast radius is zero (the helper has no importers, so nothing else was taken out with it), but it went unseen by two gates for five months, which is the point. Full suite green at 3660. Pair session with Jeroen, 2026-07-27. Co-Authored-By: Claude --- client/tests/util/scene_helper.gd | 9 +++- client/tools/parse_sweep.gd | 89 +++++++++++++++++++++++++++++++ tooling/godot-parse-sweep | 64 ++++++++++++++++++++++ 3 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 client/tools/parse_sweep.gd create mode 100755 tooling/godot-parse-sweep diff --git a/client/tests/util/scene_helper.gd b/client/tests/util/scene_helper.gd index e0c6a9bb5..8b904e360 100644 --- a/client/tests/util/scene_helper.gd +++ b/client/tests/util/scene_helper.gd @@ -73,8 +73,13 @@ func monitor_signal(node: Node, signal_name: String) -> void: _signal_hits[key] = 0 # Lambda accepts up to 4 positional args to tolerate signals with up to 4 params. # GDScript default-param lambdas handle being called with fewer args correctly. - node.connect(signal_name, func(a := null, b := null, c := null, d := null): - _signal_hits[key] = _signal_hits.get(key, 0) + 1 + # The params MUST be explicitly `: Variant` — `a := null` cannot infer a type + # from null and is a hard parse error, which silently took this whole file + # (and every suite importing it) out of the run until the parse sweep found it. + node.connect( + signal_name, + func(a: Variant = null, b: Variant = null, c: Variant = null, d: Variant = null): + _signal_hits[key] = _signal_hits.get(key, 0) + 1 ) diff --git a/client/tools/parse_sweep.gd b/client/tools/parse_sweep.gd new file mode 100644 index 000000000..ef5d1f0b9 --- /dev/null +++ b/client/tools/parse_sweep.gd @@ -0,0 +1,89 @@ +extends SceneTree + +## Parse-sweep: open EVERY project .gd so the engine reports the broken ones. +## +## Why this exists. `tooling/godot-cold-parse` only ever sees scripts on the +## STARTUP path — autoloads and the main scene chain. That is the correct +## scope for the job it was built for (Sprint 36's `Could not find base class +## "MetaScreen"`, an autoload-vs-class_name registration ORDER bug), but it is +## far narrower than its name suggests, and most of the codebase is invisible +## to it. Verified 2026-07-27 by deliberately breaking a non-startup UI script +## and a test file in turn: cold-parse reported `clean`, exit 0, for both. +## +## That gap cost real time. A parse error in `test_step_canvas_annotation_layer.gd` +## went unnoticed for hours because it passed cold-parse AND because gdUnit4 +## reports the suites that DID load as a clean pass (fixed separately by +## `tests/run-godot`'s harness guards). Two gates, one blind spot: neither +## verified that a file it never opened was openable. +## +## **This script deliberately makes no verdict of its own.** It opens every +## script and lets Godot's own front-end print the diagnosis; the wrapper +## (`tooling/godot-parse-sweep`) greps for that. Two rejected alternatives, +## both found the hard way while building this: +## +## - `ResourceLoader.load(path, "GDScript", CACHE_MODE_IGNORE)` **segfaults +## the engine** (signal 11, Godot 4.6) on a script that fails to parse — +## i.e. it dies on exactly the input this tool exists to find. +## - `GDScript.new()` + `source_code` + `reload()` returns a clean error code, +## but detaches the script from its `resource_path`, so `class_name`, +## `preload()` and relative `extends` stop resolving: it reported 150 of +## 226 healthy scripts as broken. +## +## Plain `ResourceLoader.load()` neither crashes nor lies — but it returns a +## NON-null object for a broken script, so the return value cannot be trusted +## either. The engine's stderr is the only honest signal, which is why the +## verdict lives in the wrapper. +## +## Run via `tooling/godot-parse-sweep` (never directly — this half cannot fail). + +## Vendor and generated trees are not ours to verify. `addons/` is gdUnit4, +## whose own test corpus deliberately contains broken fixtures — sweeping it +## would report failures that are the vendor's intended state. +const SKIP_PREFIXES: PackedStringArray = ["res://addons/", "res://.godot/"] + +var _checked: int = 0 +var _unreadable_dirs: int = 0 + + +func _initialize() -> void: + var scripts: PackedStringArray = [] + _collect("res://", scripts) + scripts.sort() # deterministic ordering, so output diffs cleanly run to run + + for path in scripts: + _checked += 1 + # Return value deliberately discarded — see the header. Touching the + # file is the whole job; the engine prints the diagnosis. + ResourceLoader.load(path) + + # stdout only. A non-zero verdict is the wrapper's to make, from stderr. + print("parse-sweep: opened %d scripts (%d unreadable dirs)" % [_checked, _unreadable_dirs]) + quit(0) + + +## Recursive walk of the VIRTUAL filesystem. DirAccess rather than a shell +## `find`, because `res://` is what the engine can actually open — a source-tree +## walk would include files the engine ignores and miss the distinction. +func _collect(dir_path: String, out: PackedStringArray) -> void: + for prefix in SKIP_PREFIXES: + if dir_path.begins_with(prefix): + return + + var dir := DirAccess.open(dir_path) + if dir == null: + _unreadable_dirs += 1 + return + + dir.list_dir_begin() + var name := dir.get_next() + while name != "": + if name.begins_with("."): + name = dir.get_next() + continue + var full := dir_path.path_join(name) if dir_path != "res://" else "res://" + name + if dir.current_is_dir(): + _collect(full, out) + elif name.ends_with(".gd"): + out.append(full) + name = dir.get_next() + dir.list_dir_end() diff --git a/tooling/godot-parse-sweep b/tooling/godot-parse-sweep new file mode 100755 index 000000000..ee50c275c --- /dev/null +++ b/tooling/godot-parse-sweep @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# tooling/godot-parse-sweep — open every project .gd and fail on any that won't parse. +# +# Complements `godot-cold-parse`, which does NOT cover this. That script only +# ever sees scripts on the STARTUP path (autoloads + the main scene chain) — +# correct for the registration-ORDER bug it was built for, far narrower than +# its name implies. Verified 2026-07-27 by breaking a non-startup UI script +# and a test file in turn: cold-parse reported "clean", exit 0, for both. +# +# Division of labour, deliberate: +# godot-cold-parse — cold-cache STARTUP ordering (class_name/autoload race) +# godot-parse-sweep — does every file in the project parse at all +# +# The GDScript half (client/tools/parse_sweep.gd) only opens files; it makes no +# verdict, because no Godot API reports GDScript parse failure reliably (see +# that file's header — one segfaults, one false-positives 150/226, and plain +# load() returns non-null for a broken script). The engine's own stderr is the +# only honest signal, so the verdict is made here by scraping it. +# +# Exit 0 + "clean" if every script parsed. Exit 1 + the offending lines if not. +set -euo pipefail + +REPO_ROOT="$(git rev-parse --show-toplevel)" + +set +e +RAW=$(godot --headless --path "$REPO_ROOT/client" -s res://tools/parse_sweep.gd 2>&1) +GODOT_EXIT=$? +set -e + +if [ "$GODOT_EXIT" -ne 0 ]; then + echo "godot-parse-sweep: godot itself exited $GODOT_EXIT — not a parse verdict" >&2 + printf '%s\n' "$RAW" | tail -20 >&2 + exit "$GODOT_EXIT" +fi + +# The sweep must actually have run. Without this, a future change that breaks +# the walk (or renames the script) would produce zero error lines and read as +# a clean sweep — the same false-green shape this tool exists to close. +if ! printf '%s\n' "$RAW" | grep -q '^parse-sweep: opened'; then + echo "godot-parse-sweep: the sweep did not report completion — no verdict possible" >&2 + printf '%s\n' "$RAW" | tail -20 >&2 + exit 1 +fi +SUMMARY=$(printf '%s\n' "$RAW" | grep '^parse-sweep: opened') + +# NOTE: unlike godot-cold-parse, "Cannot infer the type" is NOT filtered here. +# That filter is why cold-parse stayed silent about tests/util/scene_helper.gd, +# which genuinely does not parse — the suppressed class was hiding a real +# failure, not noise. +MATCHES=$(printf '%s\n' "$RAW" \ + | grep -E 'Parse Error|Failed to load script' \ + | grep -v "Failed loading resource: res://assets" || true) + +if [ -n "$MATCHES" ]; then + echo "PARSE SWEEP FAILED — at least one script does not parse." >&2 + printf '%s\n' "$MATCHES" | head -40 >&2 + echo "" >&2 + echo " ${SUMMARY}" >&2 + echo " An unparseable file cannot run. If it is a test suite, it did not" >&2 + echo " execute, and any pass count reported elsewhere excludes it." >&2 + exit 1 +fi + +echo "godot-parse-sweep: clean — ${SUMMARY#parse-sweep: }"