Merge pull request #90442 from vnen/gdscript-dont-warn-using-default-builtin

GDScript: Don't warn on unassigned for builtin-typed variables

modules/gdscript/gdscript_analyzer.cpp

@@ -1973,8 +1973,6 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable
 	if (p_is_local) {
 		if (p_variable->usages == 0 && !String(p_variable->identifier->name).begins_with("_")) {
 			parser->push_warning(p_variable, GDScriptWarning::UNUSED_VARIABLE, p_variable->identifier->name);
-		} else if (p_variable->assignments == 0) {
-			parser->push_warning(p_variable, GDScriptWarning::UNASSIGNED_VARIABLE, p_variable->identifier->name);
 		}
 	}
 	is_shadowing(p_variable->identifier, kind, p_is_local);
@@ -2615,9 +2613,21 @@ void GDScriptAnalyzer::update_array_literal_element_type(GDScriptParser::ArrayNo
 }
 
 void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assignment) {
-	reduce_expression(p_assignment->assignee);
 	reduce_expression(p_assignment->assigned_value);
 
+#ifdef DEBUG_ENABLED
+	// Increment assignment count for local variables.
+	// Before we reduce the assignee because we don't want to warn about not being assigned when performing the assignment.
+	if (p_assignment->assignee->type == GDScriptParser::Node::IDENTIFIER) {
+		GDScriptParser::IdentifierNode *id = static_cast<GDScriptParser::IdentifierNode *>(p_assignment->assignee);
+		if (id->source == GDScriptParser::IdentifierNode::LOCAL_VARIABLE && id->variable_source) {
+			id->variable_source->assignments++;
+		}
+	}
+#endif
+
+	reduce_expression(p_assignment->assignee);
+
 	if (p_assignment->assigned_value == nullptr || p_assignment->assignee == nullptr) {
 		return;
 	}
@@ -2754,6 +2764,14 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 	if (assignee_type.is_hard_type() && assignee_type.builtin_type == Variant::INT && assigned_value_type.builtin_type == Variant::FLOAT) {
 		parser->push_warning(p_assignment->assigned_value, GDScriptWarning::NARROWING_CONVERSION);
 	}
+	// Check for assignment with operation before assignment.
+	if (p_assignment->operation != GDScriptParser::AssignmentNode::OP_NONE && p_assignment->assignee->type == GDScriptParser::Node::IDENTIFIER) {
+		GDScriptParser::IdentifierNode *id = static_cast<GDScriptParser::IdentifierNode *>(p_assignment->assignee);
+		// Use == 1 here because this assignment was already counted in the beginning of the function.
+		if (id->source == GDScriptParser::IdentifierNode::LOCAL_VARIABLE && id->variable_source && id->variable_source->assignments == 1) {
+			parser->push_warning(p_assignment, GDScriptWarning::UNASSIGNED_VARIABLE_OP_ASSIGN, id->name);
+		}
+	}
 #endif
 }
 
@@ -3924,6 +3942,11 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
 		case GDScriptParser::IdentifierNode::LOCAL_VARIABLE:
 			p_identifier->set_datatype(p_identifier->variable_source->get_datatype());
 			found_source = true;
+#ifdef DEBUG_ENABLED
+			if (p_identifier->variable_source && p_identifier->variable_source->assignments == 0 && !(p_identifier->get_datatype().is_hard_type() && p_identifier->get_datatype().kind == GDScriptParser::DataType::BUILTIN)) {
+				parser->push_warning(p_identifier, GDScriptWarning::UNASSIGNED_VARIABLE, p_identifier->name);
+			}
+#endif
 			break;
 		case GDScriptParser::IdentifierNode::LOCAL_ITERATOR:
 			p_identifier->set_datatype(p_identifier->bind_source->get_datatype());

modules/gdscript/gdscript_parser.cpp

@@ -2769,10 +2769,6 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
 		return parse_expression(false); // Return the following expression.
 	}
 
-#ifdef DEBUG_ENABLED
-	VariableNode *source_variable = nullptr;
-#endif
-
 	switch (p_previous_operand->type) {
 		case Node::IDENTIFIER: {
 #ifdef DEBUG_ENABLED
@@ -2781,8 +2777,6 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
 			IdentifierNode *id = static_cast<IdentifierNode *>(p_previous_operand);
 			switch (id->source) {
 				case IdentifierNode::LOCAL_VARIABLE:
-
-					source_variable = id->variable_source;
 					id->variable_source->usages--;
 					break;
 				case IdentifierNode::LOCAL_CONSTANT:
@@ -2813,16 +2807,10 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
 	update_extents(assignment);
 
 	make_completion_context(COMPLETION_ASSIGN, assignment);
-#ifdef DEBUG_ENABLED
-	bool has_operator = true;
-#endif
 	switch (previous.type) {
 		case GDScriptTokenizer::Token::EQUAL:
 			assignment->operation = AssignmentNode::OP_NONE;
 			assignment->variant_op = Variant::OP_MAX;
-#ifdef DEBUG_ENABLED
-			has_operator = false;
-#endif
 			break;
 		case GDScriptTokenizer::Token::PLUS_EQUAL:
 			assignment->operation = AssignmentNode::OP_ADDITION;
@@ -2878,16 +2866,6 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
 	}
 	complete_extents(assignment);
 
-#ifdef DEBUG_ENABLED
-	if (source_variable != nullptr) {
-		if (has_operator && source_variable->assignments == 0) {
-			push_warning(assignment, GDScriptWarning::UNASSIGNED_VARIABLE_OP_ASSIGN, source_variable->identifier->name);
-		}
-
-		source_variable->assignments += 1;
-	}
-#endif
-
 	return assignment;
 }
 

modules/gdscript/gdscript_warning.cpp

@@ -40,7 +40,7 @@ String GDScriptWarning::get_message() const {
 	switch (code) {
 		case UNASSIGNED_VARIABLE:
 			CHECK_SYMBOLS(1);
-			return vformat(R"(The variable "%s" was used but never assigned a value.)", symbols[0]);
+			return vformat(R"(The variable "%s" was used before being assigned a value.)", symbols[0]);
 		case UNASSIGNED_VARIABLE_OP_ASSIGN:
 			CHECK_SYMBOLS(1);
 			return vformat(R"(Using assignment with operation but the variable "%s" was not previously assigned a value.)", symbols[0]);

modules/gdscript/tests/scripts/analyzer/features/enum_typecheck_inner_class.gd

@@ -11,6 +11,7 @@ class InnerClass:
 		var e2: InnerClass.MyEnum
 		var e3: EnumTypecheckOuterClass.InnerClass.MyEnum
 
+		@warning_ignore("unassigned_variable")
 		print("Self ", e1, e2, e3)
 		e1 = MyEnum.V1
 		e2 = MyEnum.V1
@@ -48,6 +49,7 @@ func test_outer_from_outer():
 	var e1: MyEnum
 	var e2: EnumTypecheckOuterClass.MyEnum
 
+	@warning_ignore("unassigned_variable")
 	print("Self ", e1, e2)
 	e1 = MyEnum.V1
 	e2 = MyEnum.V1
@@ -66,6 +68,7 @@ func test_inner_from_outer():
 	var e1: InnerClass.MyEnum
 	var e2: EnumTypecheckOuterClass.InnerClass.MyEnum
 
+	@warning_ignore("unassigned_variable")
 	print("Inner ", e1, e2)
 	e1 = InnerClass.MyEnum.V1
 	e2 = InnerClass.MyEnum.V1

modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.gd

@@ -0,0 +1,7 @@
+# GH-88117, GH-85796
+
+func test():
+	var array: Array
+	# Should not emit unassigned warning because the Array type has a default value.
+	array.assign([1, 2, 3])
+	print(array)

modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.out

@@ -0,0 +1,2 @@
+GDTEST_OK
+[1, 2, 3]

modules/gdscript/tests/scripts/analyzer/features/warning_ignore_warnings.gd

@@ -31,8 +31,8 @@ func int_func() -> int:
 func test_warnings(unused_private_class_variable):
 	var t = 1
 
-	@warning_ignore("unassigned_variable")
 	var unassigned_variable
+	@warning_ignore("unassigned_variable")
 	print(unassigned_variable)
 
 	var _unassigned_variable_op_assign

modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.gd

@@ -1,2 +1,11 @@
 func test():
-	var __
+	var unassigned
+	print(unassigned)
+	unassigned = "something" # Assigned only after use.
+
+	var a
+	print(a) # Unassigned, warn.
+	if a: # Still unassigned, warn.
+		a = 1
+		print(a) # Assigned (dead code), don't warn.
+	print(a) # "Maybe" assigned, don't warn.

modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.out

@@ -1,5 +1,16 @@
 GDTEST_OK
 >> WARNING
->> Line: 2
+>> Line: 3
 >> UNASSIGNED_VARIABLE
->> The variable "__" was used but never assigned a value.
+>> The variable "unassigned" was used before being assigned a value.
+>> WARNING
+>> Line: 7
+>> UNASSIGNED_VARIABLE
+>> The variable "a" was used before being assigned a value.
+>> WARNING
+>> Line: 8
+>> UNASSIGNED_VARIABLE
+>> The variable "a" was used before being assigned a value.
+<null>
+<null>
+<null>

modules/gdscript/tests/scripts/runtime/features/reset_unassigned_variables_in_loops.gd

@@ -7,6 +7,7 @@ func test():
 			var b
 			if true:
 				var c
+				@warning_ignore("unassigned_variable")
 				prints("Begin:", i, a, b, c)
 				a = 1
 				b = 1
@@ -20,6 +21,7 @@ func test():
 			var b
 			if true:
 				var c
+				@warning_ignore("unassigned_variable")
 				prints("Begin:", j, a, b, c)
 				a = 1
 				b = 1