godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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.
This commit is contained in:
George Marques 2022-03-06 11:09:12 -03:00
parent 272b355954
commit 1ebcb58e69
No known key found for this signature in database
GPG Key ID: 046BD46A3201E43D
14 changed files with 141 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
modules/gdscript/gdscript_analyzer.cpp
View File

@ -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;

2
modules/gdscript/gdscript_analyzer.h
View File

@ -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);

10
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.gd Normal file
View File

@ -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

2
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_less.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
The function signature doesn't match the parent. Parent signature is "int my_function(int)".

10
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.gd Normal file
View File

@ -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

2
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_count_more.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
The function signature doesn't match the parent. Parent signature is "int my_function(int)".

10
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.gd Normal file
View File

@ -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

2
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
The function signature doesn't match the parent. Parent signature is "int my_function(int = default)".

10
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.gd Normal file
View File

@ -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

2
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_type.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
The function signature doesn't match the parent. Parent signature is "int my_function(int)".

10
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.gd Normal file
View File

@ -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()

2
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_return_type.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
The function signature doesn't match the parent. Parent signature is "int my_function()".

17
modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.gd Normal file
View File

@ -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

3
modules/gdscript/tests/scripts/analyzer/features/function_match_parent_signature_with_extra_parameters.out Normal file
View File

@ -0,0 +1,3 @@
GDTEST_OK
1
0