Merge pull request #98873 from girdenis-p/shadowed-variable-warning

Fix analyzer pushing `SHADOWED_VARIABLE` warning for members shadowed in subclass

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.
 		</member>
 		<member name="debug/gdscript/warnings/shadowed_variable" type="int" setter="" getter="" default="1">
-			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.
 		</member>
 		<member name="debug/gdscript/warnings/shadowed_variable_base_class" type="int" setter="" getter="" default="1">
-			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.
 		</member>
 		<member name="debug/gdscript/warnings/standalone_expression" type="int" setter="" getter="" default="1">
 			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.

modules/gdscript/gdscript_analyzer.cpp

@@ -5809,8 +5809,6 @@ void GDScriptAnalyzer::validate_call_arg(const List<GDScriptParser::DataType> &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<MethodInfo> 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

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]);

modules/gdscript/gdscript_warning.h

@@ -53,8 +53,8 @@ public:
 		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).

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

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

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

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

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

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')

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

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.

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.

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.

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