From 703274fd04576dda9a3fa3755f1919d20084f744 Mon Sep 17 00:00:00 2001 From: Dmitrii Maganov Date: Wed, 8 Mar 2023 22:06:29 +0200 Subject: [PATCH] GDScript: Fix missing warning for shadowing of built-in types --- modules/gdscript/gdscript_analyzer.cpp | 28 ++++++++++--------- modules/gdscript/gdscript_analyzer.h | 2 +- .../scripts/analyzer/warnings/shadowning.gd | 12 ++++++++ .../scripts/analyzer/warnings/shadowning.out | 26 +++++++++++++++++ 4 files changed, 54 insertions(+), 14 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 38d5ae6b77c..d83b74da681 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -4778,7 +4778,7 @@ void GDScriptAnalyzer::validate_call_arg(const List &p } #ifdef DEBUG_ENABLED -bool GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context) { +void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context) { const StringName &name = p_local->name; GDScriptParser::DataType base = parser->current_class->get_datatype(); GDScriptParser::ClassNode *base_class = base.class_type; @@ -4790,50 +4790,52 @@ bool 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"); - return true; + return; } } + if (Variant::has_utility_function(name)) { parser->push_warning(p_local, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "built-in function"); - return true; + return; } else if (ClassDB::class_exists(name)) { parser->push_warning(p_local, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, p_context, name, "global class"); - return true; + 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"); + 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 true; + return; } base_class = base_class->base_type.class_type; } StringName parent = base.native_type; while (parent != StringName()) { - ERR_FAIL_COND_V_MSG(!class_exists(parent), false, "Non-existent native base class."); + 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); - return true; + 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); - return true; + 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); - return true; + 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); - return true; + 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); - return true; + return; } parent = ClassDB::get_parent_class(parent); } - - return false; } #endif diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h index 7a50b32d4c8..5902035bcdd 100644 --- a/modules/gdscript/gdscript_analyzer.h +++ b/modules/gdscript/gdscript_analyzer.h @@ -131,7 +131,7 @@ class GDScriptAnalyzer { Ref 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 - bool is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context); + void is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context); #endif public: diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd new file mode 100644 index 00000000000..61945c9c8f4 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.gd @@ -0,0 +1,12 @@ +var member: int = 0 + +@warning_ignore("unused_variable") +func test(): + var Array := 'Array' + var Node := 'Node' + var is_same := 'is_same' + var sqrt := 'sqrt' + var member := 'member' + var reference := 'reference' + + print('warn') diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out new file mode 100644 index 00000000000..9d0e5675347 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/shadowning.out @@ -0,0 +1,26 @@ +GDTEST_OK +>> WARNING +>> Line: 5 +>> SHADOWED_GLOBAL_IDENTIFIER +>> The variable 'Array' has the same name as a built-in type. +>> WARNING +>> Line: 6 +>> SHADOWED_GLOBAL_IDENTIFIER +>> The variable 'Node' has the same name as a global class. +>> WARNING +>> Line: 7 +>> SHADOWED_GLOBAL_IDENTIFIER +>> The variable 'is_same' has the same name as a built-in function. +>> WARNING +>> Line: 8 +>> SHADOWED_GLOBAL_IDENTIFIER +>> The variable 'sqrt' has the same name as a built-in function. +>> WARNING +>> Line: 9 +>> SHADOWED_VARIABLE +>> The local variable "member" is shadowing an already-declared variable at line 1. +>> WARNING +>> Line: 10 +>> SHADOWED_VARIABLE_BASE_CLASS +>> The local variable "reference" is shadowing an already-declared method at the base class "RefCounted". +warn