From 97280279ee45b628f9786548f6d7979836fd6ac5 Mon Sep 17 00:00:00 2001
From: Dmitrii Maganov <vonagam@gmail.com>
Date: Mon, 5 Dec 2022 20:24:11 +0200
Subject: [PATCH] Fix incomplete shadowing of member properties by parameters

---
 modules/gdscript/gdscript_compiler.cpp        | 10 +++++---
 modules/gdscript/gdscript_compiler.h          |  1 +
 .../runtime/features/parameter_shadowing.gd   | 25 +++++++++++++++++++
 .../runtime/features/parameter_shadowing.out  | 17 +++++++++++++
 4 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.gd
 create mode 100644 modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out

diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
index 24241b712b9..ea93e1ebfcc 100644
--- a/modules/gdscript/gdscript_compiler.cpp
+++ b/modules/gdscript/gdscript_compiler.cpp
@@ -43,7 +43,7 @@ bool GDScriptCompiler::_is_class_member_property(CodeGen &codegen, const StringN
 		return false;
 	}
 
-	if (codegen.parameters.has(p_name) || codegen.locals.has(p_name)) {
+	if (_is_local_or_parameter(codegen, p_name)) {
 		return false; //shadowed
 	}
 
@@ -65,6 +65,10 @@ bool GDScriptCompiler::_is_class_member_property(GDScript *owner, const StringNa
 	return ClassDB::has_property(nc->get_name(), p_name);
 }
 
+bool GDScriptCompiler::_is_local_or_parameter(CodeGen &codegen, const StringName &p_name) {
+	return codegen.parameters.has(p_name) || codegen.locals.has(p_name);
+}
+
 void GDScriptCompiler::_set_error(const String &p_error, const GDScriptParser::Node *p_node) {
 	if (!error.is_empty()) {
 		return;
@@ -920,7 +924,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 								StringName var_name = identifier->name;
 								if (_is_class_member_property(codegen, var_name)) {
 									assign_class_member_property = var_name;
-								} else if (!codegen.locals.has(var_name) && codegen.script->member_indices.has(var_name)) {
+								} else if (!_is_local_or_parameter(codegen, var_name) && codegen.script->member_indices.has(var_name)) {
 									is_member_property = true;
 									member_property_setter_function = codegen.script->member_indices[var_name].setter;
 									member_property_has_setter = member_property_setter_function != StringName();
@@ -1131,7 +1135,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 				bool is_in_setter = false;
 				StringName setter_function;
 				StringName var_name = static_cast<const GDScriptParser::IdentifierNode *>(assignment->assignee)->name;
-				if (!codegen.locals.has(var_name) && codegen.script->member_indices.has(var_name)) {
+				if (!_is_local_or_parameter(codegen, var_name) && codegen.script->member_indices.has(var_name)) {
 					is_member = true;
 					setter_function = codegen.script->member_indices[var_name].setter;
 					has_setter = setter_function != StringName();
diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h
index 45ca4fe3423..cba585e5a56 100644
--- a/modules/gdscript/gdscript_compiler.h
+++ b/modules/gdscript/gdscript_compiler.h
@@ -115,6 +115,7 @@ class GDScriptCompiler {
 
 	bool _is_class_member_property(CodeGen &codegen, const StringName &p_name);
 	bool _is_class_member_property(GDScript *owner, const StringName &p_name);
+	bool _is_local_or_parameter(CodeGen &codegen, const StringName &p_name);
 
 	void _set_error(const String &p_error, const GDScriptParser::Node *p_node);
 
diff --git a/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.gd b/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.gd
new file mode 100644
index 00000000000..f33ba7dffdd
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.gd
@@ -0,0 +1,25 @@
+# https://github.com/godotengine/godot/pull/69620
+
+var a: int = 1
+
+func shadow_regular_assignment(a: Variant, b: Variant) -> void:
+  print(a)
+  print(self.a)
+  a = b
+  print(a)
+  print(self.a)
+
+
+var v := Vector2(0.0, 0.0)
+
+func shadow_subscript_assignment(v: Vector2, x: float) -> void:
+  print(v)
+  print(self.v)
+  v.x += x
+  print(v)
+  print(self.v)
+
+
+func test():
+  shadow_regular_assignment('a', 'b')
+  shadow_subscript_assignment(Vector2(1.0, 1.0), 5.0)
diff --git a/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out b/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out
new file mode 100644
index 00000000000..5b981bc8bb8
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/parameter_shadowing.out
@@ -0,0 +1,17 @@
+GDTEST_OK
+>> WARNING
+>> Line: 5
+>> SHADOWED_VARIABLE
+>> The local function parameter "a" is shadowing an already-declared variable at line 3.
+>> WARNING
+>> Line: 15
+>> SHADOWED_VARIABLE
+>> The local function parameter "v" is shadowing an already-declared variable at line 13.
+a
+1
+b
+1
+(1, 1)
+(0, 0)
+(6, 1)
+(0, 0)