diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index f426030f98b..0e35bc70314 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -492,6 +492,9 @@ 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. + + 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. + 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. diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 9f9accf507f..214b484b12e 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/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 = ""; 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,8 +2054,37 @@ 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) { - p_for->variable->set_datatype(variable_type); + 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); diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp index 6057a00f9b3..af7862efc51 100644 --- a/modules/gdscript/gdscript_byte_codegen.cpp +++ b/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() { - const Address &iterator = for_iterator_variables.back()->get(); +void GDScriptByteCodeGenerator::write_for(const Address &p_variable, bool p_use_conversion) { 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(); } diff --git a/modules/gdscript/gdscript_byte_codegen.h b/modules/gdscript/gdscript_byte_codegen.h index bbcd252b13d..671dea5d6d6 100644 --- a/modules/gdscript/gdscript_byte_codegen.h +++ b/modules/gdscript/gdscript_byte_codegen.h @@ -143,7 +143,6 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator { // Lists since these can be nested. List if_jmp_addrs; List for_jmp_addrs; - List
for_iterator_variables; List
for_counter_variables; List
for_container_variables; List 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() override; + virtual void write_for_assignment(const Address &p_list) 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; diff --git a/modules/gdscript/gdscript_codegen.h b/modules/gdscript/gdscript_codegen.h index 9810f5395ab..cf17353dec1 100644 --- a/modules/gdscript/gdscript_codegen.h +++ b/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() = 0; + virtual void write_for_assignment(const Address &p_list) = 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; diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 3366fa2eec6..f964db231a6 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/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) { diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index b76ceea11f7..1dde67d2d12 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/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); diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 71660d8f60d..652faaebc34 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/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; diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp index 24aa793c474..4fec445995f 100644 --- a/modules/gdscript/gdscript_warning.cpp +++ b/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", diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h index 8444d46a880..73e12eb20e6 100644 --- a/modules/gdscript/gdscript_warning.h +++ b/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 diff --git a/modules/gdscript/tests/scripts/analyzer/errors/for_loop_wrong_specified_type.gd b/modules/gdscript/tests/scripts/analyzer/errors/for_loop_wrong_specified_type.gd new file mode 100644 index 00000000000..7e3b6e3c398 --- /dev/null +++ b/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) diff --git a/modules/gdscript/tests/scripts/analyzer/errors/for_loop_wrong_specified_type.out b/modules/gdscript/tests/scripts/analyzer/errors/for_loop_wrong_specified_type.out new file mode 100644 index 00000000000..8f8a368f9ae --- /dev/null +++ b/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". diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_equal_to_inferred.gd b/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_equal_to_inferred.gd new file mode 100644 index 00000000000..1b32491e48c --- /dev/null +++ b/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) diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_equal_to_inferred.out b/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_equal_to_inferred.out new file mode 100644 index 00000000000..3b3fbd9bd1b --- /dev/null +++ b/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. diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_supertype_of_inferred.gd b/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_supertype_of_inferred.gd new file mode 100644 index 00000000000..fcbc13c53d0 --- /dev/null +++ b/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) diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_supertype_of_inferred.out b/modules/gdscript/tests/scripts/analyzer/warnings/for_loop_specified_type_is_supertype_of_inferred.out new file mode 100644 index 00000000000..36d4a161d39 --- /dev/null +++ b/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. diff --git a/modules/gdscript/tests/scripts/runtime/errors/for_loop_iterator_type_not_match_specified.gd b/modules/gdscript/tests/scripts/runtime/errors/for_loop_iterator_type_not_match_specified.gd new file mode 100644 index 00000000000..cdc278ecf58 --- /dev/null +++ b/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) diff --git a/modules/gdscript/tests/scripts/runtime/errors/for_loop_iterator_type_not_match_specified.out b/modules/gdscript/tests/scripts/runtime/errors/for_loop_iterator_type_not_match_specified.out new file mode 100644 index 00000000000..b7ef07afb2b --- /dev/null +++ b/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'. diff --git a/modules/gdscript/tests/scripts/runtime/features/for_loop_iterator_specified_types.gd b/modules/gdscript/tests/scripts/runtime/features/for_loop_iterator_specified_types.gd new file mode 100644 index 00000000000..58b4df5a79d --- /dev/null +++ b/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()) diff --git a/modules/gdscript/tests/scripts/runtime/features/for_loop_iterator_specified_types.out b/modules/gdscript/tests/scripts/runtime/features/for_loop_iterator_specified_types.out new file mode 100644 index 00000000000..f67f7d89d55 --- /dev/null +++ b/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