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.

14 files changed, 141 insertions, 9 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;
diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h
index 2697a6ec2b..7b8883e1d3 100644
--- a/modules/gdscript/gdscript_analyzer.h
+++ b/modules/gdscript/gdscript_analyzer.h
@@ -105,7 +105,7 @@ class GDScriptAnalyzer {
 	GDScriptParser::DataType type_from_metatype(const GDScriptParser::DataType &p_meta_type) const;
 	GDScriptParser::DataType type_from_property(const PropertyInfo &p_property) const;
 	GDScriptParser::DataType make_global_class_meta_type(const StringName &p_class_name, const GDScriptParser::Node *p_source);
-	bool get_function_signature(GDScriptParser::CallNode *p_source, GDScriptParser::DataType 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 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, bool &r_static, bool &r_vararg);
 	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, bool &r_static, bool &r_vararg);
 	bool validate_call_arg(const List<GDScriptParser::DataType> &p_par_types, int p_default_args_count, bool p_is_vararg, const GDScriptParser::CallNode *p_call);
 	bool validate_call_arg(const MethodInfo &p_method, const GDScriptParser::CallNode *p_call);
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.gd
new file mode 100644
index 0000000000..435711fcaf
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.gd
@@ -0,0 +1,10 @@
+func test():
+	print("Shouldn't reach this")
+
+class Parent:
+	func my_function(_par1: int) -> int:
+		return 0
+
+class Child extends Parent:
+	func my_function() -> int:
+		return 0
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.out b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.out
new file mode 100644
index 0000000000..3baeb17066
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+The function signature doesn't match the parent. Parent signature is "int my_function(int)".
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.gd
new file mode 100644
index 0000000000..2bd392e8f8
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.gd
@@ -0,0 +1,10 @@
+func test():
+	print("Shouldn't reach this")
+
+class Parent:
+	func my_function(_par1: int) -> int:
+		return 0
+
+class Child extends Parent:
+	func my_function(_pary1: int, _par2: int) -> int:
+		return 0
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.out b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.out
new file mode 100644
index 0000000000..3baeb17066
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+The function signature doesn't match the parent. Parent signature is "int my_function(int)".
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.gd
new file mode 100644
index 0000000000..49ec82ce2d
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.gd
@@ -0,0 +1,10 @@
+func test():
+	print("Shouldn't reach this")
+
+class Parent:
+	func my_function(_par1: int = 0) -> int:
+		return 0
+
+class Child extends Parent:
+	func my_function(_par1: int) -> int:
+		return 0
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out
new file mode 100644
index 0000000000..665c229339
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+The function signature doesn't match the parent. Parent signature is "int my_function(int = default)".
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.gd
new file mode 100644
index 0000000000..4a17a7831f
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.gd
@@ -0,0 +1,10 @@
+func test():
+	print("Shouldn't reach this")
+
+class Parent:
+	func my_function(_par1: int) -> int:
+		return 0
+
+class Child extends Parent:
+	func my_function(_par1: Vector2) -> int:
+		return 0
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.out b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.out
new file mode 100644
index 0000000000..3baeb17066
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+The function signature doesn't match the parent. Parent signature is "int my_function(int)".
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.gd
new file mode 100644
index 0000000000..b205ec96ef
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.gd
@@ -0,0 +1,10 @@
+func test():
+	print("Shouldn't reach this")
+
+class Parent:
+	func my_function() -> int:
+		return 0
+
+class Child extends Parent:
+	func my_function() -> Vector2:
+		return Vector2()
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.out b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.out
new file mode 100644
index 0000000000..5b22739a93
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+The function signature doesn't match the parent. Parent signature is "int my_function()".
diff --git a/modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.gd b/modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.gd
new file mode 100644
index 0000000000..d678f3acfc
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.gd
@@ -0,0 +1,17 @@
+func test():
+	var instance := Parent.new()
+	var result := instance.my_function(1)
+	print(result)
+	assert(result == 1)
+	instance = Child.new()
+	result = instance.my_function(2)
+	print(result)
+	assert(result == 0)
+
+class Parent:
+	func my_function(par1: int) -> int:
+		return par1
+
+class Child extends Parent:
+	func my_function(_par1: int, par2: int = 0) -> int:
+		return par2
diff --git a/modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.out b/modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.out
new file mode 100644
index 0000000000..fc5315a501
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.out
@@ -0,0 +1,3 @@
+GDTEST_OK
+1
+0