Merge pull request #67361 from clayjohn/GDScript-unused-return-warning

Implement RETURN_VALUE_DISCARDED warning in GDscript

4 files changed, 18 insertions, 8 deletions

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index e1beb2f374..b3ca8ff00a 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -1041,7 +1041,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class) {
 	}
 }
 
-void GDScriptAnalyzer::resolve_node(GDScriptParser::Node *p_node) {
+void GDScriptAnalyzer::resolve_node(GDScriptParser::Node *p_node, bool p_is_root) {
 	ERR_FAIL_COND_MSG(p_node == nullptr, "Trying to resolve type of a null node.");
 
 	switch (p_node->type) {
@@ -1114,7 +1114,7 @@ void GDScriptAnalyzer::resolve_node(GDScriptParser::Node *p_node) {
 		case GDScriptParser::Node::SUBSCRIPT:
 		case GDScriptParser::Node::TERNARY_OPERATOR:
 		case GDScriptParser::Node::UNARY_OPERATOR:
-			reduce_expression(static_cast<GDScriptParser::ExpressionNode *>(p_node), true);
+			reduce_expression(static_cast<GDScriptParser::ExpressionNode *>(p_node), p_is_root);
 			break;
 		case GDScriptParser::Node::BREAK:
 		case GDScriptParser::Node::BREAKPOINT:
@@ -1411,7 +1411,7 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
 		variable_type.builtin_type = Variant::INT; // Can this ever be a float or something else?
 		p_for->variable->set_datatype(variable_type);
 	} else if (p_for->list) {
-		resolve_node(p_for->list);
+		resolve_node(p_for->list, false);
 		if (p_for->list->datatype.has_container_element_type()) {
 			variable_type = p_for->list->datatype.get_container_element_type();
 			variable_type.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED;
@@ -1439,7 +1439,7 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
 }
 
 void GDScriptAnalyzer::resolve_while(GDScriptParser::WhileNode *p_while) {
-	resolve_node(p_while->condition);
+	resolve_node(p_while->condition, false);
 
 	resolve_suite(p_while->loop);
 	p_while->set_datatype(p_while->loop->get_datatype());
@@ -1824,7 +1824,7 @@ void GDScriptAnalyzer::reduce_expression(GDScriptParser::ExpressionNode *p_expre
 			reduce_binary_op(static_cast<GDScriptParser::BinaryOpNode *>(p_expression));
 			break;
 		case GDScriptParser::Node::CALL:
-			reduce_call(static_cast<GDScriptParser::CallNode *>(p_expression), p_is_root);
+			reduce_call(static_cast<GDScriptParser::CallNode *>(p_expression), false, p_is_root);
 			break;
 		case GDScriptParser::Node::CAST:
 			reduce_cast(static_cast<GDScriptParser::CastNode *>(p_expression));
@@ -2548,6 +2548,12 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 			mark_lambda_use_self();
 		}
 
+#ifdef DEBUG_ENABLED
+		if (p_is_root && return_type.kind != GDScriptParser::DataType::UNRESOLVED && return_type.builtin_type != Variant::NIL) {
+			parser->push_warning(p_call, GDScriptWarning::RETURN_VALUE_DISCARDED, p_call->function_name);
+		}
+#endif // DEBUG_ENABLED
+
 		call_type = return_type;
 	} else {
 		bool found = false;
diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h
index 3966b81b6e..217a856ce0 100644
--- a/modules/gdscript/gdscript_analyzer.h
+++ b/modules/gdscript/gdscript_analyzer.h
@@ -63,7 +63,7 @@ class GDScriptAnalyzer {
 	void resolve_class_body(GDScriptParser::ClassNode *p_class);
 	void resolve_function_signature(GDScriptParser::FunctionNode *p_function);
 	void resolve_function_body(GDScriptParser::FunctionNode *p_function);
-	void resolve_node(GDScriptParser::Node *p_node);
+	void resolve_node(GDScriptParser::Node *p_node, bool p_is_root = true);
 	void resolve_suite(GDScriptParser::SuiteNode *p_suite);
 	void resolve_if(GDScriptParser::IfNode *p_if);
 	void resolve_for(GDScriptParser::ForNode *p_for);
diff --git a/modules/gdscript/tests/scripts/parser/features/class.gd b/modules/gdscript/tests/scripts/parser/features/class.gd
index 6652f85ad9..af24b32322 100644
--- a/modules/gdscript/tests/scripts/parser/features/class.gd
+++ b/modules/gdscript/tests/scripts/parser/features/class.gd
@@ -21,5 +21,5 @@ func test():
 	assert(test_sub.number == 25)  # From Test.
 	assert(test_sub.other_string == "bye")  # From TestSub.
 
-	TestConstructor.new()
-	TestConstructor.new(500)
+	var _test_constructor = TestConstructor.new()
+	_test_constructor = TestConstructor.new(500)
diff --git a/modules/gdscript/tests/scripts/parser/warnings/return_value_discarded.out b/modules/gdscript/tests/scripts/parser/warnings/return_value_discarded.out
index d73c5eb7cd..13f759dd46 100644
--- a/modules/gdscript/tests/scripts/parser/warnings/return_value_discarded.out
+++ b/modules/gdscript/tests/scripts/parser/warnings/return_value_discarded.out
@@ -1 +1,5 @@
 GDTEST_OK
+>> WARNING
+>> Line: 6
+>> RETURN_VALUE_DISCARDED
+>> The function 'i_return_int()' returns a value, but this value is never used.