Skip to content

Commit

Permalink
Fix analyzer pushing SHADOWED_VARIABLE warning for members shadowed i…
Browse files Browse the repository at this point in the history
…n subclasses

This fixes a bug in the analyzer where it did not push the SHADOWED_VARIABLE_BASE_CLASS
warning for members shadowed by variable in subclass. It does this by comparing the class
which contains the shadowed member with the class containing the variable, and pushing
SHADOWED_VARIABLE only if the classes are the same. Additionally, SHADOWED_VARIABLE_BASE_CLASS
can take an extra symbol which helps to specify the line for non native base class.
  • Loading branch information
girdenis-p committed Nov 6, 2024
1 parent b00e1cb commit aae9172
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 9 deletions.
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 defining a local or class variable that would shadow a member that the class defines.
</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 defining a local or subclass variable that would shadow a member that is inherited from a parent 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
19 changes: 15 additions & 4 deletions modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5839,12 +5839,23 @@ void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_identifier
}

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()));
GDScriptParser::ClassNode *current_parent_class = base_class;

while (current_parent_class != nullptr) {
if (current_parent_class->has_member(name)) {
if (current_parent_class != base_class) {
String current_class_name = current_parent_class->fqcn;
if (current_parent_class->identifier != nullptr) {
current_class_name = current_parent_class->identifier->name;
}

parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_identifier->name, current_parent_class->get_member(name).get_type_name(), itos(current_parent_class->get_member(name).get_line()), current_class_name);
return;
}
parser->push_warning(p_identifier, GDScriptWarning::SHADOWED_VARIABLE, p_context, p_identifier->name, current_parent_class->get_member(name).get_type_name(), itos(current_parent_class->get_member(name).get_line()));
return;
}
base_class = base_class->base_type.class_type;
current_parent_class = current_parent_class->base_type.class_type;
}
}

Expand Down
5 changes: 4 additions & 1 deletion modules/gdscript/gdscript_warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ String GDScriptWarning::get_message() const {
return vformat(R"(The local %s "%s" is shadowing an already-declared %s at line %s.)", 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() < 5) {
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]);
}
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]);
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, // Member shadowed by other variable in same class.
SHADOWED_VARIABLE_BASE_CLASS, // Variable shadows other member in some base class.
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
@@ -0,0 +1,16 @@
class A:
var foo1

func foo2():
pass

class B extends A:
func test1(foo1):
return foo1

func test2():
var foo2 = 1
return foo2

func test():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
GDTEST_OK
>> WARNING
>> Line: 8
>> SHADOWED_VARIABLE_BASE_CLASS
>> The local function parameter "foo1" is shadowing an already-declared variable at line 2 in the base class "A".
>> WARNING
>> Line: 12
>> SHADOWED_VARIABLE_BASE_CLASS
>> The local variable "foo2" is shadowing an already-declared function at line 4 in the base class "A".

0 comments on commit aae9172

Please sign in to comment.