Add an @OverRide annotation for enforcing proper override semantics

Author: Shadows-of-Fire

core/object/script_language.h

@@ -318,6 +318,13 @@ class ScriptLanguage : public Object {
 		int location = LOCATION_OTHER;
 		String theme_color_name;
 
+		/**
+		 * @brief A callback that is executed after this completion option is applied by the code editor (in confirm_code_completion).
+		 *        This callback receives both the CodeEdit instance and the current caret that was used for the application.
+		 *        For multi-line applications, this callback may be invoked multiple times with different caret values.
+		 */
+		Callable on_applied;
+
 		CodeCompletionOption() {}
 
 		CodeCompletionOption(const String &p_text, CodeCompletionKind p_kind, int p_location = LOCATION_OTHER, const String &p_theme_color_name = "") {

doc/classes/CodeEdit.xml

@@ -50,6 +50,7 @@
 			<param index="4" name="icon" type="Resource" default="null" />
 			<param index="5" name="value" type="Variant" default="null" />
 			<param index="6" name="location" type="int" default="1024" />
+			<param index="7" name="on_applied" type="Callable" default="Callable()" />
 			<description>
 				Submits an item to the queue of potential candidates for the autocomplete menu. Call [method update_code_completion_options] to update the list.
 				[param location] indicates location of the option relative to the location of the code completion query. See [enum CodeEdit.CodeCompletionLocation] for how to set this value.

editor/gui/code_editor.cpp

@@ -1047,7 +1047,7 @@ void CodeTextEditor::_complete_request() {
 		} else if (e.insert_text.begins_with("#") || e.insert_text.begins_with("//")) {
 			font_color = completion_comment_color;
 		}
-		text_editor->add_code_completion_option((CodeEdit::CodeCompletionKind)e.kind, e.display, e.insert_text, font_color, _get_completion_icon(e), e.default_value, e.location);
+		text_editor->add_code_completion_option((CodeEdit::CodeCompletionKind)e.kind, e.display, e.insert_text, font_color, _get_completion_icon(e), e.default_value, e.location, e.on_applied);
 	}
 	text_editor->update_code_completion_options(forced);
 }

modules/gdscript/doc_classes/@GDScript.xml

@@ -782,6 +782,28 @@
 				[/codeblock]
 			</description>
 		</annotation>
+		<annotation name="@override">
+			<return type="void" />
+			<description>
+				Marks a function as being an override. Override functions must override a function defined in a parent class, otherwise an error is raised.
+				Functions that override methods but do not have this annotation will raise a warning.
+				[codeblock]
+				class Shape:
+					func draw()
+
+				class Circle extends Shape:
+					@override func draw(): # No error, no warnings
+						print("Drawing a circle.")
+
+				class Square extends Shape:
+					func draw(): # Warning due to missing @override
+						print("Drawing a square.")
+
+                    @override func junk(): # Error as no parent defines junk()
+                        pass
+				[/codeblock]
+			</description>
+		</annotation>
 		<annotation name="@rpc">
 			<return type="void" />
 			<param index="0" name="mode" type="String" default="&quot;authority&quot;" />

modules/gdscript/gdscript_analyzer.cpp

@@ -1954,12 +1954,52 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 
 				push_error(vformat(R"(The function signature doesn't match the parent. Parent signature is "%s".)", parent_signature), p_function);
 			}
+
+			// Mark the function as an override if the check succeeds.
+			p_function->is_override = true;
+
+			// Then if we don't see the @override annotation, raise a warning.
+			if (!p_function->has_override_annot) {
+				// Resolve the name of the base class that owns the overridden for error reporting
+				StringName base_class_name{ "**UnknownNativeClass**" };
+
+				const GDScriptParser::DataType *cur_base_type = &base_type;
+				while (cur_base_type) {
+					if (cur_base_type->kind == GDScriptParser::DataType::Kind::NATIVE) {
+						MethodInfo method_info;
+						if (ClassDB::get_method_info(cur_base_type->native_type, function_name, &method_info)) {
+							base_class_name = cur_base_type->native_type;
+						}
+						break; // Break unconditionally since this method should always give us the appropriate native class name.
+					} else {
+						const GDScriptParser::ClassNode *base_class = base_type.class_type;
+						if (base_class->has_function(function_name)) {
+							if (base_class->identifier) {
+								base_class_name = base_class->identifier->name;
+							} else {
+								base_class_name = "**UnidentifiedClass**";
+							}
+							break;
+						}
+						cur_base_type = &base_class->base_type;
+					}
+				}
+
+				parser->push_warning(p_function, GDScriptWarning::IMPLICIT_FUNCTION_OVERRIDE, function_name, base_class_name);
+			}
+
 #ifdef DEBUG_ENABLED
 			if (native_base != StringName()) {
 				parser->push_warning(p_function, GDScriptWarning::NATIVE_METHOD_OVERRIDE, function_name, native_base);
 			}
 #endif // DEBUG_ENABLED
 		}
+
+		// If a function with `@override` doesn't override anything, raise an error.
+		if (p_function->has_override_annot && !p_function->is_override) {
+			push_error(vformat(R"(The function %s has the @override annotation, but does not override anything.)", function_name), p_function);
+		}
+
 #endif // TOOLS_ENABLED
 	}
 

modules/gdscript/gdscript_analyzer.h

@@ -128,7 +128,24 @@ class GDScriptAnalyzer {
 	GDScriptParser::DataType type_from_variant(const Variant &p_value, const GDScriptParser::Node *p_source);
 	GDScriptParser::DataType type_from_property(const PropertyInfo &p_property, bool p_is_arg = false, bool p_is_readonly = false) const;
 	GDScriptParser::DataType make_global_class_meta_type(const StringName &p_class_name, const GDScriptParser::Node *p_source);
+
+	/**
+	 * @brief Attempts to look up the function signature for a function based on name and base type.
+	 *
+	 * @param[in]  p_source             The source node in context of which the signature is being resolved. Used for error reporting.
+	 * @param[in]  p_is_constructor     If the target function call is a constructor. In this case `p_function` is always "new".
+	 * @param[in]  base_type            The target type to resolve the function on. If looking for a function `SomeClass.a_function()`, this is `SomeClass`.
+	 * @param[in]  p_function           The name of the target function.
+	 * @param[out] r_return_type        The return type of the found function.
+	 * @param[out] r_par_types          A list of the parameters types of the found function, in order from left to right.
+	 * @param[out] r_default_arg_count  The number of parameters which have default values.
+	 * @param[out] r_method_flags       The flags of the found function.
+	 * @param[out] r_native_class       If DEBUG_ENABLED=1, and the found function is a native method, the string name of the owning native class. Otherwise an empty string name.
+	 *
+	 * @return True if the function signature was successfully resolved, false otherwise. See the remaining output variables for other data.
+	 */
 	bool get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, BitField<MethodFlags> &r_method_flags, StringName *r_native_class = nullptr);
+
 	bool function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, BitField<MethodFlags> &r_method_flags);
 	void validate_call_arg(const List<GDScriptParser::DataType> &p_par_types, int p_default_args_count, bool p_is_vararg, const GDScriptParser::CallNode *p_call);
 	void validate_call_arg(const MethodInfo &p_method, const GDScriptParser::CallNode *p_call);

modules/gdscript/gdscript_editor.cpp

@@ -52,6 +52,7 @@
 #include "editor/editor_string_names.h"
 #include "editor/file_system/editor_file_system.h"
 #include "editor/settings/editor_settings.h"
+#include "scene/gui/code_edit.h"
 #endif
 
 Vector<String> GDScriptLanguage::get_comment_delimiters() const {
@@ -3443,6 +3444,55 @@ static void _find_call_arguments(GDScriptParser::CompletionContext &p_context, c
 	r_forced = r_result.size() > 0;
 }
 
+namespace {
+
+/**
+ * @brief Used as the `on_applied` callback for GDScript completions of category `COMPLETION_OVERRIDE_METHOD`.
+ *        This method will attempt to automatically apply the `@override` annotation when using completion to generate an override.
+ *        If the `@override` annotation is already present, nothing will occur.
+ *
+ * @param code_edit The code editor instance.
+ * @param caret     The caret that was used for this application of the completion.
+ */
+void _update_overrides_after_completion(CodeEdit *code_edit, int caret) {
+	int line = code_edit->get_caret_line(caret);
+	// Backtrack in the current line and upwards a few lines to search for `@override`. If we don't see it, apply it inline before the `func` keyword.
+	bool has_override_annot = false;
+	int lines_traversed = 0;
+	int func_line = -1; // Generally speaking we expect the line with `func` to be the current line, but for sanity we'll search for it anyway.
+
+	while (!has_override_annot && lines_traversed < 10 && line >= 0) {
+		String text = code_edit->get_line(line);
+
+		if (text.contains("func")) {
+			if (func_line == -1) {
+				func_line = line;
+			} else {
+				break; // We've hit another `func` decl. Anything in or above this line doesn't apply to this function anymore.
+			}
+		}
+
+		if (text.contains("@override")) {
+			has_override_annot = true;
+			break;
+		}
+
+		line--;
+		lines_traversed++;
+	}
+
+	// TODO: Review stage - I'd prefer to insert the `@override` on the line before `func`, but I'm not sure how to do that.
+	if (!has_override_annot && func_line != -1) {
+		String text = code_edit->get_line(func_line);
+		code_edit->set_line(func_line, "@override " + text);
+		int caret_col = code_edit->get_caret_column(caret);
+		code_edit->set_caret_line(func_line, true, true, 0, caret);
+		code_edit->set_caret_column(caret_col + strlen("@override "), false, caret);
+	}
+}
+
+} // namespace
+
 ::Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path, Object *p_owner, List<ScriptLanguage::CodeCompletionOption> *r_options, bool &r_forced, String &r_call_hint) {
 	const String quote_style = EDITOR_GET("text_editor/completion/use_single_quotes") ? "'" : "\"";
 
@@ -3704,6 +3754,10 @@ ::Error GDScriptLanguage::complete_code(const String &p_code, const String &p_pa
 							String display_name = member.function->identifier->name;
 							display_name += member.function->signature + ":";
 							ScriptLanguage::CodeCompletionOption option(display_name, ScriptLanguage::CODE_COMPLETION_KIND_FUNCTION);
+
+							// When inserting a completion for a function override, we want to automatically add the @override annotation to the completion.
+							option.on_applied = callable_mp_static(_update_overrides_after_completion);
+
 							options.insert(member.function->identifier->name, option); // Insert name instead of display to track duplicates.
 						}
 						native_type = native_type.class_type->base_type;
@@ -3778,6 +3832,10 @@ ::Error GDScriptLanguage::complete_code(const String &p_code, const String &p_pa
 				method_hint += ":";
 
 				ScriptLanguage::CodeCompletionOption option(method_hint, ScriptLanguage::CODE_COMPLETION_KIND_FUNCTION);
+
+				// When inserting a completion for a function override, we want to automatically add the @override annotation to the completion.
+				option.on_applied = callable_mp_static(_update_overrides_after_completion);
+
 				options.insert(option.display, option);
 			}
 		} break;

modules/gdscript/gdscript_parser.cpp

@@ -97,6 +97,7 @@ GDScriptParser::GDScriptParser() {
 		register_annotation(MethodInfo("@icon", PropertyInfo(Variant::STRING, "icon_path")), AnnotationInfo::SCRIPT, &GDScriptParser::icon_annotation);
 		register_annotation(MethodInfo("@static_unload"), AnnotationInfo::SCRIPT, &GDScriptParser::static_unload_annotation);
 		register_annotation(MethodInfo("@abstract"), AnnotationInfo::SCRIPT | AnnotationInfo::CLASS | AnnotationInfo::FUNCTION, &GDScriptParser::abstract_annotation);
+		register_annotation(MethodInfo("@override"), AnnotationInfo::FUNCTION, &GDScriptParser::override_annotation);
 		// Onready annotation.
 		register_annotation(MethodInfo("@onready"), AnnotationInfo::VARIABLE, &GDScriptParser::onready_annotation);
 		// Export annotations.
@@ -4397,6 +4398,20 @@ bool GDScriptParser::abstract_annotation(AnnotationNode *p_annotation, Node *p_t
 	ERR_FAIL_V_MSG(false, R"("@abstract" annotation can only be applied to classes and functions.)");
 }
 
+bool GDScriptParser::override_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
+	if (p_target->type == Node::FUNCTION) {
+		FunctionNode *function_node = static_cast<FunctionNode *>(p_target);
+		if (function_node->is_static) {
+			push_error(R"("@override" annotation cannot be applied to static functions.)", p_annotation);
+			return false;
+		}
+		// The only thing we can do here is record that we have the annotation. The analyzer gets to do the rest.
+		function_node->has_override_annot = true;
+		return true;
+	}
+	ERR_FAIL_V_MSG(false, R"("@override" annotation can only be applied to functions.)");
+}
+
 bool GDScriptParser::onready_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
 	ERR_FAIL_COND_V_MSG(p_target->type != Node::VARIABLE, false, R"("@onready" annotation can only be applied to class variables.)");
 

modules/gdscript/gdscript_parser.h

@@ -857,6 +857,15 @@ class GDScriptParser {
 		bool is_abstract = false;
 		bool is_static = false; // For lambdas it's determined in the analyzer.
 		bool is_coroutine = false;
+
+		// If this function is known to override a function in the parent type.
+		// If a function is an override, but does not have the @override annotation, a warning is raised.
+		// The value of this field is undefined unless resolved_signature is true.
+		bool is_override = false;
+
+		// If this function node has the @override annotation. This does not indicate if the function is actually an override or not.
+		bool has_override_annot = false;
+
 		Variant rpc_config;
 		MethodInfo info;
 		LambdaNode *source_lambda = nullptr;
@@ -867,6 +876,7 @@ class GDScriptParser {
 		String signature; // For autocompletion.
 #endif // TOOLS_ENABLED
 
+		// True if GDScriptAnalyzer::resolve_function_signature has been called for this function node.
 		bool resolved_signature = false;
 		bool resolved_body = false;
 
@@ -1529,6 +1539,7 @@ class GDScriptParser {
 	bool icon_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool static_unload_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool abstract_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
+	bool override_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool onready_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	template <PropertyHint t_hint, Variant::Type t_type>
 	bool export_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);

modules/gdscript/gdscript_warning.cpp

@@ -162,6 +162,9 @@ String GDScriptWarning::get_message() const {
 			return vformat(R"*(The default value uses "%s" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.)*", symbols[0]);
 		case ONREADY_WITH_EXPORT:
 			return R"("@onready" will set the default value after "@export" takes effect and will override it.)";
+		case IMPLICIT_FUNCTION_OVERRIDE:
+			CHECK_SYMBOLS(2);
+			return vformat(R"*(The method "%s()" overrides "%s::%s()" but does not have the @override annotation.)*", symbols[0], symbols[1], symbols[0]);
 #ifndef DISABLE_DEPRECATED
 		// Never produced. These warnings migrated from 3.x by mistake.
 		case PROPERTY_USED_AS_FUNCTION: // There is already an error.
@@ -238,6 +241,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
 		PNAME("NATIVE_METHOD_OVERRIDE"),
 		PNAME("GET_NODE_DEFAULT_WITHOUT_ONREADY"),
 		PNAME("ONREADY_WITH_EXPORT"),
+		PNAME("IMPLICIT_FUNCTION_OVERRIDE"),
 #ifndef DISABLE_DEPRECATED
 		"PROPERTY_USED_AS_FUNCTION",
 		"CONSTANT_USED_AS_FUNCTION",