Add nullable static types to GDScript. - #1335
Open
mcdubhghlas wants to merge 8 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughNullable type syntax, flow-sensitive non-null narrowing, nullable-aware bytecode execution, unsafe-access warnings, and analyzer and runtime tests are added across GDScript. ChangesNullable GDScript support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GDScriptParser
participant GDScriptAnalyzer
participant GDScriptCompiler
participant GDScriptVM
GDScriptParser->>GDScriptAnalyzer: provide nullable type information
GDScriptAnalyzer->>GDScriptAnalyzer: narrow identifiers and validate nullable access
GDScriptAnalyzer->>GDScriptCompiler: provide checked nullable types
GDScriptCompiler->>GDScriptVM: emit nullable assignment and return metadata
GDScriptVM->>GDScriptVM: accept NIL for nullable targets or enforce validation
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/gdscript/gdscript_analyzer.cpp`:
- Around line 970-975: Update the nested-enum resolution paths for
Variant.SomeEnum and built-in nested enums so they pass through the existing
nullable finalization block guarded by p_type->is_nullable, allowing annotations
such as Vector2.Axis? to set result.is_nullable. Preserve the current Variant
rejection behavior, and add a regression test covering nullable nested enums.
- Around line 2090-2098: Update the IF handling in the non-null narrowing logic
to use the absence of an exiting false branch rather than requiring false_block
to be null. Replace the false_block-null condition in the true_exits branch with
the corresponding !false_exits check, preserving the existing symmetric branch
and retaining the non-null fact after an else whose path does not exit.
In `@modules/gdscript/gdscript_function.h`:
- Around line 85-87: Complete nullable handling across all listed sites: in
modules/gdscript/gdscript_function.h:85-87, accept nullable Variant::NIL before
switching on GDScriptDataType::kind; in
modules/gdscript/gdscript_analyzer.cpp:4971-4975 and :5098-5102, emit
nullable-access warnings for non-meta object/class bases in attribute and index
access; in :6386-6398, reject nullable sources assigned to non-nullable targets
before object polymorphism checks while preserving nil acceptance for nullable
targets. Add regressions covering Node? and script-class assignment,
parameter/return transfers, null values, and member access.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eb70ba44-2981-44d8-bd62-91969edb0ad2
⛔ Files ignored due to path filters (40)
modules/gdscript/tests/scripts/analyzer/errors/nullable_array_element.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/errors/nullable_dictionary_key_element.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/errors/nullable_dictionary_value_element.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/errors/nullable_variant.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_and_false_no_narrow.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_break_guard_no_narrow.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_compound_assignment.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_continue_guard_no_narrow.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_narrowing_limits.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_or_true_no_narrow.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_unsafe_operations.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/nullable_unsafe_transfer.outis excluded by!**/*.outmodules/gdscript/tests/scripts/analyzer/warnings/unsafe_nullable_access.outis excluded by!**/*.outmodules/gdscript/tests/scripts/parser/errors/nullable_void_return.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_enum_null_transfer.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_null_reaches_argument.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_null_reaches_typed_local.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_null_use.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_return_null_from_nonnull.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_array_assign_mismatch.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_array_reassign_mismatch.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_dictionary_assign_mismatch.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_dictionary_reassign_mismatch.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_dictionary_return_mismatch.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_return_mismatch.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_empty_callable.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_enum_raw_integer.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_enum_transfer.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_freed_object.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_multivar_and.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_multivar_or_guard.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_narrowing.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_narrowing_mutation.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_not_compound_or.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_renarrow_after_reassign.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_self_reference_assignment.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_typed_container_returns.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_types.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_variant_null_return.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_while_body_mutation.outis excluded by!**/*.out
📒 Files selected for processing (51)
modules/gdscript/gdscript_analyzer.cppmodules/gdscript/gdscript_analyzer.hmodules/gdscript/gdscript_byte_codegen.cppmodules/gdscript/gdscript_compiler.cppmodules/gdscript/gdscript_disassembler.cppmodules/gdscript/gdscript_function.hmodules/gdscript/gdscript_parser.cppmodules/gdscript/gdscript_parser.hmodules/gdscript/gdscript_vm.cppmodules/gdscript/gdscript_warning.cppmodules/gdscript/gdscript_warning.hmodules/gdscript/tests/scripts/analyzer/errors/nullable_array_element.gdmodules/gdscript/tests/scripts/analyzer/errors/nullable_dictionary_key_element.gdmodules/gdscript/tests/scripts/analyzer/errors/nullable_dictionary_value_element.gdmodules/gdscript/tests/scripts/analyzer/errors/nullable_variant.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_and_false_no_narrow.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_break_guard_no_narrow.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_compound_assignment.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_continue_guard_no_narrow.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_narrowing_limits.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_or_true_no_narrow.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_unsafe_operations.gdmodules/gdscript/tests/scripts/analyzer/warnings/nullable_unsafe_transfer.gdmodules/gdscript/tests/scripts/analyzer/warnings/unsafe_nullable_access.gdmodules/gdscript/tests/scripts/parser/errors/nullable_void_return.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_enum_null_transfer.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_null_reaches_argument.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_null_reaches_typed_local.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_null_use.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_return_null_from_nonnull.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_array_assign_mismatch.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_array_reassign_mismatch.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_dictionary_assign_mismatch.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_dictionary_reassign_mismatch.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_dictionary_return_mismatch.gdmodules/gdscript/tests/scripts/runtime/errors/nullable_typed_return_mismatch.gdmodules/gdscript/tests/scripts/runtime/features/nullable_empty_callable.gdmodules/gdscript/tests/scripts/runtime/features/nullable_enum_raw_integer.gdmodules/gdscript/tests/scripts/runtime/features/nullable_enum_transfer.gdmodules/gdscript/tests/scripts/runtime/features/nullable_freed_object.gdmodules/gdscript/tests/scripts/runtime/features/nullable_multivar_and.gdmodules/gdscript/tests/scripts/runtime/features/nullable_multivar_or_guard.gdmodules/gdscript/tests/scripts/runtime/features/nullable_narrowing.gdmodules/gdscript/tests/scripts/runtime/features/nullable_narrowing_mutation.gdmodules/gdscript/tests/scripts/runtime/features/nullable_not_compound_or.gdmodules/gdscript/tests/scripts/runtime/features/nullable_renarrow_after_reassign.gdmodules/gdscript/tests/scripts/runtime/features/nullable_self_reference_assignment.gdmodules/gdscript/tests/scripts/runtime/features/nullable_typed_container_returns.gdmodules/gdscript/tests/scripts/runtime/features/nullable_types.gdmodules/gdscript/tests/scripts/runtime/features/nullable_variant_null_return.gdmodules/gdscript/tests/scripts/runtime/features/nullable_while_body_mutation.gd
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/gdscript/gdscript_analyzer.cpp (1)
2079-2103: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the complete narrowing state, not only its size.
These scopes snapshot
narrowed_non_nullby length and later callresize(), butinvalidate_narrowing()can erase facts that existed before the snapshot. Resizing cannot restore those removed entries, so a nested assignment or boolean expression can permanently lose an outer non-null fact and produce falseUNSAFE_NULLABLE_ACCESSwarnings afterward. Save and restore the vector contents (or use an equivalent scoped snapshot), and add a regression covering an outer null check with a nested branch that reassigns the narrowed variable.Also applies to: 2284-2392, 3015-3029, 3254-3260
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/gdscript/gdscript_analyzer.cpp` around lines 2079 - 2103, Restore the full narrowed_non_null contents after each scoped narrowing operation, rather than only resizing to the saved length. Update the snapshot/cleanup logic around the shown suite loop and the corresponding scopes near the other referenced locations, preserving facts removed by invalidate_narrowing() during nested branches or expressions. Add a regression covering an outer null check whose nested branch reassigns the narrowed variable, ensuring the outer non-null fact remains available afterward.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@modules/gdscript/gdscript_analyzer.cpp`:
- Around line 2079-2103: Restore the full narrowed_non_null contents after each
scoped narrowing operation, rather than only resizing to the saved length.
Update the snapshot/cleanup logic around the shown suite loop and the
corresponding scopes near the other referenced locations, preserving facts
removed by invalidate_narrowing() during nested branches or expressions. Add a
regression covering an outer null check whose nested branch reassigns the
narrowed variable, ensuring the outer non-null fact remains available afterward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a888371c-2741-4562-86b0-9680d6e02ded
⛔ Files ignored due to path filters (2)
modules/gdscript/tests/scripts/runtime/features/nullable_narrowing_else.outis excluded by!**/*.outmodules/gdscript/tests/scripts/runtime/features/nullable_nested_enum.outis excluded by!**/*.out
📒 Files selected for processing (4)
doc/classes/ProjectSettings.xmlmodules/gdscript/gdscript_analyzer.cppmodules/gdscript/tests/scripts/runtime/features/nullable_narrowing_else.gdmodules/gdscript/tests/scripts/runtime/features/nullable_nested_enum.gd
Member
|
LGTM |
JoltedJon
reviewed
Aug 6, 2026
JoltedJon
previously approved these changes
Aug 7, 2026
JoltedJon
left a comment
JoltedJon
left a comment
Contributor
There was a problem hiding this comment.
From a code review standpoint it looks good.
Next steps is you should write some basic documentation somewhere for how to use it and what types are compatible with it
ghost
reviewed
Aug 8, 2026
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Overall LGTM but needs docs on usage & such
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/gdscript/doc_classes/GDScript.xml`:
- Line 10: Update the “Nullable static types” documentation to limit the default
non-null guarantee to types with strict null enforcement, rather than claiming
every statically typed value rejects null. Explicitly preserve the behavior that
object reference types such as Node may hold null without the ? suffix, while
retaining the existing guidance for nullable suffixes, untyped values, and
Variant.
- Around line 27-28: Update the nullable warning description in the GDScript
documentation to exclude ==, !=, and, and or from the “in an operator” wording,
since these operators are not analyzed. Clarify the non-nullability statement so
it applies only to statically typed non-object values, while object references
may still hold null without the ? suffix; preserve the remaining warning paths
and behavior descriptions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ad56d023-f16b-437a-9526-501ad34084d8
📒 Files selected for processing (1)
modules/gdscript/doc_classes/GDScript.xml
Comment on lines
+27
to
+28
| Using a nullable value of a [b]non-object built-in type[/b] (such as [code]int?[/code], [code]Array?[/code], or [code]Dictionary?[/code]) directly (in an operator, subscript, property access, or [code]for[/code] loop), or transferring [i]any[/i] nullable value to a non-nullable target (assigning, returning, or passing it as an argument), raises the [code]UNSAFE_NULLABLE_ACCESS[/code] warning. Set its severity with [member ProjectSettings.debug/gdscript/warnings/unsafe_nullable_access], or silence a single line with [annotation @GDScript.@warning_ignore]. | ||
| Enforcement depends on the type. For non-object built-in types and enums, a [code]null[/code] reaching a non-nullable value raises a runtime error. Object references keep GDScript's existing behavior — they may hold [code]null[/code] whether or not the type uses [code]?[/code] — so on object types [code]?[/code] only enables narrowing and the transfer warning. Accessing a member or calling a method directly on a nullable object or enum is not flagged, but it still fails at runtime if the value is actually [code]null[/code], so narrow with a [code]null[/code] check first. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\bUNSAFE_NULLABLE_ACCESS\b' \
modules/gdscript/gdscript_analyzer.cpp \
modules/gdscript/gdscript_warning.h \
modules/gdscript/testsRepository: Redot-Engine/redot-engine
Length of output: 26112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- documentation ---'
sed -n '1,40p' modules/gdscript/doc_classes/GDScript.xml
printf '%s\n' '--- nullable-warning call sites ---'
rg -n -C 12 'UNSAFE_NULLABLE_ACCESS|is_nullable' \
modules/gdscript/gdscript_analyzer.cpp \
modules/gdscript/gdscript_parser.cpp \
modules/gdscript/tests/scripts/analyzer \
modules/gdscript/tests/scripts/runtime/errors
printf '%s\n' '--- nullable test sources and expected output names ---'
rg -l 'nullable|UNSAFE_NULLABLE_ACCESS' modules/gdscript/tests/scripts \
| sort | head -200Repository: Redot-Engine/redot-engine
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- analyzer methods near nullable access paths ---'
rg -n '^void GDScriptAnalyzer::|^GDScriptParser::DataType GDScriptAnalyzer::' \
modules/gdscript/gdscript_analyzer.cpp \
| awk -F: '$2 >= 4800 && $2 <= 6250'
printf '%s\n' '--- direct nullable test files ---'
rg -l -i 'nullable.*(member|method|property|enum)|((member|method|property|enum).*)nullable' \
modules/gdscript/tests/scripts modules/gdscript/tests \
| sort -u
printf '%s\n' '--- warning and runtime expectations for nullable tests ---'
rg -n -i -C 2 'UNSAFE_NULLABLE_ACCESS|nullable.*(member|method|property|enum)|((member|method|property|enum).*)nullable' \
modules/gdscript/tests/scripts \
| head -300Repository: Redot-Engine/redot-engine
Length of output: 10769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- nullable operation test sources ---'
for f in \
modules/gdscript/tests/scripts/analyzer/warnings/nullable_unsafe_operations.gd \
modules/gdscript/tests/scripts/analyzer/warnings/nullable_unsafe_transfer.gd \
modules/gdscript/tests/scripts/runtime/features/nullable_nested_enum.gd
do
if [ -f "$f" ]; then
echo "### $f"
cat -n "$f"
fi
done
printf '%s\n' '--- binary operator warning branch ---'
sed -n '3258,3285p' modules/gdscript/gdscript_analyzer.cpp
printf '%s\n' '--- method-call analysis branch ---'
sed -n '6090,6180p' modules/gdscript/gdscript_analyzer.cpp
printf '%s\n' '--- member/property analysis branches ---'
sed -n '4948,4980p' modules/gdscript/gdscript_analyzer.cppRepository: Redot-Engine/redot-engine
Length of output: 8443
Correct the nullable warning description.
- Exclude
==,!=,and, andorfrom “in an operator”; the analyzer does not warn for these operators. - Qualify “a statically-typed value cannot be
null”; object references can holdnullwithout the?suffix.
The remaining documented warning paths match the analyzer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/gdscript/doc_classes/GDScript.xml` around lines 27 - 28, Update the
nullable warning description in the GDScript documentation to exclude ==, !=,
and, and or from the “in an operator” wording, since these operators are not
analyzed. Clarify the non-nullability statement so it applies only to statically
typed non-object values, while object references may still hold null without the
? suffix; preserve the remaining warning paths and behavior descriptions.
JoltedJon
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Title.
Summary by CodeRabbit
?) for values, containers, enums, callables, and return types.nullwhen not explicitly initialized.