Merge pull request #80247 from dalexeev/gds-for-loop-var-static-typing

GDScript: Add static typing for `for` loop variable

doc/classes/ProjectSettings.xml

@@ -492,6 +492,9 @@
 		<member name="debug/gdscript/warnings/redundant_await" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a function that is not a coroutine is called with await.
 		</member>
+		<member name="debug/gdscript/warnings/redundant_for_variable_type" type="int" setter="" getter="" default="1">
+			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a [code]for[/code] variable type specifier is a supertype of the inferred type.
+		</member>
 		<member name="debug/gdscript/warnings/redundant_static_unload" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when the [code]@static_unload[/code] annotation is used in a script without any static variables.
 		</member>

modules/gdscript/gdscript_analyzer.cpp

@@ -2001,13 +2001,16 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
 	}
 
 	GDScriptParser::DataType variable_type;
+	String list_visible_type = "<unresolved type>";
 	if (list_resolved) {
 		variable_type.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED;
 		variable_type.kind = GDScriptParser::DataType::BUILTIN;
 		variable_type.builtin_type = Variant::INT;
+		list_visible_type = "Array[int]"; // NOTE: `range()` has `Array` return type.
 	} else if (p_for->list) {
 		resolve_node(p_for->list, false);
 		GDScriptParser::DataType list_type = p_for->list->get_datatype();
+		list_visible_type = list_type.to_string();
 		if (!list_type.is_hard_type()) {
 			mark_node_unsafe(p_for->list);
 		}
@@ -2051,9 +2054,38 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
 			push_error(vformat(R"(Unable to iterate on value of type "%s".)", list_type.to_string()), p_for->list);
 		}
 	}
+
 	if (p_for->variable) {
+		if (p_for->datatype_specifier) {
+			GDScriptParser::DataType specified_type = type_from_metatype(resolve_datatype(p_for->datatype_specifier));
+			if (!specified_type.is_variant()) {
+				if (variable_type.is_variant() || !variable_type.is_hard_type()) {
+					mark_node_unsafe(p_for->variable);
+					p_for->use_conversion_assign = true;
+				} else if (!is_type_compatible(specified_type, variable_type, true, p_for->variable)) {
+					if (is_type_compatible(variable_type, specified_type)) {
+						mark_node_unsafe(p_for->variable);
+						p_for->use_conversion_assign = true;
+					} else {
+						push_error(vformat(R"(Unable to iterate on value of type "%s" with variable of type "%s".)", list_visible_type, specified_type.to_string()), p_for->datatype_specifier);
+					}
+				} else if (!is_type_compatible(specified_type, variable_type)) {
+					p_for->use_conversion_assign = true;
+#ifdef DEBUG_ENABLED
+				} else {
+					parser->push_warning(p_for->datatype_specifier, GDScriptWarning::REDUNDANT_FOR_VARIABLE_TYPE, p_for->variable->name, variable_type.to_string(), specified_type.to_string());
+#endif
+				}
+#ifdef DEBUG_ENABLED
+			} else {
+				parser->push_warning(p_for->datatype_specifier, GDScriptWarning::REDUNDANT_FOR_VARIABLE_TYPE, p_for->variable->name, variable_type.to_string(), specified_type.to_string());
+#endif
+			}
+			p_for->variable->set_datatype(specified_type);
+		} else {
 			p_for->variable->set_datatype(variable_type);
 		}
+	}
 
 	resolve_suite(p_for->loop);
 	p_for->set_datatype(p_for->loop->get_datatype());

modules/gdscript/gdscript_byte_codegen.cpp

@@ -1494,19 +1494,16 @@ void GDScriptByteCodeGenerator::start_for(const GDScriptDataType &p_iterator_typ
 	for_container_variables.push_back(container);
 }
 
-void GDScriptByteCodeGenerator::write_for_assignment(const Address &p_variable, const Address &p_list) {
+void GDScriptByteCodeGenerator::write_for_assignment(const Address &p_list) {
 	const Address &container = for_container_variables.back()->get();
 
 	// Assign container.
 	append_opcode(GDScriptFunction::OPCODE_ASSIGN);
 	append(container);
 	append(p_list);
-
-	for_iterator_variables.push_back(p_variable);
 }
 
-void GDScriptByteCodeGenerator::write_for() {
+void GDScriptByteCodeGenerator::write_for(const Address &p_variable, bool p_use_conversion) {
-	const Address &iterator = for_iterator_variables.back()->get();
 	const Address &counter = for_counter_variables.back()->get();
 	const Address &container = for_container_variables.back()->get();
 
@@ -1599,11 +1596,16 @@ void GDScriptByteCodeGenerator::write_for() {
 		}
 	}
 
+	Address temp;
+	if (p_use_conversion) {
+		temp = Address(Address::LOCAL_VARIABLE, add_local("@iterator_temp", GDScriptDataType()));
+	}
+
 	// Begin loop.
 	append_opcode(begin_opcode);
 	append(counter);
 	append(container);
-	append(iterator);
+	append(p_use_conversion ? temp : p_variable);
 	for_jmp_addrs.push_back(opcodes.size());
 	append(0); // End of loop address, will be patched.
 	append_opcode(GDScriptFunction::OPCODE_JUMP);
@@ -1615,9 +1617,17 @@ void GDScriptByteCodeGenerator::write_for() {
 	append_opcode(iterate_opcode);
 	append(counter);
 	append(container);
-	append(iterator);
+	append(p_use_conversion ? temp : p_variable);
 	for_jmp_addrs.push_back(opcodes.size());
 	append(0); // Jump destination, will be patched.
+
+	if (p_use_conversion) {
+		write_assign_with_conversion(p_variable, temp);
+		const GDScriptDataType &type = p_variable.type;
+		if (type.kind != GDScriptDataType::BUILTIN || type.builtin_type == Variant::ARRAY || type.builtin_type == Variant::DICTIONARY) {
+			write_assign_false(temp); // Can contain RefCounted, so clear it.
+		}
+	}
 }
 
 void GDScriptByteCodeGenerator::write_endfor() {
@@ -1639,7 +1649,6 @@ void GDScriptByteCodeGenerator::write_endfor() {
 	current_breaks_to_patch.pop_back();
 
 	// Pop state.
-	for_iterator_variables.pop_back();
 	for_counter_variables.pop_back();
 	for_container_variables.pop_back();
 }

modules/gdscript/gdscript_byte_codegen.h

@@ -143,7 +143,6 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 	// Lists since these can be nested.
 	List<int> if_jmp_addrs;
 	List<int> for_jmp_addrs;
-	List<Address> for_iterator_variables;
 	List<Address> for_counter_variables;
 	List<Address> for_container_variables;
 	List<int> while_jmp_addrs;
@@ -536,8 +535,8 @@ public:
 	virtual void write_jump_if_shared(const Address &p_value) override;
 	virtual void write_end_jump_if_shared() override;
 	virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) override;
-	virtual void write_for_assignment(const Address &p_variable, const Address &p_list) override;
+	virtual void write_for_assignment(const Address &p_list) override;
-	virtual void write_for() override;
+	virtual void write_for(const Address &p_variable, bool p_use_conversion) override;
 	virtual void write_endfor() override;
 	virtual void start_while_condition() override;
 	virtual void write_while(const Address &p_condition) override;

modules/gdscript/gdscript_codegen.h

@@ -145,8 +145,8 @@ public:
 	virtual void write_jump_if_shared(const Address &p_value) = 0;
 	virtual void write_end_jump_if_shared() = 0;
 	virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) = 0;
-	virtual void write_for_assignment(const Address &p_variable, const Address &p_list) = 0;
+	virtual void write_for_assignment(const Address &p_list) = 0;
-	virtual void write_for() = 0;
+	virtual void write_for(const Address &p_variable, bool p_use_conversion) = 0;
 	virtual void write_endfor() = 0;
 	virtual void start_while_condition() = 0; // Used to allow a jump to the expression evaluation.
 	virtual void write_while(const Address &p_condition) = 0;

modules/gdscript/gdscript_compiler.cpp

@@ -1953,13 +1953,13 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
 					return err;
 				}
 
-				gen->write_for_assignment(iterator, list);
+				gen->write_for_assignment(list);
 
 				if (list.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
 					codegen.generator->pop_temporary();
 				}
 
-				gen->write_for();
+				gen->write_for(iterator, for_n->use_conversion_assign);
 
 				err = _parse_block(codegen, for_n->loop);
 				if (err) {

modules/gdscript/gdscript_parser.cpp

@@ -1850,7 +1850,18 @@ GDScriptParser::ForNode *GDScriptParser::parse_for() {
 		n_for->variable = parse_identifier();
 	}
 
-	consume(GDScriptTokenizer::Token::IN, R"(Expected "in" after "for" variable name.)");
+	if (match(GDScriptTokenizer::Token::COLON)) {
+		n_for->datatype_specifier = parse_type();
+		if (n_for->datatype_specifier == nullptr) {
+			push_error(R"(Expected type specifier after ":".)");
+		}
+	}
+
+	if (n_for->datatype_specifier == nullptr) {
+		consume(GDScriptTokenizer::Token::IN, R"(Expected "in" or ":" after "for" variable name.)");
+	} else {
+		consume(GDScriptTokenizer::Token::IN, R"(Expected "in" after "for" variable type specifier.)");
+	}
 
 	n_for->list = parse_expression(false);
 

modules/gdscript/gdscript_parser.h

@@ -814,6 +814,8 @@ public:
 
 	struct ForNode : public Node {
 		IdentifierNode *variable = nullptr;
+		TypeNode *datatype_specifier = nullptr;
+		bool use_conversion_assign = false;
 		ExpressionNode *list = nullptr;
 		SuiteNode *loop = nullptr;
 

modules/gdscript/gdscript_warning.cpp

@@ -113,6 +113,14 @@ String GDScriptWarning::get_message() const {
 			return R"(The "@static_unload" annotation is redundant because the file does not have a class with static variables.)";
 		case REDUNDANT_AWAIT:
 			return R"("await" keyword not needed in this case, because the expression isn't a coroutine nor a signal.)";
+		case REDUNDANT_FOR_VARIABLE_TYPE:
+			CHECK_SYMBOLS(3);
+			if (symbols[1] == symbols[2]) {
+				return vformat(R"(The for loop iterator "%s" already has inferred type "%s", the specified type is redundant.)", symbols[0], symbols[1]);
+			} else {
+				return vformat(R"(The for loop iterator "%s" has inferred type "%s" but its supertype "%s" is specified.)", symbols[0], symbols[1], symbols[2]);
+			}
+			break;
 		case ASSERT_ALWAYS_TRUE:
 			return "Assert statement is redundant because the expression is always true.";
 		case ASSERT_ALWAYS_FALSE:
@@ -209,6 +217,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
 		"STATIC_CALLED_ON_INSTANCE",
 		"REDUNDANT_STATIC_UNLOAD",
 		"REDUNDANT_AWAIT",
+		"REDUNDANT_FOR_VARIABLE_TYPE",
 		"ASSERT_ALWAYS_TRUE",
 		"ASSERT_ALWAYS_FALSE",
 		"INTEGER_DIVISION",

modules/gdscript/gdscript_warning.h

@@ -73,6 +73,7 @@ public:
 		STATIC_CALLED_ON_INSTANCE, // A static method was called on an instance of a class instead of on the class itself.
 		REDUNDANT_STATIC_UNLOAD, // The `@static_unload` annotation is used but the class does not have static data.
 		REDUNDANT_AWAIT, // await is used but expression is synchronous (not a signal nor a coroutine).
+		REDUNDANT_FOR_VARIABLE_TYPE, // The for variable type specifier is a supertype of the inferred type.
 		ASSERT_ALWAYS_TRUE, // Expression for assert argument is always true.
 		ASSERT_ALWAYS_FALSE, // Expression for assert argument is always false.
 		INTEGER_DIVISION, // Integer divide by integer, decimal part is discarded.
@@ -120,6 +121,7 @@ public:
 		WARN, // STATIC_CALLED_ON_INSTANCE
 		WARN, // REDUNDANT_STATIC_UNLOAD
 		WARN, // REDUNDANT_AWAIT
+		WARN, // REDUNDANT_FOR_VARIABLE_TYPE
 		WARN, // ASSERT_ALWAYS_TRUE
 		WARN, // ASSERT_ALWAYS_FALSE
 		WARN, // INTEGER_DIVISION

modules/gdscript/tests/scripts/analyzer/errors/for_loop_wrong_specified_type.gd

@@ -0,0 +1,4 @@
+func test():
+	var a: Array[Resource] = []
+	for node: Node in a:
+		print(node)

modules/gdscript/tests/scripts/analyzer/errors/for_loop_wrong_specified_type.out

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Unable to iterate on value of type "Array[Resource]" with variable of type "Node".

modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_equal_to_inferred.gd

@@ -0,0 +1,4 @@
+func test():
+	var a: Array[Node] = []
+	for node: Node in a:
+		print(node)

modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_equal_to_inferred.out

@@ -0,0 +1,5 @@
+GDTEST_OK
+>> WARNING
+>> Line: 3
+>> REDUNDANT_FOR_VARIABLE_TYPE
+>> The for loop iterator "node" already has inferred type "Node", the specified type is redundant.

modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_supertype_of_inferred.gd

@@ -0,0 +1,4 @@
+func test():
+	var a: Array[Node2D] = []
+	for node: Node in a:
+		print(node)

modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_supertype_of_inferred.out

@@ -0,0 +1,5 @@
+GDTEST_OK
+>> WARNING
+>> Line: 3
+>> REDUNDANT_FOR_VARIABLE_TYPE
+>> The for loop iterator "node" has inferred type "Node2D" but its supertype "Node" is specified.

modules/gdscript/tests/scripts/runtime/errors/for_loop_iterator_type_not_match_specified.gd

@@ -0,0 +1,4 @@
+func test():
+	var a: Array = [Resource.new()]
+	for node: Node in a:
+		print(node)

modules/gdscript/tests/scripts/runtime/errors/for_loop_iterator_type_not_match_specified.out

@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: test()
+>> runtime/errors/for_loop_iterator_type_not_match_specified.gd
+>> 3
+>> Trying to assign value of type 'Resource' to a variable of type 'Node'.

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

@@ -0,0 +1,34 @@
+func test():
+	print("Test range.")
+	for e: float in range(2, 5):
+		var elem := e
+		prints(var_to_str(e), var_to_str(elem))
+
+	print("Test int.")
+	for e: float in 3:
+		var elem := e
+		prints(var_to_str(e), var_to_str(elem))
+
+	print("Test untyped int array.")
+	var a1 := [10, 20, 30]
+	for e: float in a1:
+		var elem := e
+		prints(var_to_str(e), var_to_str(elem))
+
+	print("Test typed int array.")
+	var a2: Array[int] = [10, 20, 30]
+	for e: float in a2:
+		var elem := e
+		prints(var_to_str(e), var_to_str(elem))
+
+	print("Test String-keys dictionary.")
+	var d1 := {a = 1, b = 2, c = 3}
+	for k: StringName in d1:
+		var key := k
+		prints(var_to_str(k), var_to_str(key))
+
+	print("Test RefCounted-keys dictionary.")
+	var d2 := {RefCounted.new(): 1, Resource.new(): 2, ConfigFile.new(): 3}
+	for k: RefCounted in d2:
+		var key := k
+		prints(k.get_class(), key.get_class())

modules/gdscript/tests/scripts/runtime/features/for_loop_iterator_specified_types.out

@@ -0,0 +1,25 @@
+GDTEST_OK
+Test range.
+2.0 2.0
+3.0 3.0
+4.0 4.0
+Test int.
+0.0 0.0
+1.0 1.0
+2.0 2.0
+Test untyped int array.
+10.0 10.0
+20.0 20.0
+30.0 30.0
+Test typed int array.
+10.0 10.0
+20.0 20.0
+30.0 30.0
+Test String-keys dictionary.
+&"a" &"a"
+&"b" &"b"
+&"c" &"c"
+Test RefCounted-keys dictionary.
+RefCounted RefCounted
+Resource Resource
+ConfigFile ConfigFile