Merge pull request #70733 from vonagam/fix-assigning-untyped

GDScript: Fix some issues with assignments that involve untyped things

6 files changed, 144 insertions, 81 deletions

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 8afcb431ec..0c858a36e7 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -1573,6 +1573,9 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
 			if (initializer_type.is_variant() || !initializer_type.is_hard_type()) {
 				mark_node_unsafe(p_assignable->initializer);
 				p_assignable->use_conversion_assign = true;
+				if (!initializer_type.is_variant() && !is_type_compatible(specified_type, initializer_type, true, p_assignable->initializer)) {
+					downgrade_node_type_source(p_assignable->initializer);
+				}
 			} else if (!is_type_compatible(specified_type, initializer_type, true, p_assignable->initializer)) {
 				if (!is_constant && is_type_compatible(initializer_type, specified_type, true, p_assignable->initializer)) {
 					mark_node_unsafe(p_assignable->initializer);
@@ -2097,84 +2100,86 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 
 	GDScriptParser::DataType assigned_value_type = p_assignment->assigned_value->get_datatype();
 
+	bool assignee_is_variant = assignee_type.is_variant();
+	bool assignee_is_hard = assignee_type.is_hard_type();
+	bool assigned_is_variant = assigned_value_type.is_variant();
+	bool assigned_is_hard = assigned_value_type.is_hard_type();
 	bool compatible = true;
+	bool downgrades_assignee = false;
+	bool downgrades_assigned = false;
 	GDScriptParser::DataType op_type = assigned_value_type;
 	if (p_assignment->operation != GDScriptParser::AssignmentNode::OP_NONE && !op_type.is_variant()) {
 		op_type = get_operation_type(p_assignment->variant_op, assignee_type, assigned_value_type, compatible, p_assignment->assigned_value);
+
+		if (assignee_is_variant) {
+			// variant assignee
+			mark_node_unsafe(p_assignment);
+		} else if (!compatible) {
+			// incompatible hard types and non-variant assignee
+			mark_node_unsafe(p_assignment);
+			if (assigned_is_variant) {
+				// incompatible hard non-variant assignee and hard variant assigned
+				p_assignment->use_conversion_assign = true;
+			} else {
+				// incompatible hard non-variant types
+				push_error(vformat(R"(Invalid operands "%s" and "%s" for assignment operator.)", assignee_type.to_string(), assigned_value_type.to_string()), p_assignment);
+			}
+		} else if (op_type.type_source == GDScriptParser::DataType::UNDETECTED && !assigned_is_variant) {
+			// incompatible non-variant types (at least one weak)
+			downgrades_assignee = !assignee_is_hard;
+			downgrades_assigned = !assigned_is_hard;
+		}
 	}
 	p_assignment->set_datatype(op_type);
 
-	// If Assignee is a variant, then you can assign anything
-	// When the assigned value has a known type, further checks are possible.
-	if (assignee_type.is_hard_type() && !assignee_type.is_variant() && op_type.is_hard_type() && !op_type.is_variant()) {
-		if (compatible) {
-			compatible = is_type_compatible(assignee_type, op_type, true, p_assignment->assigned_value);
-			if (!compatible) {
-				// Try reverse test since it can be a masked subtype.
-				if (!is_type_compatible(op_type, assignee_type, true)) {
-					push_error(vformat(R"(Value of type "%s" cannot be assigned to a variable of type "%s".)", assigned_value_type.to_string(), assignee_type.to_string()), p_assignment->assigned_value);
-				} else {
-					// TODO: Add warning.
-					mark_node_unsafe(p_assignment);
-					p_assignment->use_conversion_assign = true;
-				}
-			}
-		} else {
-			push_error(vformat(R"(Invalid operands "%s" and "%s" for assignment operator.)", assignee_type.to_string(), assigned_value_type.to_string()), p_assignment);
+	if (assignee_is_variant) {
+		if (!assignee_is_hard) {
+			// weak variant assignee
+			mark_node_unsafe(p_assignment);
 		}
-	} else if (assignee_type.is_hard_type() && !assignee_type.is_variant()) {
-		mark_node_unsafe(p_assignment);
-		p_assignment->use_conversion_assign = true;
 	} else {
-		mark_node_unsafe(p_assignment);
-		if (assignee_type.is_hard_type() && !assignee_type.is_variant()) {
+		if (assignee_is_hard && !assigned_is_hard) {
+			// hard non-variant assignee and weak assigned
+			mark_node_unsafe(p_assignment);
 			p_assignment->use_conversion_assign = true;
-		}
-	}
-
-	if (p_assignment->assignee->type == GDScriptParser::Node::IDENTIFIER) {
-		// Change source type so it's not wrongly detected later.
-		GDScriptParser::IdentifierNode *identifier = static_cast<GDScriptParser::IdentifierNode *>(p_assignment->assignee);
-
-		switch (identifier->source) {
-			case GDScriptParser::IdentifierNode::MEMBER_VARIABLE: {
-				GDScriptParser::DataType id_type = identifier->variable_source->get_datatype();
-				if (!id_type.is_hard_type()) {
-					id_type.kind = GDScriptParser::DataType::VARIANT;
-					id_type.type_source = GDScriptParser::DataType::UNDETECTED;
-					identifier->variable_source->set_datatype(id_type);
-				}
-			} break;
-			case GDScriptParser::IdentifierNode::FUNCTION_PARAMETER: {
-				GDScriptParser::DataType id_type = identifier->parameter_source->get_datatype();
-				if (!id_type.is_hard_type()) {
-					id_type.kind = GDScriptParser::DataType::VARIANT;
-					id_type.type_source = GDScriptParser::DataType::UNDETECTED;
-					identifier->parameter_source->set_datatype(id_type);
-				}
-			} break;
-			case GDScriptParser::IdentifierNode::LOCAL_VARIABLE: {
-				GDScriptParser::DataType id_type = identifier->variable_source->get_datatype();
-				if (!id_type.is_hard_type()) {
-					id_type.kind = GDScriptParser::DataType::VARIANT;
-					id_type.type_source = GDScriptParser::DataType::UNDETECTED;
-					identifier->variable_source->set_datatype(id_type);
+			downgrades_assigned = downgrades_assigned || (!assigned_is_variant && !is_type_compatible(assignee_type, op_type, true, p_assignment->assigned_value));
+		} else if (compatible) {
+			if (op_type.is_variant()) {
+				// non-variant assignee and variant result
+				mark_node_unsafe(p_assignment);
+				if (assignee_is_hard) {
+					// hard non-variant assignee and variant result
+					p_assignment->use_conversion_assign = true;
+				} else {
+					// weak non-variant assignee and variant result
+					downgrades_assignee = true;
 				}
-			} break;
-			case GDScriptParser::IdentifierNode::LOCAL_ITERATOR: {
-				GDScriptParser::DataType id_type = identifier->bind_source->get_datatype();
-				if (!id_type.is_hard_type()) {
-					id_type.kind = GDScriptParser::DataType::VARIANT;
-					id_type.type_source = GDScriptParser::DataType::UNDETECTED;
-					identifier->bind_source->set_datatype(id_type);
+			} else if (!is_type_compatible(assignee_type, op_type, assignee_is_hard, p_assignment->assigned_value)) {
+				// non-variant assignee and incompatible result
+				mark_node_unsafe(p_assignment);
+				if (assignee_is_hard) {
+					if (is_type_compatible(op_type, assignee_type, true, p_assignment->assigned_value)) {
+						// hard non-variant assignee and maybe compatible result
+						p_assignment->use_conversion_assign = true;
+					} else {
+						// hard non-variant assignee and incompatible result
+						push_error(vformat(R"(Value of type "%s" cannot be assigned to a variable of type "%s".)", assigned_value_type.to_string(), assignee_type.to_string()), p_assignment->assigned_value);
+					}
+				} else {
+					// weak non-variant assignee and incompatible result
+					downgrades_assignee = true;
 				}
-			} break;
-			default:
-				// Nothing to do.
-				break;
+			}
 		}
 	}
 
+	if (downgrades_assignee) {
+		downgrade_node_type_source(p_assignment->assignee);
+	}
+	if (downgrades_assigned) {
+		downgrade_node_type_source(p_assignment->assigned_value);
+	}
+
 #ifdef DEBUG_ENABLED
 	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);
@@ -2638,6 +2643,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 		} else {
 			reduce_expression(subscript->base);
 			base_type = subscript->base->get_datatype();
+			is_self = subscript->base->type == GDScriptParser::Node::SELF;
 		}
 	} else {
 		// Invalid call. Error already sent in parser.
@@ -4243,9 +4249,6 @@ GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator
 }
 
 GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid, const GDScriptParser::Node *p_source) {
-	GDScriptParser::DataType result;
-	result.kind = GDScriptParser::DataType::VARIANT;
-
 	Variant::Type a_type = p_a.builtin_type;
 	Variant::Type b_type = p_b.builtin_type;
 
@@ -4265,28 +4268,18 @@ GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator
 	}
 
 	Variant::ValidatedOperatorEvaluator op_eval = Variant::get_validated_operator_evaluator(p_operation, a_type, b_type);
-
 	bool hard_operation = p_a.is_hard_type() && p_b.is_hard_type();
 	bool validated = op_eval != nullptr;
 
-	if (hard_operation && !validated) {
-		r_valid = false;
-		return result;
-	} else if (hard_operation && validated) {
+	GDScriptParser::DataType result;
+	if (validated) {
 		r_valid = true;
-		result.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED;
+		result.type_source = hard_operation ? GDScriptParser::DataType::ANNOTATED_INFERRED : GDScriptParser::DataType::INFERRED;
 		result.kind = GDScriptParser::DataType::BUILTIN;
 		result.builtin_type = Variant::get_operator_return_type(p_operation, a_type, b_type);
-	} else if (!hard_operation && !validated) {
-		r_valid = true;
-		result.type_source = GDScriptParser::DataType::UNDETECTED;
+	} else {
+		r_valid = !hard_operation;
 		result.kind = GDScriptParser::DataType::VARIANT;
-		result.builtin_type = Variant::NIL;
-	} else if (!hard_operation && validated) {
-		r_valid = true;
-		result.type_source = GDScriptParser::DataType::INFERRED;
-		result.kind = GDScriptParser::DataType::BUILTIN;
-		result.builtin_type = Variant::get_operator_return_type(p_operation, a_type, b_type);
 	}
 
 	return result;
@@ -4462,6 +4455,46 @@ void GDScriptAnalyzer::mark_node_unsafe(const GDScriptParser::Node *p_node) {
 #endif
 }
 
+void GDScriptAnalyzer::downgrade_node_type_source(GDScriptParser::Node *p_node) {
+	GDScriptParser::IdentifierNode *identifier = nullptr;
+	if (p_node->type == GDScriptParser::Node::IDENTIFIER) {
+		identifier = static_cast<GDScriptParser::IdentifierNode *>(p_node);
+	} else if (p_node->type == GDScriptParser::Node::SUBSCRIPT) {
+		GDScriptParser::SubscriptNode *subscript = static_cast<GDScriptParser::SubscriptNode *>(p_node);
+		if (subscript->is_attribute) {
+			identifier = subscript->attribute;
+		}
+	}
+	if (identifier == nullptr) {
+		return;
+	}
+
+	GDScriptParser::Node *source = nullptr;
+	switch (identifier->source) {
+		case GDScriptParser::IdentifierNode::MEMBER_VARIABLE: {
+			source = identifier->variable_source;
+		} break;
+		case GDScriptParser::IdentifierNode::FUNCTION_PARAMETER: {
+			source = identifier->parameter_source;
+		} break;
+		case GDScriptParser::IdentifierNode::LOCAL_VARIABLE: {
+			source = identifier->variable_source;
+		} break;
+		case GDScriptParser::IdentifierNode::LOCAL_ITERATOR: {
+			source = identifier->bind_source;
+		} break;
+		default:
+			break;
+	}
+	if (source == nullptr) {
+		return;
+	}
+
+	GDScriptParser::DataType datatype;
+	datatype.kind = GDScriptParser::DataType::VARIANT;
+	source->set_datatype(datatype);
+}
+
 void GDScriptAnalyzer::mark_lambda_use_self() {
 	for (GDScriptParser::LambdaNode *lambda : lambda_stack) {
 		lambda->use_self = true;
diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h
index da7b7ddb75..b22d47982f 100644
--- a/modules/gdscript/gdscript_analyzer.h
+++ b/modules/gdscript/gdscript_analyzer.h
@@ -120,6 +120,7 @@ class GDScriptAnalyzer {
 	bool is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false, const GDScriptParser::Node *p_source_node = nullptr);
 	void push_error(const String &p_message, const GDScriptParser::Node *p_origin = nullptr);
 	void mark_node_unsafe(const GDScriptParser::Node *p_node);
+	void downgrade_node_type_source(GDScriptParser::Node *p_node);
 	void mark_lambda_use_self();
 	bool class_exists(const StringName &p_class) const;
 	Ref<GDScriptParserRef> get_parser_for(const String &p_path);
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.gd b/modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.gd
new file mode 100644
index 0000000000..664e364493
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.gd
@@ -0,0 +1,3 @@
+func test():
+	var foo: bool = true
+	foo += 'bar'
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.out b/modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.out
new file mode 100644
index 0000000000..358b096a64
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Invalid operands "bool" and "String" for assignment operator.
diff --git a/modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.gd b/modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.gd
new file mode 100644
index 0000000000..2d2c2bef19
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.gd
@@ -0,0 +1,19 @@
+func test():
+	var one_0 = 0
+	one_0 = 1
+	var one_1 := one_0
+	print(one_1)
+
+	var two: Variant = 0
+	two += 2
+	print(two)
+
+	var three_0: Variant = 1
+	var three_1: int = 2
+	three_0 += three_1
+	print(three_0)
+
+	var four_0: int = 3
+	var four_1: Variant = 1
+	four_0 += four_1
+	print(four_0)
diff --git a/modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.out b/modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.out
new file mode 100644
index 0000000000..7536c38490
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.out
@@ -0,0 +1,5 @@
+GDTEST_OK
+1
+2
+3
+4