From cb8b89fd95770ab96d269d1f4d22e7945a29a8ef Mon Sep 17 00:00:00 2001 From: Danil Alexeev Date: Thu, 28 Sep 2023 11:28:28 +0300 Subject: [PATCH] GDScript: Add return type covariance and parameter type contravariance --- modules/gdscript/gdscript_analyzer.cpp | 35 ++++++++++++++++--- ...ion_param_type_invalid_contravariance_1.gd | 10 ++++++ ...on_param_type_invalid_contravariance_1.out | 2 ++ ...ion_param_type_invalid_contravariance_2.gd | 10 ++++++ ...on_param_type_invalid_contravariance_2.out | 2 ++ ...ion_param_type_invalid_contravariance_3.gd | 10 ++++++ ...on_param_type_invalid_contravariance_3.out | 2 ++ ...nction_return_type_invalid_covariance_1.gd | 10 ++++++ ...ction_return_type_invalid_covariance_1.out | 2 ++ ...nction_return_type_invalid_covariance_2.gd | 10 ++++++ ...ction_return_type_invalid_covariance_2.out | 2 ++ ...nction_return_type_invalid_covariance_3.gd | 10 ++++++ ...ction_return_type_invalid_covariance_3.out | 2 ++ ...nction_return_type_invalid_covariance_4.gd | 10 ++++++ ...ction_return_type_invalid_covariance_4.out | 2 ++ .../function_param_type_contravariance.gd | 20 +++++++++++ .../function_param_type_contravariance.out | 1 + .../function_return_type_covariance.gd | 32 +++++++++++++++++ .../function_return_type_covariance.out | 1 + 19 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.out create mode 100644 modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.out create mode 100644 modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.out diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 02b6af1e87a..bd1535ef16a 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1671,15 +1671,42 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode * StringName native_base; if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, method_flags, &native_base)) { bool valid = p_function->is_static == method_flags.has_flag(METHOD_FLAG_STATIC); - valid = valid && parent_return_type == p_function->get_datatype(); + + if (p_function->return_type != nullptr) { + // Check return type covariance. + GDScriptParser::DataType return_type = p_function->get_datatype(); + if (return_type.is_variant()) { + // `is_type_compatible()` returns `true` if one of the types is `Variant`. + // Don't allow an explicitly specified `Variant` if the parent return type is narrower. + valid = valid && parent_return_type.is_variant(); + } else if (return_type.kind == GDScriptParser::DataType::BUILTIN && return_type.builtin_type == Variant::NIL) { + // `is_type_compatible()` returns `true` if target is an `Object` and source is `null`. + // Don't allow `void` if the parent return type is a hard non-`void` type. + if (parent_return_type.is_hard_type() && !(parent_return_type.kind == GDScriptParser::DataType::BUILTIN && parent_return_type.builtin_type == Variant::NIL)) { + valid = false; + } + } else { + valid = valid && is_type_compatible(parent_return_type, return_type); + } + } int par_count_diff = p_function->parameters.size() - parameters_types.size(); valid = valid && par_count_diff >= 0; valid = valid && default_value_count >= 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) { + int i = 0; + for (const GDScriptParser::DataType &parent_par_type : parameters_types) { + // Check parameter type contravariance. + GDScriptParser::DataType current_par_type = p_function->parameters[i++]->get_datatype(); + if (parent_par_type.is_variant() && parent_par_type.is_hard_type()) { + // `is_type_compatible()` returns `true` if one of the types is `Variant`. + // Don't allow narrowing a hard `Variant`. + valid = valid && current_par_type.is_variant(); + } else { + valid = valid && is_type_compatible(current_par_type, parent_par_type); + } + } } if (!valid) { diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.gd new file mode 100644 index 00000000000..fdf22f6843a --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.gd @@ -0,0 +1,10 @@ +class A: + func f(_p: Object): + pass + +class B extends A: + func f(_p: Node): + pass + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.out b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.out new file mode 100644 index 00000000000..c6a7e40e8cb --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_1.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +The function signature doesn't match the parent. Parent signature is "f(Object) -> Variant". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.gd new file mode 100644 index 00000000000..e4094f1d761 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.gd @@ -0,0 +1,10 @@ +class A: + func f(_p: Variant): + pass + +class B extends A: + func f(_p: Node): # No `is_type_compatible()` misuse. + pass + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.out b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.out new file mode 100644 index 00000000000..52a6efc6fca --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_2.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +The function signature doesn't match the parent. Parent signature is "f(Variant) -> Variant". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.gd new file mode 100644 index 00000000000..17663da4f60 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.gd @@ -0,0 +1,10 @@ +class A: + func f(_p: int): + pass + +class B extends A: + func f(_p: float): # No implicit conversion. + pass + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.out b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.out new file mode 100644 index 00000000000..7a6207fd45b --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_param_type_invalid_contravariance_3.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +The function signature doesn't match the parent. Parent signature is "f(int) -> Variant". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.gd new file mode 100644 index 00000000000..6dfa75ecbc2 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.gd @@ -0,0 +1,10 @@ +class A: + func f() -> Node: + return null + +class B extends A: + func f() -> Object: + return null + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.out b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.out new file mode 100644 index 00000000000..e680b2bd779 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_1.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +The function signature doesn't match the parent. Parent signature is "f() -> Node". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.gd new file mode 100644 index 00000000000..366494b94fb --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.gd @@ -0,0 +1,10 @@ +class A: + func f() -> Node: + return null + +class B extends A: + func f() -> Variant: # No `is_type_compatible()` misuse. + return null + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.out b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.out new file mode 100644 index 00000000000..e680b2bd779 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_2.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +The function signature doesn't match the parent. Parent signature is "f() -> Node". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.gd new file mode 100644 index 00000000000..2cb4e7c6161 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.gd @@ -0,0 +1,10 @@ +class A: + func f() -> Node: + return null + +class B extends A: + func f() -> void: # No `is_type_compatible()` misuse. + return + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.out b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.out new file mode 100644 index 00000000000..e680b2bd779 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_3.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +The function signature doesn't match the parent. Parent signature is "f() -> Node". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.gd b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.gd new file mode 100644 index 00000000000..2cabce46f5e --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.gd @@ -0,0 +1,10 @@ +class A: + func f() -> float: + return 0.0 + +class B extends A: + func f() -> int: # No implicit conversion. + return 0 + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.out b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.out new file mode 100644 index 00000000000..72f2c493d48 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/function_return_type_invalid_covariance_4.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +The function signature doesn't match the parent. Parent signature is "f() -> float". diff --git a/modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.gd b/modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.gd new file mode 100644 index 00000000000..a43c233625d --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.gd @@ -0,0 +1,20 @@ +class A: + func int_to_variant(_p: int): pass + func node_to_variant(_p: Node): pass + func node_2d_to_node(_p: Node2D): pass + + func variant_to_untyped(_p: Variant): pass + func int_to_untyped(_p: int): pass + func node_to_untyped(_p: Node): pass + +class B extends A: + func int_to_variant(_p: Variant): pass + func node_to_variant(_p: Variant): pass + func node_2d_to_node(_p: Node): pass + + func variant_to_untyped(_p): pass + func int_to_untyped(_p): pass + func node_to_untyped(_p): pass + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.out b/modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.out new file mode 100644 index 00000000000..d73c5eb7cde --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/function_param_type_contravariance.out @@ -0,0 +1 @@ +GDTEST_OK diff --git a/modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.gd b/modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.gd new file mode 100644 index 00000000000..4de50b67316 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.gd @@ -0,0 +1,32 @@ +class A: + func variant_to_int() -> Variant: return 0 + func variant_to_node() -> Variant: return null + func node_to_node_2d() -> Node: return null + + func untyped_to_void(): pass + func untyped_to_variant(): pass + func untyped_to_int(): pass + func untyped_to_node(): pass + + func void_to_untyped() -> void: pass + func variant_to_untyped() -> Variant: return null + func int_to_untyped() -> int: return 0 + func node_to_untyped() -> Node: return null + +class B extends A: + func variant_to_int() -> int: return 0 + func variant_to_node() -> Node: return null + func node_to_node_2d() -> Node2D: return null + + func untyped_to_void() -> void: pass + func untyped_to_variant() -> Variant: return null + func untyped_to_int() -> int: return 0 + func untyped_to_node() -> Node: return null + + func void_to_untyped(): pass + func variant_to_untyped(): pass + func int_to_untyped(): pass + func node_to_untyped(): pass + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.out b/modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.out new file mode 100644 index 00000000000..d73c5eb7cde --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/function_return_type_covariance.out @@ -0,0 +1 @@ +GDTEST_OK