Skip to content

Commit

Permalink
Fix for not being able to ignore shadowing warnings on class scope
Browse files Browse the repository at this point in the history
  • Loading branch information
jpcerrone committed Apr 3, 2023
1 parent ba27db3 commit 68463bf
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 48 deletions.
47 changes: 23 additions & 24 deletions modules/gdscript/gdscript_analyzer.cpp
Expand Up @@ -1496,7 +1496,7 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
if (p_function->parameters[i]->usages == 0 && !String(p_function->parameters[i]->identifier->name).begins_with("_")) {
parser->push_warning(p_function->parameters[i]->identifier, GDScriptWarning::UNUSED_PARAMETER, function_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) {
Expand Down Expand Up @@ -1801,9 +1801,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
}

Expand All @@ -1816,9 +1815,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
}

Expand Down Expand Up @@ -1980,7 +1978,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
}
Expand Down Expand Up @@ -2076,7 +2074,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);
}
Expand Down Expand Up @@ -4778,8 +4776,8 @@ void GDScriptAnalyzer::validate_call_arg(const List<GDScriptParser::DataType> &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;

Expand All @@ -4789,49 +4787,50 @@ 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;
while (parent != StringName()) {
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);
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_analyzer.h
Expand Up @@ -131,7 +131,7 @@ class GDScriptAnalyzer {
Ref<GDScriptParserRef> 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:
Expand Down
14 changes: 0 additions & 14 deletions modules/gdscript/gdscript_parser.cpp
Expand Up @@ -772,20 +772,6 @@ void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)()

if (member->identifier != nullptr) {
if (!((String)member->identifier->name).is_empty()) { // Enums may be unnamed.

#ifdef DEBUG_ENABLED
List<MethodInfo> 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 {
Expand Down
@@ -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

Expand All @@ -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)
@@ -1,5 +1,5 @@
GDTEST_OK
shorthand <null>
call <null>
call_no_cast <null>
shorthand_with_cast <null>
call_with_cast <null>
@@ -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'
Expand Down
16 changes: 10 additions & 6 deletions 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

0 comments on commit 68463bf

Please sign in to comment.