Add an @override annotation for enforcing proper override semantics

Author: Shadows-of-Fire

core/object/script_language.h

@@ -306,6 +306,23 @@ class ScriptLanguage : public Object {
 		LOCATION_OTHER = 1 << 10,
 	};
 
+	/**
+	 * @brief Represents a position in a text editor. Uses zero-indexed line and column numbers.
+	 */
+	struct CodePos {
+		int line;
+		int column;
+	};
+
+	/**
+	 * @brief Represents a replacement hunk of text for a given region based on the start and end pos.
+	 */
+	struct TextEdit {
+		CodePos start;
+		CodePos end;
+		String new_text;
+	};
+
 	struct CodeCompletionOption {
 		CodeCompletionKind kind = CODE_COMPLETION_KIND_PLAIN_TEXT;
 		String display;
@@ -318,6 +335,14 @@ class ScriptLanguage : public Object {
 		int location = LOCATION_OTHER;
 		String theme_color_name;
 
+		/**
+		 * @brief Additional text edits that will be applied if this suggestion is applied.
+		 *        These should not modify the range that is being modified by insert_text.
+		 *
+		 * @note  The caret(s) will attempt to maintain their relative positions regardless of what changes are applied here.
+		 */
+		Vector<TextEdit> additional_edits;
+
 		CodeCompletionOption() {}
 
 		CodeCompletionOption(const String &p_text, CodeCompletionKind p_kind, int p_location = LOCATION_OTHER, const String &p_theme_color_name = "") {

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.additional_edits);
 	}
 	text_editor->update_code_completion_options(forced);
 }

modules/gdscript/doc_classes/@GDScript.xml

@@ -782,6 +782,30 @@
 				[/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

@@ -3443,6 +3443,66 @@ 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.
+ */
+Vector<ScriptLanguage::TextEdit> get_override_text_edits(const GDScriptParser::CompletionContext &ctx, const Vector<String> &code_by_line) {
+	int line = ctx.current_line;
+	// 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_by_line.get(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++;
+	}
+
+	if (!has_override_annot && func_line != -1) {
+		String text = code_by_line.get(func_line);
+		ScriptLanguage::TextEdit edit;
+		edit.start = { func_line - 1, 0 }; // Always start at the base of the line above the func_line
+		edit.end = { func_line, 0 }; // And then end on the func line itself.
+
+		// What we actually need to do here is replace whatever the line above the func_line was with "<line>\n@override\n"
+		// But the @override has to have the same indentation level as the func_line.
+
+		int func_column = text.length() - text.lstrip(" \t").length();
+		String new_text = code_by_line.get(func_line - 1);
+		new_text = new_text + "\n" + text.substr(0, func_column) + "@override\n";
+
+		edit.new_text = new_text;
+
+		return { edit };
+	}
+
+	return {};
+}
+
+} // 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") ? "'" : "\"";
 
@@ -3452,6 +3512,8 @@ ::Error GDScriptLanguage::complete_code(const String &p_code, const String &p_pa
 	parser.parse(p_code, p_path, true);
 	analyzer.analyze();
 
+	Vector<String> code_by_line = p_code.split("\n");
+
 	r_forced = false;
 	HashMap<String, ScriptLanguage::CodeCompletionOption> options;
 
@@ -3704,6 +3766,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.additional_edits = get_override_text_edits(completion_context, code_by_line);
+
 							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 +3844,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.additional_edits = get_override_text_edits(completion_context, code_by_line);
+
 				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.
@@ -266,6 +267,7 @@ void GDScriptParser::override_completion_context(const Node *p_for_node, Complet
 	context.current_function = current_function;
 	context.current_suite = current_suite;
 	context.current_line = tokenizer->get_cursor_line();
+	context.current_column = tokenizer->get_cursor_column();
 	context.current_argument = p_argument;
 	context.node = p_node;
 	context.parser = this;
@@ -288,6 +290,7 @@ void GDScriptParser::make_completion_context(CompletionType p_type, Node *p_node
 	context.current_function = current_function;
 	context.current_suite = current_suite;
 	context.current_line = tokenizer->get_cursor_line();
+	context.current_column = tokenizer->get_cursor_column();
 	context.current_argument = p_argument;
 	context.node = p_node;
 	context.parser = this;
@@ -310,6 +313,7 @@ void GDScriptParser::make_completion_context(CompletionType p_type, Variant::Typ
 	context.current_function = current_function;
 	context.current_suite = current_suite;
 	context.current_line = tokenizer->get_cursor_line();
+	context.current_column = tokenizer->get_cursor_column();
 	context.builtin_type = p_builtin_type;
 	context.parser = this;
 	if (!completion_call_stack.is_empty()) {
@@ -4397,6 +4401,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;
 
@@ -1319,6 +1329,7 @@ class GDScriptParser {
 		FunctionNode *current_function = nullptr;
 		SuiteNode *current_suite = nullptr;
 		int current_line = -1;
+		int current_column = -1;
 		union {
 			int current_argument = -1;
 			int type_chain_index;
@@ -1529,6 +1540,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",

modules/gdscript/gdscript_warning.h

@@ -89,6 +89,7 @@ class GDScriptWarning {
 		NATIVE_METHOD_OVERRIDE, // The script method overrides a native one, this may not work as intended.
 		GET_NODE_DEFAULT_WITHOUT_ONREADY, // A class variable uses `get_node()` (or the `$` notation) as its default value, but does not use the @onready annotation.
 		ONREADY_WITH_EXPORT, // The `@onready` annotation will set the value after `@export` which is likely not intended.
+		IMPLICIT_FUNCTION_OVERRIDE, // A function overrides a function in a parent class, but does not have the `@override` annotation.
 #ifndef DISABLE_DEPRECATED
 		PROPERTY_USED_AS_FUNCTION, // Function not found, but there's a property with the same name.
 		CONSTANT_USED_AS_FUNCTION, // Function not found, but there's a constant with the same name.
@@ -146,6 +147,7 @@ class GDScriptWarning {
 		ERROR, // NATIVE_METHOD_OVERRIDE // May not work as expected.
 		ERROR, // GET_NODE_DEFAULT_WITHOUT_ONREADY // May not work as expected.
 		ERROR, // ONREADY_WITH_EXPORT // May not work as expected.
+		WARN, // IMPLICIT_FUNCTION_OVERRIDE
 #ifndef DISABLE_DEPRECATED
 		WARN, // PROPERTY_USED_AS_FUNCTION
 		WARN, // CONSTANT_USED_AS_FUNCTION