Skip to content

Fix formatter bugs and consolidate snapshot tests#1012

Open
DaelonSuzuka wants to merge 11 commits into
godotengine:masterfrom
DaelonSuzuka:fix/formatter-issues
Open

Fix formatter bugs and consolidate snapshot tests#1012
DaelonSuzuka wants to merge 11 commits into
godotengine:masterfrom
DaelonSuzuka:fix/formatter-issues

Conversation

@DaelonSuzuka
Copy link
Copy Markdown
Collaborator

@DaelonSuzuka DaelonSuzuka commented May 5, 2026

Summary

Grammar fixes (#858, #1002)

  • match_keyword and statement_keyword patterns now use (?:^|:) instead of ^\s*, so match is recognized after :
  • function_call begin pattern now allows \s* between name and (, so func_name (args) scopes as a function call

Formatter fixes (#858, #865, #928, #938, #850, yield, signal)

  • between() in textmate.ts now checks for = before -/+ tokens, treating them as unary when preceded by assignment
  • Added yield to no-space-before-paren list (alongside func, assert, export)
  • Made signal tokenization scope-aware: keyword in declarations, variable in expressions
  • is_merge_conflict_marker() skips lines starting with <<<<<<<, =======, or >>>>>>> entirely, preventing the formatter from corrupting git conflict markers (Autoformatter Incorrectly Formats Merge Conflicts #850)
  • inLambdaBody flag on tokens tracks when we're inside a lambda body that's a function call argument — dense parameter formatting is suppressed for lambda body content so assignments keep their spaces (Formatting issue of lambda functions #938)
  • Scientific notation 1e-6 is correctly tokenized as a single constant.numeric.float.gdscript token by the grammar — verified, was already fixed (Formatter adds space inside float literal with negative exponent. #928)

Setting enhancement (#1004)

  • maxEmptyLines changed from enum ["1","2"] to number with minimum: 0, default 2
  • Config value is sanitized: negatives clamped to 0, non-numbers default to 2

Test infrastructure

  • All 17 in.gd/out.gd directory pairs converted to single inline .gd files
  • Removed the pair-test runner and get_options() helper from formatter.test.ts
  • Renamed test.gdtrailing_newlines.gd, merged scientific_notation.gdfloat_notation.gd, merged self_keyword_spacing.gdkeywords.gd

New test files for previously uncovered features

  • annotations.gd@export, @onready, @rpc, @warning_ignore, @abstract, stacked annotations
  • control_flow.gdif/elif/else, for..in, while, ternary if, pass/break/continue
  • declarations.gd — return types, class_name, extends, signal, static, const
  • dense_params.gddenseFunctionParameters config option, lambda body spacing (Formatting issue of lambda functions #938)
  • dictionaries.gd — dict literals, nested/typed dicts, subscript access
  • godot3_compat.gdtool, export, onready, remote/master/puppet keywords, setget syntax, yield()
  • is_as_operator.gdis/is not and as operators
  • merge_conflict.gd — git conflict markers left untouched (Autoformatter Incorrectly Formats Merge Conflicts #850)
  • method_calls.gd — function call spacing, method chains
  • trailing_newlines.gdstrictTrailingNewlines config

Test plan

  • All 56+ formatter snapshot test cases pass
  • Verify syntax highlighting for nested match and spaced function calls looks correct in VS Code
  • Verify maxEmptyLines accepts any non-negative integer in settings
  • Verify merge conflict markers are not corrupted by formatting
  • Verify lambda body spacing is preserved with denseFunctionParameters: true

Add non-null assertions for RegExpMatchArray.index and use type
predicates in filter() calls to satisfy strictNullChecks.
When '-' followed an '=' token, the formatter treated it as a binary
subtraction and added a space before the next identifier. Now checks
for '=' before the operator to correctly identify unary negation.

Fixes godotengine#865.
Tests for godotengine#889 (negative indices), godotengine#976 (self keyword spacing),
and godotengine#984 (@abstract + is). These bugs are already fixed in current
code — tests prevent regressions.
Replace in.gd/out.gd directory pairs with single-file inline format
using # --- IN --- / # --- OUT --- / # --- END --- markers. This
makes tests easier to read and edit, and consistent with existing
inline snapshot tests.
All snapshot tests are now in the inline single-file format. Remove
the directory-based pair test runner, the get_options() helper, and
the in.gd/out.gd file filter that are no longer needed.
Change maxEmptyLines from a dropdown of 1/2 to a numeric input
allowing any non-negative integer. Sanitize configured values to
clamp negatives to 0 and round to integers, defaulting to 2.
…ces (godotengine#858, godotengine#1002)

- match_keyword pattern: change from ^\s* anchor to (?:^|:) so 'match'
  is recognized after ':', fixing 'if true: match x:' being tokenized as
  'matchx' by the formatter
- function_call begin pattern: add \s* between name and \( so
  'func_name (args)' is scoped as a function call, not a variable
  followed by a grouped expression
- statement_keyword: same (?:^|:) change for case/match consistency
- Add nested_match.gd snapshot test covering the godotengine#858 fix
New test files for previously uncovered features:
- annotations.gd: @export, @onready, @rpc, @warning_ignore, @abstract
- control_flow.gd: if/elif/else, for..in, while, ternary if, pass/break/continue
- declarations.gd: return types, class_name, extends, signal, static, const
- dense_params.gd: denseFunctionParameters config option
- dictionaries.gd: dict literals, nested/typed dicts, subscript access
- is_as_operator.gd: is/is not and as operators
- method_calls.gd: function call spacing, method chains
- trailing_newlines.gd: strictTrailingNewlines config (renamed from test.gd)

Reorganized existing tests:
- Merge scientific_notation.gd into float_notation.gd
- Merge self_keyword_spacing.gd into keywords.gd (expanded with super, const, class_name, signal, self)
- Expand match.gd with more pattern types (string, array, dict, wildcard, comma-separated)
- Rename test.gd to trailing_newlines.gd

Discovered formatter gaps (marked as idempotency tests for now):
- class_name/extends ARE now formatted (was unknown)
- signal with args IS formatted (was unknown)
- static func/var IS formatted (was unknown)
- const inside functions IS formatted (was unknown)
- denseFunctionParameters does not affect := spacing
- Add yield to no-space-before-paren list (alongside func, assert, export)
  so yield(expr) stays compact instead of becoming yield (expr)
- Make signal tokenization scope-aware: keyword in declarations
  (signal my_signal) but variable in expression context (yield(signal, ...))
  so it doesn't get extra keyword spacing inside function calls
- Add godot3_compat.gd test covering: tool, export, onready, remote/
  master/puppet sync keywords, setget syntax, and yield()
The biome linter on CI (newer version) treats noNonNullAssertion as an
error. Replace match.index! assertions with early-continue guards that
handle the undefined case explicitly.
Add 'npm run test:headless' that runs tests under xvfb-run with
WAYLAND_DISPLAY unset. Add --ozone-platform=x11 to test launchArgs
so VS Code's Electron uses X11 instead of crashing on Wayland systems.

This prevents the test runner from stealing window focus on Linux
desktops running Wayland.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment