diff options
author | Yuri Sizov <yuris@humnom.net> | 2023-07-31 21:01:36 +0200 |
---|---|---|
committer | Yuri Sizov <yuris@humnom.net> | 2023-07-31 21:01:36 +0200 |
commit | 3de7dd902c3b491b92cad822eb1ce7018001c24b (patch) | |
tree | 29ac72747d9bb8ac089ad7a05842676861e97901 /modules/gdscript | |
parent | 8b12849fef2059421583e4e5bf2a27f654d8ab42 (diff) | |
parent | d53fc92b4c6b5e4484e8f0bfff6ac55163dde3fb (diff) | |
download | redot-engine-3de7dd902c3b491b92cad822eb1ce7018001c24b.tar.gz |
Merge pull request #79880 from dalexeev/gds-fix-id-shadowing-below
GDScript: Fix bug with identifier shadowed below in current scope
Diffstat (limited to 'modules/gdscript')
18 files changed, 354 insertions, 206 deletions
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 01e94ebb22..cb04913620 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1772,6 +1772,15 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi bool is_constant = p_assignable->type == GDScriptParser::Node::CONSTANT; +#ifdef DEBUG_ENABLED + if (p_assignable->identifier != nullptr && p_assignable->identifier->suite != nullptr && p_assignable->identifier->suite->parent_block != nullptr) { + if (p_assignable->identifier->suite->parent_block->has_local(p_assignable->identifier->name)) { + const GDScriptParser::SuiteNode::Local &local = p_assignable->identifier->suite->parent_block->get_local(p_assignable->identifier->name); + parser->push_warning(p_assignable->identifier, GDScriptWarning::CONFUSABLE_LOCAL_DECLARATION, local.get_name(), p_assignable->identifier->name); + } + } +#endif + GDScriptParser::DataType specified_type; bool has_specified_type = p_assignable->datatype_specifier != nullptr; if (has_specified_type) { @@ -3518,12 +3527,14 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod case GDScriptParser::ClassNode::Member::FUNCTION: { if (is_base && (!base.is_meta_type || member.function->is_static)) { p_identifier->set_datatype(make_callable_type(member.function->info)); + p_identifier->source = GDScriptParser::IdentifierNode::MEMBER_FUNCTION; return; } } break; case GDScriptParser::ClassNode::Member::CLASS: { reduce_identifier_from_base_set_class(p_identifier, member.get_datatype()); + p_identifier->source = GDScriptParser::IdentifierNode::MEMBER_CLASS; return; } @@ -3662,9 +3673,17 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident found_source = true; } break; case GDScriptParser::IdentifierNode::UNDEFINED_SOURCE: + case GDScriptParser::IdentifierNode::MEMBER_FUNCTION: + case GDScriptParser::IdentifierNode::MEMBER_CLASS: break; } +#ifdef DEBUG_ENABLED + if (!found_source && p_identifier->suite != nullptr && p_identifier->suite->has_local(p_identifier->name)) { + parser->push_warning(p_identifier, GDScriptWarning::CONFUSABLE_LOCAL_USAGE, p_identifier->name); + } +#endif + // Not a local, so check members. if (!found_source) { reduce_identifier_from_base(p_identifier); diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 745ccbb1ed..2a52db4158 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -225,194 +225,211 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code StringName identifier = in->name; - // Try function parameters. - if (codegen.parameters.has(identifier)) { - return codegen.parameters[identifier]; - } - - // Try local variables and constants. - if (!p_initializer && codegen.locals.has(identifier)) { - return codegen.locals[identifier]; - } + switch (in->source) { + // LOCALS. + case GDScriptParser::IdentifierNode::FUNCTION_PARAMETER: + case GDScriptParser::IdentifierNode::LOCAL_VARIABLE: + case GDScriptParser::IdentifierNode::LOCAL_CONSTANT: + case GDScriptParser::IdentifierNode::LOCAL_ITERATOR: + case GDScriptParser::IdentifierNode::LOCAL_BIND: { + // Try function parameters. + if (codegen.parameters.has(identifier)) { + return codegen.parameters[identifier]; + } - // Try class members. - if (_is_class_member_property(codegen, identifier)) { - // Get property. - GDScriptCodeGenerator::Address temp = codegen.add_temporary(_gdtype_from_datatype(p_expression->get_datatype(), codegen.script)); - gen->write_get_member(temp, identifier); - return temp; - } + // Try local variables and constants. + if (!p_initializer && codegen.locals.has(identifier)) { + return codegen.locals[identifier]; + } + } break; - // Try members. - if (!codegen.function_node || !codegen.function_node->is_static) { - // Try member variables. - if (codegen.script->member_indices.has(identifier)) { - if (codegen.script->member_indices[identifier].getter != StringName() && codegen.script->member_indices[identifier].getter != codegen.function_name) { - // Perform getter. - GDScriptCodeGenerator::Address temp = codegen.add_temporary(codegen.script->member_indices[identifier].data_type); - Vector<GDScriptCodeGenerator::Address> args; // No argument needed. - gen->write_call_self(temp, codegen.script->member_indices[identifier].getter, args); + // MEMBERS. + case GDScriptParser::IdentifierNode::MEMBER_VARIABLE: + case GDScriptParser::IdentifierNode::INHERITED_VARIABLE: { + // Try class members. + if (_is_class_member_property(codegen, identifier)) { + // Get property. + GDScriptCodeGenerator::Address temp = codegen.add_temporary(_gdtype_from_datatype(p_expression->get_datatype(), codegen.script)); + gen->write_get_member(temp, identifier); return temp; - } else { - // No getter or inside getter: direct member access. - int idx = codegen.script->member_indices[identifier].index; - return GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::MEMBER, idx, codegen.script->get_member_type(identifier)); } - } - } - // Try static variables. - { - GDScript *scr = codegen.script; - while (scr) { - if (scr->static_variables_indices.has(identifier)) { - if (scr->static_variables_indices[identifier].getter != StringName() && scr->static_variables_indices[identifier].getter != codegen.function_name) { - // Perform getter. - GDScriptCodeGenerator::Address temp = codegen.add_temporary(scr->static_variables_indices[identifier].data_type); - GDScriptCodeGenerator::Address class_addr(GDScriptCodeGenerator::Address::CLASS); - Vector<GDScriptCodeGenerator::Address> args; // No argument needed. - gen->write_call(temp, class_addr, scr->static_variables_indices[identifier].getter, args); - return temp; - } else { - // No getter or inside getter: direct variable access. - GDScriptCodeGenerator::Address temp = codegen.add_temporary(scr->static_variables_indices[identifier].data_type); - GDScriptCodeGenerator::Address _class = codegen.add_constant(scr); - int index = scr->static_variables_indices[identifier].index; - gen->write_get_static_variable(temp, _class, index); - return temp; + // Try members. + if (!codegen.function_node || !codegen.function_node->is_static) { + // Try member variables. + if (codegen.script->member_indices.has(identifier)) { + if (codegen.script->member_indices[identifier].getter != StringName() && codegen.script->member_indices[identifier].getter != codegen.function_name) { + // Perform getter. + GDScriptCodeGenerator::Address temp = codegen.add_temporary(codegen.script->member_indices[identifier].data_type); + Vector<GDScriptCodeGenerator::Address> args; // No argument needed. + gen->write_call_self(temp, codegen.script->member_indices[identifier].getter, args); + return temp; + } else { + // No getter or inside getter: direct member access. + int idx = codegen.script->member_indices[identifier].index; + return GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::MEMBER, idx, codegen.script->get_member_type(identifier)); + } } } - scr = scr->_base; - } - } + } break; + case GDScriptParser::IdentifierNode::MEMBER_FUNCTION: + case GDScriptParser::IdentifierNode::MEMBER_SIGNAL: { + // Try methods and signals (can be Callable and Signal). + + // Search upwards through parent classes: + const GDScriptParser::ClassNode *base_class = codegen.class_node; + while (base_class != nullptr) { + if (base_class->has_member(identifier)) { + const GDScriptParser::ClassNode::Member &member = base_class->get_member(identifier); + if (member.type == GDScriptParser::ClassNode::Member::FUNCTION || member.type == GDScriptParser::ClassNode::Member::SIGNAL) { + // Get like it was a property. + GDScriptCodeGenerator::Address temp = codegen.add_temporary(); // TODO: Get type here. + GDScriptCodeGenerator::Address self(GDScriptCodeGenerator::Address::SELF); + + gen->write_get_named(temp, identifier, self); + return temp; + } + } + base_class = base_class->base_type.class_type; + } - // Try class constants. - { - GDScript *owner = codegen.script; - while (owner) { - GDScript *scr = owner; + // Try in native base. + GDScript *scr = codegen.script; GDScriptNativeClass *nc = nullptr; while (scr) { - if (scr->constants.has(identifier)) { - return codegen.add_constant(scr->constants[identifier]); // TODO: Get type here. - } if (scr->native.is_valid()) { nc = scr->native.ptr(); } scr = scr->_base; } - // Class C++ integer constant. - if (nc) { - bool success = false; - int64_t constant = ClassDB::get_integer_constant(nc->get_name(), identifier, &success); - if (success) { - return codegen.add_constant(constant); - } - } + if (nc && (ClassDB::has_signal(nc->get_name(), identifier) || ClassDB::has_method(nc->get_name(), identifier))) { + // Get like it was a property. + GDScriptCodeGenerator::Address temp = codegen.add_temporary(); // TODO: Get type here. + GDScriptCodeGenerator::Address self(GDScriptCodeGenerator::Address::SELF); - owner = owner->_owner; - } - } + gen->write_get_named(temp, identifier, self); + return temp; + } + } break; + case GDScriptParser::IdentifierNode::MEMBER_CONSTANT: + case GDScriptParser::IdentifierNode::MEMBER_CLASS: { + // Try class constants. + GDScript *owner = codegen.script; + while (owner) { + GDScript *scr = owner; + GDScriptNativeClass *nc = nullptr; + while (scr) { + if (scr->constants.has(identifier)) { + return codegen.add_constant(scr->constants[identifier]); // TODO: Get type here. + } + if (scr->native.is_valid()) { + nc = scr->native.ptr(); + } + scr = scr->_base; + } - // Try signals and methods (can be made callables). - { - // Search upwards through parent classes: - const GDScriptParser::ClassNode *base_class = codegen.class_node; - while (base_class != nullptr) { - if (base_class->has_member(identifier)) { - const GDScriptParser::ClassNode::Member &member = base_class->get_member(identifier); - if (member.type == GDScriptParser::ClassNode::Member::FUNCTION || member.type == GDScriptParser::ClassNode::Member::SIGNAL) { - // Get like it was a property. - GDScriptCodeGenerator::Address temp = codegen.add_temporary(); // TODO: Get type here. - GDScriptCodeGenerator::Address self(GDScriptCodeGenerator::Address::SELF); + // Class C++ integer constant. + if (nc) { + bool success = false; + int64_t constant = ClassDB::get_integer_constant(nc->get_name(), identifier, &success); + if (success) { + return codegen.add_constant(constant); + } + } - gen->write_get_named(temp, identifier, self); - return temp; + owner = owner->_owner; + } + } break; + case GDScriptParser::IdentifierNode::STATIC_VARIABLE: { + // Try static variables. + GDScript *scr = codegen.script; + while (scr) { + if (scr->static_variables_indices.has(identifier)) { + if (scr->static_variables_indices[identifier].getter != StringName() && scr->static_variables_indices[identifier].getter != codegen.function_name) { + // Perform getter. + GDScriptCodeGenerator::Address temp = codegen.add_temporary(scr->static_variables_indices[identifier].data_type); + GDScriptCodeGenerator::Address class_addr(GDScriptCodeGenerator::Address::CLASS); + Vector<GDScriptCodeGenerator::Address> args; // No argument needed. + gen->write_call(temp, class_addr, scr->static_variables_indices[identifier].getter, args); + return temp; + } else { + // No getter or inside getter: direct variable access. + GDScriptCodeGenerator::Address temp = codegen.add_temporary(scr->static_variables_indices[identifier].data_type); + GDScriptCodeGenerator::Address _class = codegen.add_constant(scr); + int index = scr->static_variables_indices[identifier].index; + gen->write_get_static_variable(temp, _class, index); + return temp; + } } + scr = scr->_base; } - base_class = base_class->base_type.class_type; - } + } break; - // Try in native base. - GDScript *scr = codegen.script; - GDScriptNativeClass *nc = nullptr; - while (scr) { - if (scr->native.is_valid()) { - nc = scr->native.ptr(); + // GLOBALS. + case GDScriptParser::IdentifierNode::UNDEFINED_SOURCE: { + // Try globals. + if (GDScriptLanguage::get_singleton()->get_global_map().has(identifier)) { + // If it's an autoload singleton, we postpone to load it at runtime. + // This is so one autoload doesn't try to load another before it's compiled. + HashMap<StringName, ProjectSettings::AutoloadInfo> autoloads = ProjectSettings::get_singleton()->get_autoload_list(); + if (autoloads.has(identifier) && autoloads[identifier].is_singleton) { + GDScriptCodeGenerator::Address global = codegen.add_temporary(_gdtype_from_datatype(in->get_datatype(), codegen.script)); + int idx = GDScriptLanguage::get_singleton()->get_global_map()[identifier]; + gen->write_store_global(global, idx); + return global; + } else { + int idx = GDScriptLanguage::get_singleton()->get_global_map()[identifier]; + Variant global = GDScriptLanguage::get_singleton()->get_global_array()[idx]; + return codegen.add_constant(global); + } } - scr = scr->_base; - } - - if (nc && (ClassDB::has_signal(nc->get_name(), identifier) || ClassDB::has_method(nc->get_name(), identifier))) { - // Get like it was a property. - GDScriptCodeGenerator::Address temp = codegen.add_temporary(); // TODO: Get type here. - GDScriptCodeGenerator::Address self(GDScriptCodeGenerator::Address::SELF); - - gen->write_get_named(temp, identifier, self); - return temp; - } - } - - // Try globals. - if (GDScriptLanguage::get_singleton()->get_global_map().has(identifier)) { - // If it's an autoload singleton, we postpone to load it at runtime. - // This is so one autoload doesn't try to load another before it's compiled. - HashMap<StringName, ProjectSettings::AutoloadInfo> autoloads = ProjectSettings::get_singleton()->get_autoload_list(); - if (autoloads.has(identifier) && autoloads[identifier].is_singleton) { - GDScriptCodeGenerator::Address global = codegen.add_temporary(_gdtype_from_datatype(in->get_datatype(), codegen.script)); - int idx = GDScriptLanguage::get_singleton()->get_global_map()[identifier]; - gen->write_store_global(global, idx); - return global; - } else { - int idx = GDScriptLanguage::get_singleton()->get_global_map()[identifier]; - Variant global = GDScriptLanguage::get_singleton()->get_global_array()[idx]; - return codegen.add_constant(global); - } - } - // Try global classes. - if (ScriptServer::is_global_class(identifier)) { - const GDScriptParser::ClassNode *class_node = codegen.class_node; - while (class_node->outer) { - class_node = class_node->outer; - } + // Try global classes. + if (ScriptServer::is_global_class(identifier)) { + const GDScriptParser::ClassNode *class_node = codegen.class_node; + while (class_node->outer) { + class_node = class_node->outer; + } - Ref<Resource> res; + Ref<Resource> res; - if (class_node->identifier && class_node->identifier->name == identifier) { - res = Ref<GDScript>(main_script); - } else { - String global_class_path = ScriptServer::get_global_class_path(identifier); - if (ResourceLoader::get_resource_type(global_class_path) == "GDScript") { - Error err = OK; - res = GDScriptCache::get_full_script(global_class_path, err); - if (err != OK) { - _set_error("Can't load global class " + String(identifier), p_expression); - r_error = ERR_COMPILATION_FAILED; - return GDScriptCodeGenerator::Address(); - } - } else { - res = ResourceLoader::load(global_class_path); - if (res.is_null()) { - _set_error("Can't load global class " + String(identifier) + ", cyclic reference?", p_expression); - r_error = ERR_COMPILATION_FAILED; - return GDScriptCodeGenerator::Address(); + if (class_node->identifier && class_node->identifier->name == identifier) { + res = Ref<GDScript>(main_script); + } else { + String global_class_path = ScriptServer::get_global_class_path(identifier); + if (ResourceLoader::get_resource_type(global_class_path) == "GDScript") { + Error err = OK; + res = GDScriptCache::get_full_script(global_class_path, err); + if (err != OK) { + _set_error("Can't load global class " + String(identifier), p_expression); + r_error = ERR_COMPILATION_FAILED; + return GDScriptCodeGenerator::Address(); + } + } else { + res = ResourceLoader::load(global_class_path); + if (res.is_null()) { + _set_error("Can't load global class " + String(identifier) + ", cyclic reference?", p_expression); + r_error = ERR_COMPILATION_FAILED; + return GDScriptCodeGenerator::Address(); + } + } } - } - } - return codegen.add_constant(res); - } + return codegen.add_constant(res); + } #ifdef TOOLS_ENABLED - if (GDScriptLanguage::get_singleton()->get_named_globals_map().has(identifier)) { - GDScriptCodeGenerator::Address global = codegen.add_temporary(); // TODO: Get type. - gen->write_store_named_global(global, identifier); - return global; - } + if (GDScriptLanguage::get_singleton()->get_named_globals_map().has(identifier)) { + GDScriptCodeGenerator::Address global = codegen.add_temporary(); // TODO: Get type. + gen->write_store_named_global(global, identifier); + return global; + } #endif + } break; + } + // Not found, error. _set_error("Identifier not found: " + String(identifier), p_expression); r_error = ERR_COMPILATION_FAILED; diff --git a/modules/gdscript/gdscript_editor.cpp b/modules/gdscript/gdscript_editor.cpp index 47ceee3943..8ad2486e2b 100644 --- a/modules/gdscript/gdscript_editor.cpp +++ b/modules/gdscript/gdscript_editor.cpp @@ -1403,7 +1403,7 @@ struct RecursionCheck { } }; -static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, const StringName &p_identifier, GDScriptCompletionIdentifier &r_type); +static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, const GDScriptParser::IdentifierNode *p_identifier, GDScriptCompletionIdentifier &r_type); static bool _guess_identifier_type_from_base(GDScriptParser::CompletionContext &p_context, const GDScriptCompletionIdentifier &p_base, const StringName &p_identifier, GDScriptCompletionIdentifier &r_type); static bool _guess_method_return_type_from_base(GDScriptParser::CompletionContext &p_context, const GDScriptCompletionIdentifier &p_base, const StringName &p_method, GDScriptCompletionIdentifier &r_type); @@ -1467,7 +1467,7 @@ static bool _guess_expression_type(GDScriptParser::CompletionContext &p_context, } break; case GDScriptParser::Node::IDENTIFIER: { const GDScriptParser::IdentifierNode *id = static_cast<const GDScriptParser::IdentifierNode *>(p_expression); - found = _guess_identifier_type(p_context, id->name, r_type); + found = _guess_identifier_type(p_context, id, r_type); } break; case GDScriptParser::Node::DICTIONARY: { // Try to recreate the dictionary. @@ -1910,7 +1910,7 @@ static bool _guess_expression_type(GDScriptParser::CompletionContext &p_context, return found; } -static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, const StringName &p_identifier, GDScriptCompletionIdentifier &r_type) { +static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, const GDScriptParser::IdentifierNode *p_identifier, GDScriptCompletionIdentifier &r_type) { static int recursion_depth = 0; RecursionCheck recursion(&recursion_depth); if (unlikely(recursion.check())) { @@ -1924,36 +1924,49 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, GDScriptParser::SuiteNode *suite = p_context.current_suite; bool is_function_parameter = false; - if (suite) { - if (suite->has_local(p_identifier)) { - const GDScriptParser::SuiteNode::Local &local = suite->get_local(p_identifier); + bool can_be_local = true; + switch (p_identifier->source) { + case GDScriptParser::IdentifierNode::MEMBER_VARIABLE: + case GDScriptParser::IdentifierNode::MEMBER_CONSTANT: + case GDScriptParser::IdentifierNode::MEMBER_FUNCTION: + case GDScriptParser::IdentifierNode::MEMBER_SIGNAL: + case GDScriptParser::IdentifierNode::MEMBER_CLASS: + case GDScriptParser::IdentifierNode::INHERITED_VARIABLE: + case GDScriptParser::IdentifierNode::STATIC_VARIABLE: + can_be_local = false; + break; + default: + break; + } - id_type = local.get_datatype(); + if (can_be_local && suite && suite->has_local(p_identifier->name)) { + const GDScriptParser::SuiteNode::Local &local = suite->get_local(p_identifier->name); - // Check initializer as the first assignment. - switch (local.type) { - case GDScriptParser::SuiteNode::Local::VARIABLE: - if (local.variable->initializer) { - last_assign_line = local.variable->initializer->end_line; - last_assigned_expression = local.variable->initializer; - } - break; - case GDScriptParser::SuiteNode::Local::CONSTANT: - if (local.constant->initializer) { - last_assign_line = local.constant->initializer->end_line; - last_assigned_expression = local.constant->initializer; - } - break; - case GDScriptParser::SuiteNode::Local::PARAMETER: - if (local.parameter->initializer) { - last_assign_line = local.parameter->initializer->end_line; - last_assigned_expression = local.parameter->initializer; - } - is_function_parameter = true; - break; - default: - break; - } + id_type = local.get_datatype(); + + // Check initializer as the first assignment. + switch (local.type) { + case GDScriptParser::SuiteNode::Local::VARIABLE: + if (local.variable->initializer) { + last_assign_line = local.variable->initializer->end_line; + last_assigned_expression = local.variable->initializer; + } + break; + case GDScriptParser::SuiteNode::Local::CONSTANT: + if (local.constant->initializer) { + last_assign_line = local.constant->initializer->end_line; + last_assigned_expression = local.constant->initializer; + } + break; + case GDScriptParser::SuiteNode::Local::PARAMETER: + if (local.parameter->initializer) { + last_assign_line = local.parameter->initializer->end_line; + last_assigned_expression = local.parameter->initializer; + } + is_function_parameter = true; + break; + default: + break; } } @@ -1968,7 +1981,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, const GDScriptParser::AssignmentNode *assign = static_cast<const GDScriptParser::AssignmentNode *>(suite->statements[i]); if (assign->end_line > last_assign_line && assign->assignee && assign->assigned_value && assign->assignee->type == GDScriptParser::Node::IDENTIFIER) { const GDScriptParser::IdentifierNode *id = static_cast<const GDScriptParser::IdentifierNode *>(assign->assignee); - if (id->name == p_identifier) { + if (id->name == p_identifier->name && id->source == p_identifier->source) { last_assign_line = assign->assigned_value->end_line; last_assigned_expression = assign->assigned_value; } @@ -1986,7 +1999,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, // Credit: Zylann. // TODO: this could be hacked to detect ANDed conditions too... const GDScriptParser::TypeTestNode *type_test = static_cast<const GDScriptParser::TypeTestNode *>(suite->parent_if->condition); - if (type_test->operand && type_test->test_type && type_test->operand->type == GDScriptParser::Node::IDENTIFIER && static_cast<const GDScriptParser::IdentifierNode *>(type_test->operand)->name == p_identifier) { + if (type_test->operand && type_test->test_type && type_test->operand->type == GDScriptParser::Node::IDENTIFIER && static_cast<const GDScriptParser::IdentifierNode *>(type_test->operand)->name == p_identifier->name && static_cast<const GDScriptParser::IdentifierNode *>(type_test->operand)->source == p_identifier->source) { // Bingo. GDScriptParser::CompletionContext c = p_context; c.current_line = type_test->operand->start_line; @@ -2022,8 +2035,8 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, case GDScriptParser::DataType::CLASS: if (base_type.class_type->has_function(p_context.current_function->identifier->name)) { GDScriptParser::FunctionNode *parent_function = base_type.class_type->get_member(p_context.current_function->identifier->name).function; - if (parent_function->parameters_indices.has(p_identifier)) { - const GDScriptParser::ParameterNode *parameter = parent_function->parameters[parent_function->parameters_indices[p_identifier]]; + if (parent_function->parameters_indices.has(p_identifier->name)) { + const GDScriptParser::ParameterNode *parameter = parent_function->parameters[parent_function->parameters_indices[p_identifier->name]]; if ((!id_type.is_set() || id_type.is_variant()) && parameter->get_datatype().is_hard_type()) { id_type = parameter->get_datatype(); } @@ -2048,7 +2061,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, MethodInfo info; if (ClassDB::get_method_info(base_type.native_type, p_context.current_function->identifier->name, &info)) { for (const PropertyInfo &E : info.arguments) { - if (E.name == p_identifier) { + if (E.name == p_identifier->name) { r_type = _type_from_property(E); return true; } @@ -2076,14 +2089,14 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, base.type.class_type = p_context.current_class; base.type.is_meta_type = p_context.current_function && p_context.current_function->is_static; - if (_guess_identifier_type_from_base(p_context, base, p_identifier, r_type)) { + if (_guess_identifier_type_from_base(p_context, base, p_identifier->name, r_type)) { return true; } } // Check global scripts. - if (ScriptServer::is_global_class(p_identifier)) { - String script = ScriptServer::get_global_class_path(p_identifier); + if (ScriptServer::is_global_class(p_identifier->name)) { + String script = ScriptServer::get_global_class_path(p_identifier->name); if (script.to_lower().ends_with(".gd")) { Error err = OK; Ref<GDScriptParserRef> parser = GDScriptCache::get_parser(script, GDScriptParserRef::INTERFACE_SOLVED, err); @@ -2099,7 +2112,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, return true; } } else { - Ref<Script> scr = ResourceLoader::load(ScriptServer::get_global_class_path(p_identifier)); + Ref<Script> scr = ResourceLoader::load(ScriptServer::get_global_class_path(p_identifier->name)); if (scr.is_valid()) { r_type = _type_from_variant(scr); r_type.type.is_meta_type = true; @@ -2110,20 +2123,20 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, } // Check global variables (including autoloads). - if (GDScriptLanguage::get_singleton()->get_named_globals_map().has(p_identifier)) { - r_type = _type_from_variant(GDScriptLanguage::get_singleton()->get_named_globals_map()[p_identifier]); + if (GDScriptLanguage::get_singleton()->get_named_globals_map().has(p_identifier->name)) { + r_type = _type_from_variant(GDScriptLanguage::get_singleton()->get_named_globals_map()[p_identifier->name]); return true; } // Check ClassDB. - if (ClassDB::class_exists(p_identifier) && ClassDB::is_class_exposed(p_identifier)) { + if (ClassDB::class_exists(p_identifier->name) && ClassDB::is_class_exposed(p_identifier->name)) { r_type.type.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT; r_type.type.kind = GDScriptParser::DataType::NATIVE; - r_type.type.native_type = p_identifier; + r_type.type.native_type = p_identifier->name; r_type.type.is_constant = true; - if (Engine::get_singleton()->has_singleton(p_identifier)) { + if (Engine::get_singleton()->has_singleton(p_identifier->name)) { r_type.type.is_meta_type = false; - r_type.value = Engine::get_singleton()->get_singleton_object(p_identifier); + r_type.value = Engine::get_singleton()->get_singleton_object(p_identifier->name); } else { r_type.type.is_meta_type = true; r_type.value = Variant(); diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index a6f83146a0..0e602561c6 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -2280,6 +2280,9 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_identifier(ExpressionNode IdentifierNode *identifier = alloc_node<IdentifierNode>(); complete_extents(identifier); identifier->name = previous.get_identifier(); +#ifdef DEBUG_ENABLED + identifier->suite = current_suite; +#endif if (current_suite != nullptr && current_suite->has_local(identifier->name)) { const SuiteNode::Local &declaration = current_suite->get_local(identifier->name); diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index dd4e05b613..20f5dcf06d 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -859,19 +859,24 @@ public: struct IdentifierNode : public ExpressionNode { StringName name; +#ifdef DEBUG_ENABLED + SuiteNode *suite = nullptr; // The block in which the identifier is used. +#endif enum Source { UNDEFINED_SOURCE, FUNCTION_PARAMETER, - LOCAL_CONSTANT, LOCAL_VARIABLE, + LOCAL_CONSTANT, LOCAL_ITERATOR, // `for` loop iterator. LOCAL_BIND, // Pattern bind. - MEMBER_SIGNAL, MEMBER_VARIABLE, - STATIC_VARIABLE, MEMBER_CONSTANT, + MEMBER_FUNCTION, + MEMBER_SIGNAL, + MEMBER_CLASS, INHERITED_VARIABLE, + STATIC_VARIABLE, }; Source source = UNDEFINED_SOURCE; diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp index 8de78d2b9a..24aa793c47 100644 --- a/modules/gdscript/gdscript_warning.cpp +++ b/modules/gdscript/gdscript_warning.cpp @@ -136,6 +136,12 @@ String GDScriptWarning::get_message() const { case CONFUSABLE_IDENTIFIER: CHECK_SYMBOLS(1); return vformat(R"(The identifier "%s" has misleading characters and might be confused with something else.)", symbols[0]); + case CONFUSABLE_LOCAL_DECLARATION: + CHECK_SYMBOLS(2); + return vformat(R"(The %s "%s" is declared below in the parent block.)", symbols[0], symbols[1]); + case CONFUSABLE_LOCAL_USAGE: + CHECK_SYMBOLS(1); + return vformat(R"(The identifier "%s" will be shadowed below in the block.)", symbols[0]); case INFERENCE_ON_VARIANT: CHECK_SYMBOLS(1); return vformat("The %s type is being inferred from a Variant value, so it will be typed as Variant.", symbols[0]); @@ -213,6 +219,8 @@ String GDScriptWarning::get_name_from_code(Code p_code) { "DEPRECATED_KEYWORD", "RENAMED_IN_GODOT_4_HINT", "CONFUSABLE_IDENTIFIER", + "CONFUSABLE_LOCAL_DECLARATION", + "CONFUSABLE_LOCAL_USAGE", "INFERENCE_ON_VARIANT", "NATIVE_METHOD_OVERRIDE", "GET_NODE_DEFAULT_WITHOUT_ONREADY", diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h index ae6207fcdc..8444d46a88 100644 --- a/modules/gdscript/gdscript_warning.h +++ b/modules/gdscript/gdscript_warning.h @@ -83,6 +83,8 @@ public: DEPRECATED_KEYWORD, // The keyword is deprecated and should be replaced. RENAMED_IN_GODOT_4_HINT, // A variable or function that could not be found has been renamed in Godot 4. CONFUSABLE_IDENTIFIER, // The identifier contains misleading characters that can be confused. E.g. "usеr" (has Cyrillic "е" instead of Latin "e"). + CONFUSABLE_LOCAL_DECLARATION, // The parent block declares an identifier with the same name below. + CONFUSABLE_LOCAL_USAGE, // The identifier will be shadowed below in the block. INFERENCE_ON_VARIANT, // The declaration uses type inference but the value is typed as Variant. NATIVE_METHOD_OVERRIDE, // The script method overrides a native one, this may not work as intended. GET_NODE_DEFAULT_WITHOUT_ONREADY, // A class variable uses `get_node()` (or the `$` notation) as its default value, but does not use the @onready annotation. @@ -128,6 +130,8 @@ public: WARN, // DEPRECATED_KEYWORD WARN, // RENAMED_IN_GODOT_4_HINT WARN, // CONFUSABLE_IDENTIFIER + WARN, // CONFUSABLE_LOCAL_DECLARATION + WARN, // CONFUSABLE_LOCAL_USAGE ERROR, // INFERENCE_ON_VARIANT // Most likely done by accident, usually inference is trying for a particular type. ERROR, // NATIVE_METHOD_OVERRIDE // May not work as expected. ERROR, // GET_NODE_DEFAULT_WITHOUT_ONREADY // May not work as expected. diff --git a/modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_var_self.gd b/modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_var_self.gd new file mode 100644 index 0000000000..57ae41922f --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_var_self.gd @@ -0,0 +1,4 @@ +var v1 = v1 + +func test(): + print(v1) diff --git a/modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_var_self.out b/modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_var_self.out new file mode 100644 index 0000000000..c337882d9c --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_var_self.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Could not resolve member "v1": Cyclic reference. diff --git a/modules/gdscript/tests/scripts/analyzer/features/typed_array_usage.gd b/modules/gdscript/tests/scripts/analyzer/features/typed_array_usage.gd index c4108f50de..b000c82717 100644 --- a/modules/gdscript/tests/scripts/analyzer/features/typed_array_usage.gd +++ b/modules/gdscript/tests/scripts/analyzer/features/typed_array_usage.gd @@ -126,7 +126,7 @@ func test(): assert(a_objects.get_typed_builtin() == TYPE_OBJECT) assert(a_objects.get_typed_script() == A) - var a_passed = (func check_a_passing(a_objects: Array[A]): return a_objects.size()).call(a_objects) + var a_passed = (func check_a_passing(p_objects: Array[A]): return p_objects.size()).call(a_objects) assert(a_passed == 4) var b_passed = (func check_b_passing(basic: Array): return basic[0] != null).call(b_objects) diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_declaration.gd b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_declaration.gd new file mode 100644 index 0000000000..3178f8d496 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_declaration.gd @@ -0,0 +1,6 @@ +func test(): + if true: + var a = 1 + print(a) + var a = 2 + print(a) diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_declaration.out b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_declaration.out new file mode 100644 index 0000000000..7365072ea7 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_declaration.out @@ -0,0 +1,7 @@ +GDTEST_OK +>> WARNING +>> Line: 3 +>> CONFUSABLE_LOCAL_DECLARATION +>> The variable "a" is declared below in the parent block. +1 +2 diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.gd b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.gd new file mode 100644 index 0000000000..4462c067bc --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.gd @@ -0,0 +1,6 @@ +var a = 1 + +func test(): + print(a) + var a = 2 + print(a) diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.out b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.out new file mode 100644 index 0000000000..0e0d607831 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.out @@ -0,0 +1,11 @@ +GDTEST_OK +>> WARNING +>> Line: 4 +>> CONFUSABLE_LOCAL_USAGE +>> The identifier "a" will be shadowed below in the block. +>> WARNING +>> Line: 5 +>> SHADOWED_VARIABLE +>> The local variable "a" is shadowing an already-declared variable at line 1. +1 +2 diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.gd b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.gd new file mode 100644 index 0000000000..eef8eb66e6 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.gd @@ -0,0 +1,6 @@ +var a = 1 + +func test(): + print(a) + var a = a + 1 + print(a) diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.out b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.out new file mode 100644 index 0000000000..228a510490 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.out @@ -0,0 +1,15 @@ +GDTEST_OK +>> WARNING +>> Line: 4 +>> CONFUSABLE_LOCAL_USAGE +>> The identifier "a" will be shadowed below in the block. +>> WARNING +>> Line: 5 +>> CONFUSABLE_LOCAL_USAGE +>> The identifier "a" will be shadowed below in the block. +>> WARNING +>> Line: 5 +>> SHADOWED_VARIABLE +>> The local variable "a" is shadowing an already-declared variable at line 1. +1 +2 diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.gd b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.gd new file mode 100644 index 0000000000..1f207f27ac --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.gd @@ -0,0 +1,7 @@ +var a = 1 + +func test(): + for _i in 3: + print(a) + var a = 2 + print(a) diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.out b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.out new file mode 100644 index 0000000000..0d20e9f7a0 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.out @@ -0,0 +1,15 @@ +GDTEST_OK +>> WARNING +>> Line: 5 +>> CONFUSABLE_LOCAL_USAGE +>> The identifier "a" will be shadowed below in the block. +>> WARNING +>> Line: 6 +>> SHADOWED_VARIABLE +>> The local variable "a" is shadowing an already-declared variable at line 1. +1 +2 +1 +2 +1 +2 |