Skip to content

Commit

Permalink
[Nullable] Allow the configuration of the detection of assigning the …
Browse files Browse the repository at this point in the history
…result of a function that returns void to a variable.
  • Loading branch information
Macksaur committed Jun 21, 2024
1 parent 6fb3b72 commit 520219d
Show file tree
Hide file tree
Showing 16 changed files with 98 additions and 39 deletions.
4 changes: 4 additions & 0 deletions doc/classes/ProjectSettings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,10 @@
<member name="debug/gdscript/warnings/return_value_discarded" type="int" setter="" getter="" default="0">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when calling a function without using its return value (by assigning it to a variable or using it as a function argument). These return values are sometimes used to indicate possible errors using the [enum Error] enum.
</member>
<member name="debug/gdscript/warnings/return_value_void" type="int" setter="" getter="" default="0">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when trying to assign the result of a function that returns void.
When set to [code]ignore[/code] neither the editor nor the debug export will detect this kind of assignment when running the game.
</member>
<member name="debug/gdscript/warnings/shadowed_global_identifier" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when defining a local or member variable, signal, or enum that would have the same name as a built-in function or global class name, thus shadowing it.
</member>
Expand Down
30 changes: 26 additions & 4 deletions modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1917,7 +1917,14 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
if (!initializer_type.is_set() || initializer_type.has_no_type() || !initializer_type.is_hard_type()) {
push_error(vformat(R"(Cannot infer the type of "%s" %s because the value doesn't have a set type.)", p_assignable->identifier->name, p_kind), p_assignable->initializer);
} else if (initializer_type.kind == GDScriptParser::DataType::BUILTIN && initializer_type.builtin_type == Variant::NIL && !is_constant) {
push_error(vformat(R"(Cannot infer the type of "%s" %s because the value is "null".)", p_assignable->identifier->name, p_kind), p_assignable->initializer);
#ifdef DEBUG_ENABLED
GDScriptWarning::WarnLevel return_value_void_warn_level = GDScriptWarning::get_project_warning_level(GDScriptWarning::RETURN_VALUE_VOID);
if (return_value_void_warn_level == GDScriptWarning::WarnLevel::IGNORE) {
parser->push_warning(p_assignable, GDScriptWarning::INFERENCE_ON_VARIANT, p_kind);
} else {
push_error(vformat(R"(Cannot infer the type of "%s" %s because the value is "null".)", p_assignable->identifier->name, p_kind), p_assignable->initializer);
}
#endif
}
#ifdef DEBUG_ENABLED
if (initializer_type.is_hard_type() && initializer_type.is_variant()) {
Expand Down Expand Up @@ -3182,7 +3189,12 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
MethodInfo function_info = GDScriptUtilityFunctions::get_function_info(function_name);

if (!p_is_root && !p_is_await && function_info.return_val.type == Variant::NIL && ((function_info.return_val.usage & PROPERTY_USAGE_NIL_IS_VARIANT) == 0)) {
push_error(vformat(R"*(Cannot get return value of call to "%s()" because it returns "void".)*", function_name), p_call);
#ifdef DEBUG_ENABLED
GDScriptWarning::WarnLevel return_value_void_warn_level = GDScriptWarning::get_project_warning_level(GDScriptWarning::RETURN_VALUE_VOID);
if (return_value_void_warn_level != GDScriptWarning::WarnLevel::IGNORE) {
parser->push_warning(p_call, GDScriptWarning::RETURN_VALUE_VOID, function_name);
}
#endif
}

if (all_is_constant && GDScriptUtilityFunctions::is_function_constant(function_name)) {
Expand Down Expand Up @@ -3233,7 +3245,12 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
MethodInfo function_info = info_from_utility_func(function_name);

if (!p_is_root && !p_is_await && function_info.return_val.type == Variant::NIL && ((function_info.return_val.usage & PROPERTY_USAGE_NIL_IS_VARIANT) == 0)) {
push_error(vformat(R"*(Cannot get return value of call to "%s()" because it returns "void".)*", function_name), p_call);
#ifdef DEBUG_ENABLED
GDScriptWarning::WarnLevel return_value_void_warn_level = GDScriptWarning::get_project_warning_level(GDScriptWarning::RETURN_VALUE_VOID);
if (return_value_void_warn_level != GDScriptWarning::WarnLevel::IGNORE) {
parser->push_warning(p_call, GDScriptWarning::RETURN_VALUE_VOID, function_name);
}
#endif
}

if (all_is_constant && Variant::get_utility_function_type(function_name) == Variant::UTILITY_FUNC_TYPE_MATH) {
Expand Down Expand Up @@ -3395,7 +3412,12 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
}

if (!p_is_root && !p_is_await && return_type.is_hard_type() && return_type.kind == GDScriptParser::DataType::BUILTIN && return_type.builtin_type == Variant::NIL) {
push_error(vformat(R"*(Cannot get return value of call to "%s()" because it returns "void".)*", p_call->function_name), p_call);
#ifdef DEBUG_ENABLED
GDScriptWarning::WarnLevel return_value_void_warn_level = GDScriptWarning::get_project_warning_level(GDScriptWarning::RETURN_VALUE_VOID);
if (return_value_void_warn_level != GDScriptWarning::WarnLevel::IGNORE) {
parser->push_warning(p_call, GDScriptWarning::RETURN_VALUE_VOID, p_call->function_name);
}
#endif
}

#ifdef DEBUG_ENABLED
Expand Down
4 changes: 3 additions & 1 deletion modules/gdscript/gdscript_vm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@

#ifdef DEBUG_ENABLED

#include "gdscript_warning.h"

static bool _profile_count_as_native(const Object *p_base_obj, const StringName &p_methodname) {
if (!p_base_obj) {
return false;
Expand Down Expand Up @@ -1736,7 +1738,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
GET_INSTRUCTION_ARG(ret, argc + 1);
base->callp(*methodname, (const Variant **)argptrs, argc, *ret, err);
#ifdef DEBUG_ENABLED
if (ret->get_type() == Variant::NIL) {
if (ret->get_type() == Variant::NIL && GDScriptWarning::get_project_warning_level(GDScriptWarning::RETURN_VALUE_VOID) == GDScriptWarning::ERROR) {
if (base_type == Variant::OBJECT) {
if (base_obj) {
MethodBind *method = ClassDB::get_method(base_class, *methodname);
Expand Down
9 changes: 9 additions & 0 deletions modules/gdscript/gdscript_warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@

#ifdef DEBUG_ENABLED

#include "core/config/project_settings.h"

String GDScriptWarning::get_message() const {
#define CHECK_SYMBOLS(m_amount) ERR_FAIL_COND_V(symbols.size() < m_amount, String());

Expand Down Expand Up @@ -163,6 +165,8 @@ String GDScriptWarning::get_message() const {
case FUNCTION_USED_AS_PROPERTY: // This is valid, returns `Callable`.
break;
#endif
case RETURN_VALUE_VOID:
return vformat(R"*(Cannot get return value of call to "%s()" because it returns "void".)*", symbols[0]);
case WARNING_MAX:
break; // Can't happen, but silences warning.
}
Expand Down Expand Up @@ -240,6 +244,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
"CONSTANT_USED_AS_FUNCTION",
"FUNCTION_USED_AS_PROPERTY",
#endif
"RETURN_VALUE_VOID",
};

static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names.");
Expand All @@ -261,4 +266,8 @@ GDScriptWarning::Code GDScriptWarning::get_code_from_name(const String &p_name)
return WARNING_MAX;
}

GDScriptWarning::WarnLevel GDScriptWarning::get_project_warning_level(Code p_code) {
return (GDScriptWarning::WarnLevel)(int)GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(p_code));
}

#endif // DEBUG_ENABLED
3 changes: 3 additions & 0 deletions modules/gdscript/gdscript_warning.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class GDScriptWarning {
CONSTANT_USED_AS_FUNCTION, // Function not found, but there's a constant with the same name.
FUNCTION_USED_AS_PROPERTY, // Property not found, but there's a function with the same name.
#endif
RETURN_VALUE_VOID, // Functions that return void cannot have their return value stored.
WARNING_MAX,
};

Expand Down Expand Up @@ -146,6 +147,7 @@ class GDScriptWarning {
WARN, // CONSTANT_USED_AS_FUNCTION
WARN, // FUNCTION_USED_AS_PROPERTY
#endif
IGNORE, // RETURN_VALUE_VOID
};

static_assert((sizeof(default_warning_levels) / sizeof(default_warning_levels[0])) == WARNING_MAX, "Amount of default levels does not match the amount of warnings.");
Expand All @@ -162,6 +164,7 @@ class GDScriptWarning {
static String get_name_from_code(Code p_code);
static String get_settings_path_from_code(Code p_code);
static Code get_code_from_name(const String &p_name);
static GDScriptWarning::WarnLevel get_project_warning_level(Code p_code);
};

#endif // DEBUG_ENABLED
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
GDTEST_ANALYZER_ERROR
Cannot get return value of call to "reverse()" because it returns "void".
GDTEST_OK
>> WARNING
>> Line: 3
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "reverse()" because it returns "void".
<null>
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
GDTEST_ANALYZER_ERROR
Cannot get return value of call to "foo()" because it returns "void".
GDTEST_OK
>> WARNING
>> Line: 5
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "foo()" because it returns "void".
<null>
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
GDTEST_ANALYZER_ERROR
Cannot get return value of call to "print_debug()" because it returns "void".
GDTEST_OK
>> WARNING
>> Line: 2
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "print_debug()" because it returns "void".

<null>
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
GDTEST_ANALYZER_ERROR
Cannot get return value of call to "free()" because it returns "void".
GDTEST_OK
>> WARNING
>> Line: 3
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "free()" because it returns "void".
<null>
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
GDTEST_ANALYZER_ERROR
Cannot get return value of call to "print()" because it returns "void".
GDTEST_OK
>> WARNING
>> Line: 2
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "print()" because it returns "void".

<null>
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
func test():
var obj
obj = Node.new()
var obj := Node.new()
print(obj.free())
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
GDTEST_RUNTIME_ERROR
>> SCRIPT ERROR
>> on function: test()
>> runtime/errors/use_return_value_of_free_call.gd
>> 4
>> Trying to get a return value of a method that returns "void"
GDTEST_OK
>> WARNING
>> Line: 3
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "free()" because it returns "void".
<null>
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
func test():
var value
value = []
var value := []
print(value.reverse())
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
GDTEST_RUNTIME_ERROR
>> SCRIPT ERROR
>> on function: test()
>> runtime/errors/use_return_value_of_void_builtin_method_call.gd
>> 4
>> Trying to get a return value of a method that returns "void"
GDTEST_OK
>> WARNING
>> Line: 3
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "reverse()" because it returns "void".
<null>
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
func test():
var obj
obj = RefCounted.new()
var obj := RefCounted.new()
print(obj.notify_property_list_changed())
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
GDTEST_RUNTIME_ERROR
>> SCRIPT ERROR
>> on function: test()
>> runtime/errors/use_return_value_of_void_native_method_call.gd
>> 4
>> Trying to get a return value of a method that returns "void"
GDTEST_OK
>> WARNING
>> Line: 3
>> RETURN_VALUE_VOID
>> Cannot get return value of call to "notify_property_list_changed()" because it returns "void".
<null>

0 comments on commit 520219d

Please sign in to comment.