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

If the type of a variable is a built-in Variant type, then it will
automatically be assigned a default value based on the type. This means
that the explicit initialization may be unnecessary. Thus this commit
removes the warning in such case.

This also changes the meaning of the unassigned warning to happen when
the variable is used before being assigned, not when it has zero
assignments.

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
 }
 
@@ -3926,6 +3944,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