godotengine
Invalid reporting of shadowed variable as functions
Godot version
v4.0.stable.official [92bee43ad]
System information
Windows 10, MacOS
Issue description
I am in the process of fixing my plugin to rid it of warnings, but the warnings are partly just nonsensical or wrong.
So why i get a warning for a shadowed variable shadowing an already-declared function? A variable is just a variable and not a function and vise versa.
a simple example:
var _value :int
func _init(value :int):
_value = value
func value() -> int:
return _value
The constructor uses a variable called "value" and initializes the member variable "_value".
Why it should not allowed to use value as name for the argument?
How can this argument name shadow a function, that makes no sense.
Same problem for static functions, a static function is a prioritized way to decouple code from the class to minimize side effects. From documentation: it has no access to the instance member variables
var _value :int
func value() -> int:
return _value
static func foo():
var value = 10
The variable value inside the static function should never affect the stuff from the class itself.
All these problems sound like a script parser problem where there is no way to distinguish between an argument and a function. This kind of warnings confuse the developer and force him to use circumstantial names for arguments.
Steps to reproduce
try with this lines
var _value :int
func _init(value :int):
_value = value
func value() -> int:
return _value
Row 3 (SHADOWED_VARIABLE):The local function parameter "value" is shadowing an already-declared function at line 6.
and
var _value :int
func value() -> int:
return _value
static func foo():
var value = 10
Row 8 (SHADOWED_VARIABLE):The local variable "value" is shadowing an already-declared function at line 3.
Minimal reproduction project
N/A
This is intentional. In 4.0, functions and signals are first class objects, so they now share the same identifier space as variables and constants. Local identifiers can shadow member identifiers, but this raises a warning (and it is good, since bugs with identifier shadowing can be difficult to notice).
It is recommended to use local identifiers names that differ from member and global names. For example, for method parameters you can use the prefix p_ in case of a name conflict.
it may be that functions and variables now share the same identifier space, but why is the syntax break here passed on to the developers? This is absolutely inconvenient that you now have to provide the variables with a pseudo hotfix ala suffix just because the compiler does not get it done to distinguish a variable from a function. ;(
At least this is a regression, i need not to rename my arguments to solve this warning. The code is also reading creasy with this suggestion of using a suffix
func foo(value_ :int) -> void
_value = value_
This kind of code pops-up the question what the hell is this? I never seen such code.
This is not a bug, but a matter of the naming you are using.
# Private member.
var _something: int
func _init(something: int) -> void: # Parameter / local variable.
set_something(something)
# Public getter.
func get_something() -> int:
return _something
# Public setter.
func set_something(value: int) -> void:
_something = value
# Public member (property).
var something: int:
get:
return _something
set(value):
_something = value
# Private member (actual).
var _something: int
func _init(p_something: int) -> void: # Parameter / local variable.
something = p_something
I don't like to use this code noise of get_ set_, a function signature is clear enough to evaluate if it is a setter or getter ;) There is also no official Godot code style to do this.
I have never seen such code where you are forced to find a cumbersome name for an argument. Especially the argument should only be valid in the content of the function used and never have any effect on class members or functions. This is actually fundamental design of a programming language.
All these problems sound like a fundamental code parser problem to me, sorry. All suggestion actually looks like a workaround.
I am currently absolutely not satisfied with the way fundamental problems are addressed here. Every developer who sees such code just shakes his head. These are just Godot homemade problems that I have never seen in other languages.
Currently, unfortunately, I have no choice but to rename all these places in my plugin cumbersome to solve these warnings.
All these problems sound like a fundamental code parser problem to me, sorry.
This is not a parser problem, GDScript allows you to shadow identifiers. These warnings were added intentionally to avoid name shadowing bugs.
In 3.x the behavior is the same, the only difference in 4.0 is that properties, methods and signals now share the same identifier space. The reverse situation (separate spaces) has its downsides (for example, in PHP you can't have a class member of type callable) and IMO it's less clear when the same name can mean different things.
I don't like to use this code noise of get_ set_, a function signature is clear enough to evaluate if it is a setter or getter ;) There is also no official Godot code style to do this.
This implicit convention is used in many languages and libraries, including the Godot API. There are a few exceptions like Array.size() and String.length(), but the set_ and get_ prefixes are used in the vast majority of cases.
