Skip to content

Conversation

@Shadows-of-Fire
Copy link
Contributor

@Shadows-of-Fire Shadows-of-Fire commented Aug 31, 2025

This PR adds the @override annotation, which is used to enforce that a method overrides a method in a parent class. A method that is declared @override but does not actually override anything will be treated as an error.

Methods that override without explicitly declaring @override will trigger an IMPLICIT_FUNCTION_OVERRIDE warning.

To support this feature, this PR also adds a new feature to CodeCompletionOption - an on_applied callback. This can be used to make additional changes in the editor after the suggestion has been applied. The GDScriptLanguage makes use of this to automatically apply the @override annotation when generating a stub for a method override.

Partially addresses godotengine/godot-proposals#12310

@Shadows-of-Fire Shadows-of-Fire requested review from a team as code owners August 31, 2025 06:07
Comment on lines +4404 to +4410
if (function_node->is_static) {
push_error(R"("@override" annotation cannot be applied to static functions.)", p_annotation);
return false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should probably also add an exclusion rule for @abstract here, since an abstract method can never override something.

@Shadows-of-Fire Shadows-of-Fire force-pushed the override-annot branch 2 times, most recently from f96ee6f to dd4696b Compare August 31, 2025 23:03
_add_code_completion_option(p_type, p_display_text, p_insert_text, p_text_color, p_icon, p_value, p_location, {});
}

void CodeEdit::_add_code_completion_option(CodeCompletionKind p_type, const String &p_display_text, const String &p_insert_text, const Color &p_text_color, const Ref<Resource> &p_icon, const Variant &p_value, int p_location, const Vector<ScriptLanguage::TextEdit> &additional_edits) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happened because I didn't know how to properly get TextEdit and CodePos exposed to scripting, and add_code_completion_option is available to scripting. The ClassDB bind methods are unable to handle the extra default argument, so the original add_code_completion_option became a bouncer to this method.

Comment on lines 2425 to 2431
for (const ScriptLanguage::TextEdit &edit : selected_option.additional_edits) {
ScriptLanguage::CodePos start = edit.start;
ScriptLanguage::CodePos end = edit.end;

int nb_lines = this->get_text().size();

this->remove_text(start.line, start.column, end.line, end.column);
this->insert_text(edit.new_text, start.line, start.column);

int new_nb_lines = this->get_text().size();
// TODO: Does the caret magically move or do we need to update it?
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another mental TODO here is that this currently applies each time for a multi-caret completion.

Given that the positions in ScriptLanguage::TextEdit are absolute, and are not aware of the fact that a completion could be used for multiple carets, we might want to only apply them once after all carets have completed

https://i.imgur.com/9liZOSc.mp4

@Lazy-Rabbit-2001
Copy link
Contributor

Lazy-Rabbit-2001 commented Sep 1, 2025

Salvages #103859

@Shadows-of-Fire Shadows-of-Fire changed the title Add an @override annotation for enforcing proper override semantics GDScript: Add an @override annotation for enforcing proper override semantics Sep 20, 2025
@Lazy-Rabbit-2001
Copy link
Contributor

Haven't checked your codes but I'm not sure if there is a method list with methods defined in the parent when @override is used and you are about to type the name of the method in the super class during the definition of function signature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants