SHADOWED_VARIABLE warning is unclear for derived classes

[Pennycook]

trafficstars

Godot version

v4.0.stable.official [92bee43ad]

System information

Steam OS Holo, Steam Deck

Issue description

The SHADOWED_VARIABLE warning is unclear when the shadowed variable belongs to a user-defined base class.

Base.gd

extends Node

var inherited

func foo(name : String):
	pass

Derived.gd

extends "res://Base.gd"

var shadowed

func bar(inherited):
	pass

func baz(shadowed):
	pass

The use of name in Base triggers a very clear SHADOWED_VARIABLE_BASE_CLASS warning:

W 0:00:00:0492   The local function parameter "name" is shadowing an already-declared property at the base class "Node".
  <GDScript Error>SHADOWED_VARIABLE_BASE_CLASS
  <GDScript Source>Base.gd:5

However, the use of inherited in Derived triggers a SHADOWED_VARIABLE warning that does not reference the base class:

W 0:00:00:0493   The local function parameter "inherited" is shadowing an already-declared variable at line 3.
  <GDScript Error>SHADOWED_VARIABLE
  <GDScript Source>Derived.gd:5

To demonstrate how confusing this could be, I've deliberately set things up here such that shadowed triggers a warning pointing to the same line number, despite shadowing a different variable defined in a different class:

W 0:00:00:0493   The local function parameter "shadowed" is shadowing an already-declared variable at line 3.
  <GDScript Error>SHADOWED_VARIABLE
  <GDScript Source>Derived.gd:8

Steps to reproduce

Run the minimal reproduction project.

Minimal reproduction project

ShadowedVariables.zip

[RoyBeer]

stumbled over this, thanks to this issue I am no longer questioning my sanity

[MikeSchulze]

looks like the same issue also for function parameters

extends Node


var _report :String


func foo(report: String) -> void:
	_report = report


func report() -> String:
	return _report

(SHADOWED_VARIABLE):The local function parameter "report" is shadowing an already-declared function at line 11. The function argument report on foo() is an argument and do not shadow the function report()

[kleonc]

extends Node


var _report :String


func foo(report: String) -> void:
	_report = report


func report() -> String:
	return _report

(SHADOWED_VARIABLE):The local function parameter "report" is shadowing an already-declared function at line 11. The function argument report on foo() is an argument and do not shadow the function report()

It does shadow it, within foo:

• self.report refers to the report: Callable member function.
• report refers to the report: String local function parameter. This is shadowing.

However, it's kind of partial shadowing because it seems like report() is treated/recognized specifically as a method call and thus when resolving what report means in there only the methods are considered, the local variables/arguments are not taken into account (just a suspiction, haven't actually looked into the code). Hence no name collision found / no shadowing detected.

• self.report() refers to the report: Callable member function.
• report() refers to the report: Callable member function. Kinda inconsistent/confusing!

Being strict/consistent shouldn't report() fail in such case? :thinking: Not sure if that's desired/feasible though. Up to the GDScript team. :upside_down_face:

looks like the same issue also for function parameters

I'd say the above is a separate issue.

[MikeSchulze]

It does shadow it, within foo:

It should not be recognized as a shadow, it is a local typed parameter of type string. On the heap within the function foo there is a variable named report of type string, the parser should be able to distinguish between a local variable or a function reference (callable).

[MikeSchulze]

Can you explain why you just downvote without any comments?

As I said, the function argument report cannot be a shadow of the function report()! It is just a string type variable used as a function parameter, it cannot be a callable.

This is simply a problem of the parser implementation, and you are not ready to fix it. The parser should be able to distinguish between a local variable and a script function.

[dalexeev]

In 4.x, variables and functions share a common scope. When you access a function without parentheses, you get it as a Callable. Accordingly, the local variable (parameter) report in the foo() function shadows the report() method, you can no longer get it as a Callable using the report identifier. Yes, you can still use self.report, but it doesn't matter, we're talking about report.

[MikeSchulze]

Thanks for the explanation 👍

In 4.x, variables and functions share a common scope.

You say it shares, so it will hold both a variable and a function with the same name? I guess I understand the main issue here, a function can be used as a Callable by using the plain name. e.g. <signal>.connect(<Callable>) as node.connect(my_function)

From my perspective, this is a self-made issue by allow the plain function name as a Callable, sure this has potential to conflict with variable names. So my question, what was the design decision to allow plain names as Callable reference? Why it was not designed to use like a link symbol to reference to a Callable. e.g. node.connect(@my_function)

I'm just thinking loud ;)

[dalexeev]

You say it shares, so it will hold both a variable and a function with the same name?

No. Let me explain in more detail.

3.x

In 3.x, variables, functions and signals live in separate scopes. We have warnings PROPERTY_USED_AS_FUNCTION, CONSTANT_USED_AS_FUNCTION, and FUNCTION_USED_AS_PROPERTY but overall it works. Functions are not first class values, you cannot get them as Callable. You can't call variables either. So there's no conflict here, although it's a little confusing.

Variables
=========
a: 0
b: 1
c: 2

Functions
=========
a: 0
f: 1
g: 2

4.x

In 4.x, functions and signals became first class values. You can get a function as a Callable and call it later. Now there are no separate tables, there is a common member table:

Members
=======
a: 0 - Function
b: 1 - Variable
c: 2 - Variable

So my question, what was the design decision to allow plain names as Callable reference?

This is a common approach. JavaScript and other languages also do not have separate scopes for variables and functions. I find it more obvious to have just one scope for all identifier types, since there are already several locality scopes (global, class and block). Why have several identifiers of the same name that mean different things?

Why it was not designed to use like a link symbol to reference to a Callable. e.g. node.connect(@my_function)

Some languages use sigils for identifiers of different types. For example in PHP you can have a function f and a variable $f. But the benefits of this option are not convincing and it is less convenient to type.

[MikeSchulze]

All right, thanks for diving deeper in this topic to give me a better understanding.