Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix analyzer pushing SHADOWED_VARIABLE warning for members shadowed in subclass #98873

Merged
merged 1 commit into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc/classes/ProjectSettings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
48 changes: 31 additions & 17 deletions modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions modules/gdscript/gdscript_warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
4 changes: 2 additions & 2 deletions modules/gdscript/gdscript_warning.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class_name ShadowingBase

const base_const_member = 1
var base_variable_member

func base_function_member():
pass
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class_name ShadowedClass
extends ShadowingBase

var member: int = 0

Expand All @@ -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'
Expand All @@ -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')
32 changes: 22 additions & 10 deletions modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading