diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index 3358e6838c5f..4d8ea28bc6fc 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -561,10 +561,10 @@ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when defining a local or member variable, signal, or enum that would have the same name as a built-in function or global class name, thus shadowing it. - When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when defining a local or member variable that would shadow a member variable that the class defines. + When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a local variable or local constant shadows a member declared in the current class. - When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when defining a local or subclass member variable that would shadow a variable that is inherited from a parent class. + When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a local variable or local constant shadows a member declared in a base class. When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when calling an expression that may have no effect on the surrounding code, such as writing [code]2 + 2[/code] as a statement. diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 93d4a512a923..6241ada06a12 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -5809,8 +5809,6 @@ void GDScriptAnalyzer::validate_call_arg(const List &p #ifdef DEBUG_ENABLED 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; { List gdscript_funcs; @@ -5838,37 +5836,53 @@ void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_identifier } } + const GDScriptParser::DataType current_class_type = parser->current_class->get_datatype(); if (p_in_local_scope) { - while (base_class != nullptr) { + GDScriptParser::ClassNode *base_class = current_class_type.class_type; + + if (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; } + + while (base_class != nullptr) { + if (base_class->has_member(name)) { + String base_class_name = base_class->get_global_name(); + if (base_class_name.is_empty()) { + base_class_name = base_class->fqcn; + } + + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, base_class->get_member(name).get_type_name(), itos(base_class->get_member(name).get_line()), base_class_name); + return; + } + base_class = base_class->base_type.class_type; + } } - StringName parent = base.native_type; - while (parent != StringName()) { - ERR_FAIL_COND_MSG(!class_exists(parent), "Non-existent native base class."); + StringName native_base_class = current_class_type.native_type; + while (native_base_class != StringName()) { + ERR_FAIL_COND_MSG(!class_exists(native_base_class), "Non-existent native base class."); - if (ClassDB::has_method(parent, name, true)) { - parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "method", parent); + if (ClassDB::has_method(native_base_class, name, true)) { + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "method", native_base_class); return; - } else if (ClassDB::has_signal(parent, name, true)) { - parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "signal", parent); + } else if (ClassDB::has_signal(native_base_class, name, true)) { + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "signal", native_base_class); return; - } else if (ClassDB::has_property(parent, name, true)) { - parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "property", parent); + } else if (ClassDB::has_property(native_base_class, name, true)) { + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "property", native_base_class); return; - } else if (ClassDB::has_integer_constant(parent, name, true)) { - parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "constant", parent); + } else if (ClassDB::has_integer_constant(native_base_class, name, true)) { + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "constant", native_base_class); return; - } else if (ClassDB::has_enum(parent, name, true)) { - parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "enum", parent); + } else if (ClassDB::has_enum(native_base_class, name, true)) { + parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, "enum", native_base_class); return; } - parent = ClassDB::get_parent_class(parent); + native_base_class = ClassDB::get_parent_class(native_base_class); } } #endif // DEBUG_ENABLED diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp index 4ffb4bd9d13f..a601cc499328 100644 --- a/modules/gdscript/gdscript_warning.cpp +++ b/modules/gdscript/gdscript_warning.cpp @@ -61,10 +61,13 @@ String GDScriptWarning::get_message() const { return vformat(R"(The signal "%s" is declared but never explicitly used in the class.)", symbols[0]); case SHADOWED_VARIABLE: CHECK_SYMBOLS(4); - return vformat(R"(The local %s "%s" is shadowing an already-declared %s at line %s.)", symbols[0], symbols[1], symbols[2], symbols[3]); + return vformat(R"(The local %s "%s" is shadowing an already-declared %s at line %s in the current class.)", symbols[0], symbols[1], symbols[2], symbols[3]); case SHADOWED_VARIABLE_BASE_CLASS: CHECK_SYMBOLS(4); - return vformat(R"(The local %s "%s" is shadowing an already-declared %s at the base class "%s".)", symbols[0], symbols[1], symbols[2], symbols[3]); + if (symbols.size() > 4) { + return vformat(R"(The local %s "%s" is shadowing an already-declared %s at line %s in the base class "%s".)", symbols[0], symbols[1], symbols[2], symbols[3], symbols[4]); + } + return vformat(R"(The local %s "%s" is shadowing an already-declared %s in the base class "%s".)", symbols[0], symbols[1], symbols[2], symbols[3]); case SHADOWED_GLOBAL_IDENTIFIER: CHECK_SYMBOLS(3); return vformat(R"(The %s "%s" has the same name as a %s.)", symbols[0], symbols[1], symbols[2]); diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h index ffcf00a83014..99e9b30af579 100644 --- a/modules/gdscript/gdscript_warning.h +++ b/modules/gdscript/gdscript_warning.h @@ -53,8 +53,8 @@ class GDScriptWarning { UNUSED_PRIVATE_CLASS_VARIABLE, // Class variable is declared private ("_" prefix) but never used in the class. UNUSED_PARAMETER, // Function parameter is never used. UNUSED_SIGNAL, // Signal is defined but never explicitly used in the class. - SHADOWED_VARIABLE, // Variable name shadowed by other variable in same class. - SHADOWED_VARIABLE_BASE_CLASS, // Variable name shadowed by other variable in some base class. + SHADOWED_VARIABLE, // A local variable/constant shadows a current class member. + SHADOWED_VARIABLE_BASE_CLASS, // A local variable/constant shadows a base class member. SHADOWED_GLOBAL_IDENTIFIER, // A global class or function has the same name as variable. UNREACHABLE_CODE, // Code after a return statement. UNREACHABLE_PATTERN, // Pattern in a match statement after a catch all pattern (wildcard or bind). diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.out b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.out index 0e0d60783137..cfe91e00bde6 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.out +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage.out @@ -6,6 +6,6 @@ GDTEST_OK >> WARNING >> Line: 5 >> SHADOWED_VARIABLE ->> The local variable "a" is shadowing an already-declared variable at line 1. +>> The local variable "a" is shadowing an already-declared variable at line 1 in the current class. 1 2 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 index 228a51049030..ae0f2d8b8be2 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.out +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_initializer.out @@ -10,6 +10,6 @@ GDTEST_OK >> WARNING >> Line: 5 >> SHADOWED_VARIABLE ->> The local variable "a" is shadowing an already-declared variable at line 1. +>> The local variable "a" is shadowing an already-declared variable at line 1 in the current class. 1 2 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 index 0d20e9f7a0b1..101d27df9d1b 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.out +++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_local_usage_loop.out @@ -6,7 +6,7 @@ GDTEST_OK >> WARNING >> Line: 6 >> SHADOWED_VARIABLE ->> The local variable "a" is shadowing an already-declared variable at line 1. +>> The local variable "a" is shadowing an already-declared variable at line 1 in the current class. 1 2 1 diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/lambda_shadowing_arg.out b/modules/gdscript/tests/scripts/analyzer/warnings/lambda_shadowing_arg.out index a98d80514c5b..5d059b919372 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/lambda_shadowing_arg.out +++ b/modules/gdscript/tests/scripts/analyzer/warnings/lambda_shadowing_arg.out @@ -2,5 +2,5 @@ GDTEST_OK >> WARNING >> Line: 4 >> SHADOWED_VARIABLE ->> The local function parameter "shadow" is shadowing an already-declared variable at line 1. +>> The local function parameter "shadow" is shadowing an already-declared variable at line 1 in the current class. shadow diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/shadowing_base.notest.gd b/modules/gdscript/tests/scripts/analyzer/warnings/shadowing_base.notest.gd new file mode 100644 index 000000000000..5819246ded4e --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/shadowing_base.notest.gd @@ -0,0 +1,7 @@ +class_name ShadowingBase + +const base_const_member = 1 +var base_variable_member + +func base_function_member(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd index 939e787ea5de..6a16ae6bcc0b 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd +++ b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd @@ -1,4 +1,5 @@ class_name ShadowedClass +extends ShadowingBase var member: int = 0 @@ -7,6 +8,7 @@ var print_debug := 'print_debug' var print := 'print' @warning_ignore("unused_variable") +@warning_ignore("unused_local_constant") func test(): var Array := 'Array' var Node := 'Node' @@ -15,5 +17,8 @@ func test(): var member := 'member' var reference := 'reference' var ShadowedClass := 'ShadowedClass' + var base_variable_member + const base_function_member = 1 + var base_const_member print('warn') diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out index 8297eed4b8e1..075f5d32250e 100644 --- a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out +++ b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out @@ -1,34 +1,46 @@ GDTEST_OK >> WARNING ->> Line: 5 +>> Line: 6 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "print_debug" has the same name as a built-in function. >> WARNING ->> Line: 11 +>> Line: 13 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "Array" has the same name as a built-in type. >> WARNING ->> Line: 12 +>> Line: 14 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "Node" has the same name as a native class. >> WARNING ->> Line: 13 +>> Line: 15 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "is_same" has the same name as a built-in function. >> WARNING ->> Line: 14 +>> Line: 16 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "sqrt" has the same name as a built-in function. >> WARNING ->> Line: 15 +>> Line: 17 >> SHADOWED_VARIABLE ->> The local variable "member" is shadowing an already-declared variable at line 3. +>> The local variable "member" is shadowing an already-declared variable at line 4 in the current class. >> WARNING ->> Line: 16 +>> Line: 18 >> SHADOWED_VARIABLE_BASE_CLASS ->> The local variable "reference" is shadowing an already-declared method at the base class "RefCounted". +>> The local variable "reference" is shadowing an already-declared method in the base class "RefCounted". >> WARNING ->> Line: 17 +>> Line: 19 >> SHADOWED_GLOBAL_IDENTIFIER >> The variable "ShadowedClass" has the same name as a global class defined in "shadowning.gd". +>> WARNING +>> Line: 20 +>> SHADOWED_VARIABLE_BASE_CLASS +>> The local variable "base_variable_member" is shadowing an already-declared variable at line 4 in the base class "ShadowingBase". +>> WARNING +>> Line: 21 +>> SHADOWED_VARIABLE_BASE_CLASS +>> The local constant "base_function_member" is shadowing an already-declared function at line 6 in the base class "ShadowingBase". +>> WARNING +>> Line: 22 +>> SHADOWED_VARIABLE_BASE_CLASS +>> The local variable "base_const_member" is shadowing an already-declared constant at line 3 in the base class "ShadowingBase". warn diff --git a/modules/gdscript/tests/scripts/parser/warnings/shadowed_constant.out b/modules/gdscript/tests/scripts/parser/warnings/shadowed_constant.out index 75fa01f9282d..04df229f664f 100644 --- a/modules/gdscript/tests/scripts/parser/warnings/shadowed_constant.out +++ b/modules/gdscript/tests/scripts/parser/warnings/shadowed_constant.out @@ -6,4 +6,4 @@ GDTEST_OK >> WARNING >> Line: 8 >> SHADOWED_VARIABLE ->> The local constant "TEST" is shadowing an already-declared constant at line 2. +>> The local constant "TEST" is shadowing an already-declared constant at line 2 in the current class. diff --git a/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_class.out b/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_class.out index aab27e78e2c5..4a6964f503c7 100644 --- a/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_class.out +++ b/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_class.out @@ -6,4 +6,4 @@ GDTEST_OK >> WARNING >> Line: 8 >> SHADOWED_VARIABLE ->> The local variable "foo" is shadowing an already-declared variable at line 1. +>> The local variable "foo" is shadowing an already-declared variable at line 1 in the current class. diff --git a/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_function.out b/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_function.out index e3cd358126f5..45fb7718295c 100644 --- a/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_function.out +++ b/modules/gdscript/tests/scripts/parser/warnings/shadowed_variable_function.out @@ -6,4 +6,4 @@ GDTEST_OK >> WARNING >> Line: 2 >> SHADOWED_VARIABLE ->> The local variable "test" is shadowing an already-declared function at line 1. +>> The local variable "test" is shadowing an already-declared function at line 1 in the current class. diff --git a/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out b/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out index 5492c8f76399..1650acadb569 100644 --- a/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out +++ b/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out @@ -2,11 +2,11 @@ GDTEST_OK >> WARNING >> Line: 5 >> SHADOWED_VARIABLE ->> The local function parameter "a" is shadowing an already-declared variable at line 3. +>> The local function parameter "a" is shadowing an already-declared variable at line 3 in the current class. >> WARNING >> Line: 15 >> SHADOWED_VARIABLE ->> The local function parameter "v" is shadowing an already-declared variable at line 13. +>> The local function parameter "v" is shadowing an already-declared variable at line 13 in the current class. a 1 b