From a47d4d57cac7077c9ad1c63f27ea095e6ce73561 Mon Sep 17 00:00:00 2001 From: George Marques Date: Fri, 27 Jan 2023 17:54:07 -0300 Subject: [PATCH] GDScript: Allow void functions to return calls to other void functions --- doc/classes/ProjectSettings.xml | 3 ++ modules/gdscript/gdscript_analyzer.cpp | 41 +++++++++++++++---- modules/gdscript/gdscript_compiler.cpp | 7 +++- modules/gdscript/gdscript_parser.h | 1 + modules/gdscript/gdscript_warning.cpp | 5 +++ modules/gdscript/gdscript_warning.h | 1 + .../allow_void_function_to_return_void.gd | 20 +++++++++ .../allow_void_function_to_return_void.out | 10 +++++ 8 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.out diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index f1ada58e0d6..fb46705f77e 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -474,6 +474,9 @@ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when accessing a property whose presence is not guaranteed at compile-time in the class. + + When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when returning a call from a [code]void[/code] function when such call cannot be guaranteed to be also [code]void[/code]. + When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a local constant is never used. diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index f788236af30..0dafb5d98ff 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1972,17 +1972,40 @@ void GDScriptAnalyzer::resolve_return(GDScriptParser::ReturnNode *p_return) { } if (p_return->return_value != nullptr) { - reduce_expression(p_return->return_value); - if (p_return->return_value->type == GDScriptParser::Node::ARRAY && has_expected_type && expected_type.has_container_element_type()) { - update_array_literal_element_type(static_cast(p_return->return_value), expected_type.get_container_element_type()); + bool is_void_function = has_expected_type && expected_type.is_hard_type() && expected_type.kind == GDScriptParser::DataType::BUILTIN && expected_type.builtin_type == Variant::NIL; + bool is_call = p_return->return_value->type == GDScriptParser::Node::CALL; + if (is_void_function && is_call) { + // Pretend the call is a root expression to allow those that are "void". + reduce_call(static_cast(p_return->return_value), false, true); + } else { + reduce_expression(p_return->return_value); } - if (has_expected_type && expected_type.is_hard_type() && expected_type.kind == GDScriptParser::DataType::BUILTIN && expected_type.builtin_type == Variant::NIL) { - push_error("A void function cannot return a value.", p_return); + if (is_void_function) { + p_return->void_return = true; + const GDScriptParser::DataType &return_type = p_return->return_value->datatype; + if (is_call && !return_type.is_hard_type()) { + String function_name = parser->current_function->identifier ? parser->current_function->identifier->name.operator String() : String(""); + String called_function_name = static_cast(p_return->return_value)->function_name.operator String(); +#ifdef DEBUG_ENABLED + parser->push_warning(p_return, GDScriptWarning::UNSAFE_VOID_RETURN, function_name, called_function_name); +#endif + mark_node_unsafe(p_return); + } else if (!is_call) { + push_error("A void function cannot return a value.", p_return); + } + result.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT; + result.kind = GDScriptParser::DataType::BUILTIN; + result.builtin_type = Variant::NIL; + result.is_constant = true; + } else { + if (p_return->return_value->type == GDScriptParser::Node::ARRAY && has_expected_type && expected_type.has_container_element_type()) { + update_array_literal_element_type(static_cast(p_return->return_value), expected_type.get_container_element_type()); + } + if (has_expected_type && expected_type.is_hard_type() && p_return->return_value->is_constant) { + update_const_expression_builtin_type(p_return->return_value, expected_type, "return"); + } + result = p_return->return_value->get_datatype(); } - if (has_expected_type && expected_type.is_hard_type() && p_return->return_value->is_constant) { - update_const_expression_builtin_type(p_return->return_value, expected_type, "return"); - } - result = p_return->return_value->get_datatype(); } else { // Return type is null by default. result.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT; diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 373d8a3efd4..46cd4b0d55a 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -1859,7 +1859,12 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui } } - gen->write_return(return_value); + if (return_n->void_return) { + // Always return "null", even if the expression is a call to a void function. + gen->write_return(codegen.add_constant(Variant())); + } else { + gen->write_return(return_value); + } if (return_value.mode == GDScriptCodeGenerator::Address::TEMPORARY) { codegen.generator->pop_temporary(); } diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index bb4549c15c6..5a83aeb9462 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -970,6 +970,7 @@ public: struct ReturnNode : public Node { ExpressionNode *return_value = nullptr; + bool void_return = false; ReturnNode() { type = RETURN; diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp index 024fed85170..6aecb88c011 100644 --- a/modules/gdscript/gdscript_warning.cpp +++ b/modules/gdscript/gdscript_warning.cpp @@ -125,6 +125,10 @@ String GDScriptWarning::get_message() const { CHECK_SYMBOLS(4); return "The argument '" + symbols[0] + "' of the function '" + symbols[1] + "' requires a the subtype '" + symbols[2] + "' but the supertype '" + symbols[3] + "' was provided"; } break; + case UNSAFE_VOID_RETURN: { + CHECK_SYMBOLS(2); + return "The method '" + symbols[0] + "()' returns 'void' but it's trying to return a call to '" + symbols[1] + "()' that can't be ensured to also be 'void'."; + } break; case DEPRECATED_KEYWORD: { CHECK_SYMBOLS(2); return "The '" + symbols[0] + "' keyword is deprecated and will be removed in a future release, please replace its uses by '" + symbols[1] + "'."; @@ -218,6 +222,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) { "UNSAFE_METHOD_ACCESS", "UNSAFE_CAST", "UNSAFE_CALL_ARGUMENT", + "UNSAFE_VOID_RETURN", "DEPRECATED_KEYWORD", "STANDALONE_TERNARY", "ASSERT_ALWAYS_TRUE", diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h index 7492972c1ac..07c5b7da976 100644 --- a/modules/gdscript/gdscript_warning.h +++ b/modules/gdscript/gdscript_warning.h @@ -69,6 +69,7 @@ public: UNSAFE_METHOD_ACCESS, // Function not found in the detected type (but can be in subtypes). UNSAFE_CAST, // Cast used in an unknown type. UNSAFE_CALL_ARGUMENT, // Function call argument is of a supertype of the require argument. + UNSAFE_VOID_RETURN, // Function returns void but returned a call to a function that can't be type checked. DEPRECATED_KEYWORD, // The keyword is deprecated and should be replaced. STANDALONE_TERNARY, // Return value of ternary expression is discarded. ASSERT_ALWAYS_TRUE, // Expression for assert argument is always true. diff --git a/modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.gd b/modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.gd new file mode 100644 index 00000000000..df89137f40a --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.gd @@ -0,0 +1,20 @@ +func test(): + return_call() + return_nothing() + return_side_effect() + var r = return_side_effect.call() # Untyped call to check return value. + prints(r, typeof(r) == TYPE_NIL) + print("end") + +func side_effect(v): + print("effect") + return v + +func return_call() -> void: + return print("hello") + +func return_nothing() -> void: + return + +func return_side_effect() -> void: + return side_effect("x") diff --git a/modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.out b/modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.out new file mode 100644 index 00000000000..7c0416371f1 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/allow_void_function_to_return_void.out @@ -0,0 +1,10 @@ +GDTEST_OK +>> WARNING +>> Line: 20 +>> UNSAFE_VOID_RETURN +>> The method 'return_side_effect()' returns 'void' but it's trying to return a call to 'side_effect()' that can't be ensured to also be 'void'. +hello +effect +effect + true +end