From 13c73500ab9b09c5b946968716bef03a814fb65d Mon Sep 17 00:00:00 2001 From: jpcerrone Date: Mon, 3 Apr 2023 10:57:41 -0300 Subject: [PATCH] Fix for not being able to ignore shadowing warnings on class scope --- modules/gdscript/gdscript_analyzer.cpp | 47 +++++++++---------- modules/gdscript/gdscript_analyzer.h | 2 +- modules/gdscript/gdscript_parser.cpp | 14 ------ .../features/allow_get_node_with_onready.gd | 4 +- .../features/allow_get_node_with_onready.out | 2 +- .../scripts/analyzer/warnings/shadowning.gd | 4 ++ .../scripts/analyzer/warnings/shadowning.out | 16 ++++--- 7 files changed, 41 insertions(+), 48 deletions(-) diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index d3445b8cc0c8..5498864c3b7d 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1553,7 +1553,7 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode * } parser->push_warning(p_function->parameters[i]->identifier, GDScriptWarning::UNUSED_PARAMETER, visible_name, p_function->parameters[i]->identifier->name); } - is_shadowing(p_function->parameters[i]->identifier, "function parameter"); + is_shadowing(p_function->parameters[i]->identifier, "function parameter", true); #endif // DEBUG_ENABLED #ifdef TOOLS_ENABLED if (p_function->parameters[i]->initializer) { @@ -1874,9 +1874,8 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable } else if (p_variable->assignments == 0) { parser->push_warning(p_variable, GDScriptWarning::UNASSIGNED_VARIABLE, p_variable->identifier->name); } - - is_shadowing(p_variable->identifier, kind); } + is_shadowing(p_variable->identifier, kind, p_is_local); #endif } @@ -1889,9 +1888,8 @@ void GDScriptAnalyzer::resolve_constant(GDScriptParser::ConstantNode *p_constant if (p_constant->usages == 0) { parser->push_warning(p_constant, GDScriptWarning::UNUSED_LOCAL_CONSTANT, p_constant->identifier->name); } - - is_shadowing(p_constant->identifier, kind); } + is_shadowing(p_constant->identifier, kind, p_is_local); #endif } @@ -2052,7 +2050,7 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) { p_for->set_datatype(p_for->loop->get_datatype()); #ifdef DEBUG_ENABLED if (p_for->variable) { - is_shadowing(p_for->variable, R"("for" iterator variable)"); + is_shadowing(p_for->variable, R"("for" iterator variable)", true); } #endif } @@ -2148,7 +2146,7 @@ void GDScriptAnalyzer::resolve_match_pattern(GDScriptParser::PatternNode *p_matc } p_match_pattern->bind->set_datatype(result); #ifdef DEBUG_ENABLED - is_shadowing(p_match_pattern->bind, "pattern bind"); + is_shadowing(p_match_pattern->bind, "pattern bind", true); if (p_match_pattern->bind->usages == 0 && !String(p_match_pattern->bind->name).begins_with("_")) { parser->push_warning(p_match_pattern->bind, GDScriptWarning::UNUSED_VARIABLE, p_match_pattern->bind->name); } @@ -4890,8 +4888,8 @@ void GDScriptAnalyzer::validate_call_arg(const List &p } #ifdef DEBUG_ENABLED -void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context) { - const StringName &name = p_local->name; +void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_identifier, const String &p_context, const bool p_in_local_scope) { + const StringName &name = p_identifier->name; GDScriptParser::DataType base = parser->current_class->get_datatype(); GDScriptParser::ClassNode *base_class = base.class_type; @@ -4901,29 +4899,30 @@ void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_local, con for (MethodInfo &info : gdscript_funcs) { if (info.name == name) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "built-in function"); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "built-in function"); return; } } - if (Variant::has_utility_function(name)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "built-in function"); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "built-in function"); return; } else if (ClassDB::class_exists(name)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "global class"); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "global class"); return; } else if (GDScriptParser::get_builtin_type(name) != Variant::VARIANT_MAX) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "built-in type"); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "built-in type"); return; } } - while (base_class != nullptr) { - if (base_class->has_member(name)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE, p_context, p_local->name, base_class->get_member(name).get_type_name(), itos(base_class->get_member(name).get_line())); - return; + if (p_in_local_scope) { + while (base_class != nullptr) { + if (base_class->has_member(name)) { + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE, p_context, p_identifier->name, base_class->get_member(name).get_type_name(), itos(base_class->get_member(name).get_line())); + return; + } + base_class = base_class->base_type.class_type; } - base_class = base_class->base_type.class_type; } StringName parent = base.native_type; @@ -4931,19 +4930,19 @@ void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_local, con ERR_FAIL_COND_MSG(!class_exists(parent), "Non-existent native base class."); if (ClassDB::has_method(parent, name, true)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "method", parent); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "method", parent); return; } else if (ClassDB::has_signal(parent, name, true)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "signal", parent); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "signal", parent); return; } else if (ClassDB::has_property(parent, name, true)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "property", parent); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "property", parent); return; } else if (ClassDB::has_integer_constant(parent, name, true)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "constant", parent); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "constant", parent); return; } else if (ClassDB::has_enum(parent, name, true)) { - parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "enum", parent); + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "enum", parent); return; } parent = ClassDB::get_parent_class(parent); diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h index 195e23b503bb..a2845a360c74 100644 --- a/modules/gdscript/gdscript_analyzer.h +++ b/modules/gdscript/gdscript_analyzer.h @@ -133,7 +133,7 @@ class GDScriptAnalyzer { Ref get_parser_for(const String &p_path); void reduce_identifier_from_base_set_class(GDScriptParser::IdentifierNode *p_identifier, GDScriptParser::DataType p_identifier_datatype); #ifdef DEBUG_ENABLED - void is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context); + void is_shadowing(GDScriptParser::IdentifierNode *p_identifier, const String &p_context, const bool p_in_local_scope); #endif public: diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 5c2d4a060cb1..2b91ba8f860f 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -783,20 +783,6 @@ void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)(b if (member->identifier != nullptr) { if (!((String)member->identifier->name).is_empty()) { // Enums may be unnamed. - -#ifdef DEBUG_ENABLED - List gdscript_funcs; - GDScriptLanguage::get_singleton()->get_public_functions(&gdscript_funcs); - for (MethodInfo &info : gdscript_funcs) { - if (info.name == member->identifier->name) { - push_warning(member->identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_member_kind, member->identifier->name, "built-in function"); - } - } - if (Variant::has_utility_function(member->identifier->name)) { - push_warning(member->identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_member_kind, member->identifier->name, "built-in function"); - } -#endif - if (current_class->members_indices.has(member->identifier->name)) { push_error(vformat(R"(%s "%s" has the same name as a previously declared %s.)", p_member_kind.capitalize(), member->identifier->name, current_class->get_member(member->identifier->name).get_type_name()), member->identifier); } else { diff --git a/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.gd b/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.gd index a9004a346b8f..3e647407cd3b 100644 --- a/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.gd +++ b/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.gd @@ -1,7 +1,7 @@ extends Node @onready var shorthand = $Node -@onready var call = get_node(^"Node") +@onready var call_no_cast = get_node(^"Node") @onready var shorthand_with_cast = $Node as Node @onready var call_with_cast = get_node(^"Node") as Node @@ -13,6 +13,6 @@ func _init(): func test(): # Those are expected to be `null` since `_ready()` is never called on tests. prints("shorthand", shorthand) - prints("call", call) + prints("call_no_cast", call_no_cast) prints("shorthand_with_cast", shorthand_with_cast) prints("call_with_cast", call_with_cast) diff --git a/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.out b/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.out index eddc2deec0c1..78b830aad091 100644 --- a/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.out +++ b/modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.out @@ -1,5 +1,5 @@ GDTEST_OK shorthand -call +call_no_cast shorthand_with_cast call_with_cast diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd index 61945c9c8f47..29239a19dacd 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd +++ b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd @@ -1,5 +1,9 @@ var member: int = 0 +var print_debug := 'print_debug' +@warning_ignore("shadowed_global_identifier") +var print := 'print' + @warning_ignore("unused_variable") func test(): var Array := 'Array' diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out index 8467697a9630..accc791d8a1e 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out +++ b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out @@ -1,26 +1,30 @@ GDTEST_OK >> WARNING ->> Line: 5 +>> Line: 3 +>> SHADOWED_GLOBAL_IDENTIFIER +>> The variable "print_debug" has the same name as a built-in function. +>> WARNING +>> Line: 9 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "Array" has the same name as a built-in type. >> WARNING ->> Line: 6 +>> Line: 10 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "Node" has the same name as a global class. >> WARNING ->> Line: 7 +>> Line: 11 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "is_same" has the same name as a built-in function. >> WARNING ->> Line: 8 +>> Line: 12 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "sqrt" has the same name as a built-in function. >> WARNING ->> Line: 9 +>> Line: 13 >> SHADOWED_VARIABLE >> The local variable "member" is shadowing an already-declared variable at line 1. >> WARNING ->> Line: 10 +>> Line: 14 >> SHADOWED_VARIABLE_BASE_CLASS >> The local variable "reference" is shadowing an already-declared method at the base class "RefCounted". warn