From 703274fd04576dda9a3fa3755f1919d20084f744 Mon Sep 17 00:00:00 2001
From: Dmitrii Maganov <vonagam@gmail.com>
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<GDScriptParser::DataType> &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<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
-	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