GDScript: Check if method signature matches the parent

To guarantee polymorphism, a method signature must be compatible with
the parent. This checks if:

1. Return type is the same.
2. The subclass method takes at least the same amount of parameters.
3. The matching parameters have the same type.
4. If the subclass takes more parameters, all of the extra ones have a
default value.
5. If the superclass has default values, so must have the subclass.

There's a few test cases to ensure this holds up.

1 files changed, 60 insertions, 8 deletions

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 3ff8b2b91a..547fd1cfd5 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -1139,7 +1139,7 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 #endif // TOOLS_ENABLED
 	}
 
-	if (p_function->identifier->name == "_init") {
+	if (p_function->identifier->name == GDScriptLanguage::get_singleton()->strings._init) {
 		// Constructor.
 		GDScriptParser::DataType return_type = parser->current_class->get_datatype();
 		return_type.is_meta_type = false;
@@ -1153,6 +1153,57 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 	} else {
 		GDScriptParser::DataType return_type = resolve_datatype(p_function->return_type);
 		p_function->set_datatype(return_type);
+
+#ifdef DEBUG_ENABLED
+		// Check if the function signature matches the parent. If not it's an error since it breaks polymorphism.
+		// Not for the constructor which can vary in signature.
+		GDScriptParser::DataType base_type = parser->current_class->base_type;
+		GDScriptParser::DataType parent_return_type;
+		List<GDScriptParser::DataType> parameters_types;
+		int default_par_count = 0;
+		bool is_static = false;
+		bool is_vararg = false;
+		if (get_function_signature(p_function, false, base_type, p_function->identifier->name, parent_return_type, parameters_types, default_par_count, is_static, is_vararg)) {
+			bool valid = p_function->is_static == is_static;
+			valid = valid && parent_return_type == p_function->get_datatype();
+
+			int par_count_diff = p_function->parameters.size() - parameters_types.size();
+			valid = valid && par_count_diff >= 0;
+			valid = valid && p_function->default_arg_values.size() >= default_par_count + par_count_diff;
+
+			int i = 0;
+			for (const GDScriptParser::DataType &par_type : parameters_types) {
+				valid = valid && par_type == p_function->parameters[i++]->get_datatype();
+			}
+
+			if (!valid) {
+				// Compute parent signature as a string to show in the error message.
+				String parent_signature = parent_return_type.is_hard_type() ? parent_return_type.to_string() : "Variant";
+				if (parent_signature == "null") {
+					parent_signature = "void";
+				}
+				parent_signature += " " + p_function->identifier->name.operator String() + "(";
+				int j = 0;
+				for (const GDScriptParser::DataType &par_type : parameters_types) {
+					if (j > 0) {
+						parent_signature += ", ";
+					}
+					String parameter = par_type.to_string();
+					if (parameter == "null") {
+						parameter = "Variant";
+					}
+					parent_signature += parameter;
+					if (j == parameters_types.size() - default_par_count) {
+						parent_signature += " = default";
+					}
+
+					j++;
+				}
+				parent_signature += ")";
+				push_error(vformat(R"(The function signature doesn't match the parent. Parent signature is "%s".)", parent_signature), p_function);
+			}
+		}
+#endif
 	}
 
 	parser->current_function = previous_function;
@@ -2416,7 +2467,9 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 	GDScriptParser::DataType return_type;
 	List<GDScriptParser::DataType> par_types;
 
-	if (get_function_signature(p_call, base_type, p_call->function_name, return_type, par_types, default_arg_count, is_static, is_vararg)) {
+	bool is_constructor = (base_type.is_meta_type || (p_call->callee && p_call->callee->type == GDScriptParser::Node::IDENTIFIER)) && p_call->function_name == SNAME("new");
+
+	if (get_function_signature(p_call, is_constructor, base_type, p_call->function_name, return_type, par_types, default_arg_count, is_static, is_vararg)) {
 		// If the function require typed arrays we must make literals be typed.
 		for (const KeyValue<int, GDScriptParser::ArrayNode *> &E : arrays) {
 			int index = E.key;
@@ -3576,7 +3629,7 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo
 	return result;
 }
 
-bool GDScriptAnalyzer::get_function_signature(GDScriptParser::CallNode *p_source, GDScriptParser::DataType p_base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg) {
+bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType p_base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg) {
 	r_static = false;
 	r_vararg = false;
 	r_default_arg_count = 0;
@@ -3616,8 +3669,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::CallNode *p_source
 		return false;
 	}
 
-	bool is_constructor = (p_base_type.is_meta_type || (p_source->callee && p_source->callee->type == GDScriptParser::Node::IDENTIFIER)) && p_function == StaticCString::create("new");
-	if (is_constructor) {
+	if (p_is_constructor) {
 		function_name = "_init";
 		r_static = true;
 	}
@@ -3638,7 +3690,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::CallNode *p_source
 	}
 
 	if (found_function != nullptr) {
-		r_static = is_constructor || found_function->is_static;
+		r_static = p_is_constructor || found_function->is_static;
 		for (int i = 0; i < found_function->parameters.size(); i++) {
 			r_par_types.push_back(found_function->parameters[i]->get_datatype());
 			if (found_function->parameters[i]->default_value != nullptr) {
@@ -3664,7 +3716,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::CallNode *p_source
 	}
 
 	// If the base is a script, it might be trying to access members of the Script class itself.
-	if (p_base_type.is_meta_type && !is_constructor && (p_base_type.kind == GDScriptParser::DataType::SCRIPT || p_base_type.kind == GDScriptParser::DataType::CLASS)) {
+	if (p_base_type.is_meta_type && !p_is_constructor && (p_base_type.kind == GDScriptParser::DataType::SCRIPT || p_base_type.kind == GDScriptParser::DataType::CLASS)) {
 		MethodInfo info;
 		StringName script_class = p_base_type.kind == GDScriptParser::DataType::SCRIPT ? p_base_type.script_type->get_class_name() : StringName(GDScript::get_class_static());
 
@@ -3684,7 +3736,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::CallNode *p_source
 	}
 #endif
 
-	if (is_constructor) {
+	if (p_is_constructor) {
 		// Native types always have a default constructor.
 		r_return_type = p_base_type;
 		r_return_type.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;