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

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()

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? 🤔 Not sure if that's desired/feasible though. Up to the GDScript team. 🙃

looks like the same issue also for function parameters

I'd say the above is a separate issue.

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).

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.

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.

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 ;)

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.

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

I have been working on a PR for this and I wonder which of the following solutions, if any, would be preferred. I have attached a screenshot for the result of each solution.

1. Use the SHADOWED_VARIABLE warning and add extra information to the last symbol if needed (e.g. the shadowed variable is in a parent class).
   
2. Use the SHADOWED_VARIABLE_BASE_CLASS warning.
   
3. Create a new warning SHADOWED_VARIABLE_PARENT_CLASS which contains both the line number and parent class.
   

I don't think we need a new warning type. The comments, the messags and the documentation all say the same thing:

However, SHADOWED_VARIABLE_BASE_CLASS is never produced for GDScript classes, SHADOWED_VARIABLE is always produced:

So we should:

1. Fix the bug in the analyzer, shadowing inherited GDScript members should produce SHADOWED_VARIABLE_BASE_CLASS warning. SHADOWED_VARIABLE should be produced only for the current class.
2. Add the optional at line %s part to the SHADOWED_VARIABLE_BASE_CLASS warning message (missing for native classes).
3. (Probably) Add the in this class part to the SHADOWED_VARIABLE warning message for clarity.
4. Update the documentation, since in 4.x a variable can shadow not only a variable, but a class member of any kind.