Incorrect type checking

[isokissa]

Godot version

3.5

System information

Windows 10

Issue description

According to my understanding, the point of specifying variable types is to allow compiler-time checks even though Godot does not have static typing at run-time. However, when dealing with class hierarchies, Godot does not handle typed variables as expected. If we declare a variable to be an instance of class C, Godot compiler will allow the access to properties and functions that are not defined in given class C, but in its subclasses, which is incorrect.

It has to be fixed so that this code does not compile:

class_name Parent 
func parentFunction(): 
    print("parent")
...
class_name Child
extends Parent
func childFunction():
    print("child")
...

var a: Parent = Child.new()
a.childFunction()            # this should not compile, because class Parent does not have childFunction

Steps to reproduce

1. define a class Parent
2. define a class Child, extending Parent
3. define a function m in class Child, such that it does not exist in Parent
4. define variable a of type Parent
5. initialize variable a with an instance of class Child
6. try to call function m, which exists in Child but not in Parent
7. this code should not compile, because it tries to call a function which is not defined in the type of variable a

Minimal reproduction project

project.zip

[dalexeev]

You can enable warnings:

Additionally, you can treat all warnings as errors:

In 4.0-dev you can set this up on a per-type basis:

[isokissa]

Thank you for the clarification! I still a bit wonder what would be the use case for a developer to declare the type for a variable and then not want the compiler to enforce it. It results in confusing code.

[TheDuriel]

GDScript is not compiled. The compiler thus can not enforce anything.

This does seem like a case though where in 3.5 you would have gotten an error in the IDE telling you about this. Since tracing back the fact the function does not exist should work just fine with static typing. Not just a warning.

[isokissa]

By "compiler" I meant the thing which obviously parses the code and checks the syntax and semantics. It easly can be just IDE, I am not familiar with how gdscript gets executed. Anyway the point is: the language allows programmer to specify variable's type and then does not enforce it, which can be even more confusing than not specifying type at all.

[TheDuriel]

Either way, this is completely fine behavior. Though I personally think it should raise an error instead of a warning.

Your variable obviously contains a Child, not a Parent, and so accessing Child functions isn't causing an error despite the typing being for Parent.

If the IDE did not know there was obviously a Child, then it would error as expected with a method not found. But... the method IS found.

[isokissa]

No it's not fine. It must be treated as semantically incorrect code. My variable obviously contains a Child only in that minimal example. In reality the assignment can be anywhere and very far from sight. Consider:

func weirdFunction(a: Parent):
    a.childFunction()   # !!!!!!! NO, this is not fine, class Parent does not have childFunction

How on earth it can be fine that weirdFunction calls a method which cannot be found in Parent class? The function does not even need to know that class Child exists. If I expect that parameter a is of type Child it should be declared as Child. That's the whole point of declaring types.

[TheDuriel]

func weirdFunction(a: Parent):
a.childFunction() # !!!!!!! NO, this is not fine, class Parent does not have childFunction

I am pretty sure this example does raise an error though. And if it does not. Then that is a definitive bug.

It is:

var a: Parent = Child.new()
a.chilFunction()

Which does not raise an error. Because... well, it wouldn't error now would it?

[isokissa]

var a: Parent = Child.new()
a.chilFunction()

Which does not raise an error. Because... well, it wouldn't error now would it?

It would not be an error if I did not restrict the type of a to be Parent.

[TheDuriel]

But that's my point.

In any case. It may be worth investigating this further and establish a list of examples in which an error versus a warning would be expected.

var a: Parent = Child.new()
a.chilFunction()

This is valid, functional, executable, code. Despite the type miss match. If you run this, it will function just fine. (Type hints don't really exist at runtime.)

It is also a constant expression. There is no ambiguity about the fact that A holds a child by the time you try to call childFunction. So the IDE seemingly accepts this as correct.

I agree that it probably should not be allowed. But this needs more testing to determine if this is:

1. A bug that needs fixing.
2. An intentional decision to allow valid code but raise a warning due to type miss match.
3. Completely unaccounted for.

[isokissa]

This is valid, functional, executable, code. Despite the type miss match. If you run this, it will function just fine. (Type hints don't really exist at runtime.)

This is not valid functional, executable code in any programming language with static typing. I know that types don't exist at run time in GDScript, but because at development time GDScript gives the possibility to specify (restrict) type of a variable, it should behave as expected. If I give a type to a variable, it means that I want to assume that variable will have exactly that type and I don't want to be allowed to call methods and access properties that don't belong to that type. And I want that Liskov Substitution Principle holds, so that the code works regardles of what object is used as value for a variable, as long as it is inherited from that variable's type.

[TheDuriel]

specify (restrict) type of a variable

That is not what it does.

I don't want to be allowed to call methods and access properties that don't belong to that type

GDScript is not statically typed. And it is interpreted. The type hints exist to allow the Editor to provide you with useful information. There is no way to prevent you from writing code that ignores the hints. You can save a script with a type error and run it just fine so long as you don't run into an actual execution error.

If you think of GDScript as being in the same class of language as JavaScript, your expectations may be more in line with how things work. Or TypeScript since you are using the optional type hints.

Point is. It will execute without error. So the editor, is not giving you an error as it stands right now. I am plainly explaining why things work how they do, and what frame of reference allows for this to be acceptable.

It probably should error. I will reiterate.

And I will reiterate. That we (you) should investigate under which circumstances it is not providing an error, even though we believe that it should.

[dalexeev]

Static typing is optional in GDScript. In particular, the following code is allowed by default, although it is not type-safe:

var my_var: MyClass = dict.key

If you want strict checks, I wrote above how to enable them. See also this comment.

[isokissa]

Fair enough, thanks for replying.

[TheDuriel]

Upon further investigation.

extends Node2D

func some_func_a:
	var a: FirstClass = SecondClass.new()
	a.second_class_func() # No code completion shown. No error. No warning.

func some_func_b(a: FirstClass):
	a.second_class_func() # No code completion shown. No error. No warning.

class FirstClass:
	func first_class_func():
		pass

class SecondClass extends FirstClass:
	func second_class_func():
		pass

Neither of these create an Error in the editor. Both will create a warning if the warning is enabled. Neither of them show up in code completion.

Everything is working correctly here. The warning is simply disabled by default.

@isokissa There is no bug and no false behavior present here. BUT, you can make a proposal about enabling these warnings by default if you wish to start a discussion on that. I do think it makes sense. And we might want to clarify the description of the warnings to not include "compile time" when that's not really a thing in the typically expected way.

[dalexeev]

BUT, you can make a proposal about enabling these warnings by default if you wish to start a discussion on that. I do think it makes sense.

Personally, I'm against it. Static typing is optional (and dynamic is default), type hints are disabled by default in the editor, static types are not used in official examples. This would be too unfriendly to beginners and those who prefer dynamic typing.

There would be a huge number (much more than even the infamous RETURN_VALUE_DISCARDED) of warnings that are useless to most, thus devaluing other, more critical warnings.

I find type safe lines to be a good tool and a compromise between code type safety and convenience. Type safe lines do not distract the developer, do not force them to constantly please the type system (which sometimes, on the contrary, leads to problems if you mask an error, as is the case with the as keyword). But type safe lines gently draw the developer's attention to potential problems, think about the health of the code if there are too many gray lines. If I see a gray line, then I check more carefully if everything is correct. But this does not mean that there is an error in this line.

[TheDuriel]

If you were to hold a poll about safe lines. I am willing to bet only experienced users could tell you: They exist, what they do, and what they look like. Meanwhile typing is something everyone recommends and which actively prevents errors and makes the code easier to use.

In any case. This is veering off into discussion. There is no bug present in this issue, so it can probably be closed?

[isokissa]

Personally, I'm against it. Static typing is optional, type hints are disabled by default in the editor, static types are not used in official examples. This would be too unfriendly to beginners and those who prefer dynamic typing.

Yes, static typing is optional, and those who do not like it should not use it. And for those who want it, it should work as expected. When I specify the type of a variable, this is because I care that the variable contains exactly that type and no other.

EDIT: and it is wrong to assume that kids/beginners/amateurs prefer dynamically typed languages. If you are interested in programming you are certainly able to understand that there are different types of data, like text, numbers... It does not help anyone if the language just allows assignment of any type to single variable, it's just confusing and error-prone.

EDIT 2: IMHO this is a bug. If you claim that static typing is optional, it means I can choose that option and enjoy the static typing.

[dalexeev]

I can choose that option and enjoy the static typing.

You can do it now, without changing the defaults. And since you have more programming experience than beginners, it's easier for you to switch the settings to what you want than for them.

I don't consider myself a beginner, I use static typing wherever possible, but I deliberately don't activate these warnings. Because I think that personally these warnings will bring more inconvenience to me and force me to appease the type system without any benefit.

IMHO this is a bug.

It's definitely not a bug, it's a feature of the language's type system. Different languages have different ways how types work.

[Calinou]

Closing per the above comments. Please continue the discussion in a proposal (but see godotengine/godot-proposals#173 first).

In general, I wouldn't enable the unsafe property/method access warnings by default as they usually end up being too intrusive. People already have trouble with the "discarded return value" warning and regularly end up disabling it, even though the warning can spot real bugs. Warning fatigue is something to be careful about 🙂

[isokissa]

Obviosly the wrong options are disabled/enabled by default. Nobody should care if you don't use function's return value. But it's pretty big thing if your "static typing" allows calling the method which does not exist.