GDScript parser errors on shadowed variables.

[jonbonazza]

Godot version:

3.2.2 stable

OS/device including version:

Windows 10 64-bit, GPU and renderer unrelated.

Issue description:

When upgrading from 3.2.1 to 3.2.2, the GDScript parser now errors when variables are shadowed. This breaks backwards compatibility, as shadowing a variable is a common practice (for better or worse) in many code bases.

Steps to reproduce:
Shadow a variable in a GDScript file.

Minimal reproduction project:

The nakama-godot project has this issue:
https://github.com/heroiclabs/nakama-godot/blob/3e4014a9321bb59bf8d76a9a4d65a3e0ee99738d/addons/com.heroiclabs.nakama/utils/NakamaSerializer.gd#L41
(The k variable in the linked line shadows the k variable on line 9 and in 3.2.2 this is considered an error.

[ThakeeNathees]

GDScript always prevent from re defining LOCAL variables inside the inner scope like in a while block, match block (both declaring inside or as a binding name), if block etc. but in for loop it wasn't. which makes the language inconsistence with it's grammar. and now it's fixed.

[jonbonazza]

If this is by design then I suppose that this issue can be closed, however I do want to voice that I find this decision to be less than ideal. For one, it's confusing to people coming from other languages. While variable shadowing is generally ill-considered, I don't know of any major programming language that strait up disallows it. Additionally, it does break backwards compatibility, so it would have been better to make this change in 4.0.

At the very least, it should be messaged to users that this change was enacted so developers can know what to expect.

[akien-mga]

This is indeed by design, though if it's arguable whether this was a good decision to make in the stable branch (I didn't fully grasp the implication when cherry-picking the change).

But indeed, shadowing local variable doesn't work in GDScript in its current implementation, and the lack of error on for loops was not a feature but a bug.

In 3.2.1:

var i = 42

func _ready():
	print(i) # 42
	
	var i = 981
	print(i) # 981
	
	for i in range(4):
		print(i) # 0, 1, 2, 3
	
	print(i) # 981

	while true:
		var i = 13 # Error, already defined.
		print(i)
		break

	if true:
		var i = 71 # Error, already defined.
		print(i)

In 3.2.2, the for loop now errors like if or while already did.

It's a good bugfix as some apparently tried to use this construct as a way to capture the last value of the for iterator in the local variable, which did not work as shown above (before and after the for loop, i is 981). (I just tested and that construct seems to work in Python 3, which seems... unconventional).

Note that shadowing global/member variables seems to work fine.

If it's not handled in the GDScript 2.0 refactoring already, we could consider a proposal to handle shadowing in a more consistent way (either allow it all the way - with warnings - like C/C++, forbid it at all levels but that'd be overkill IMO, or go the Python-way with local, nonlocal, etc. but I'd find it confusing).

I've added a mention to this compat breakage in the 3.2.2 release notes' "Known incompatibilities".