GDScript: Fix wrong marking of some lines related to Variant as unsafe

[vonagam]

I think most of the things were implemented in other PRs. Here is what remains in this one:

func variant() -> Variant: return null

func as_return() -> Variant:
  return variant() # safe now

func as_argument(arg: Variant): pass
  
func _ready():
  var mystery := variant()
  
  as_argument(mystery) # safe now
  
  if mystery == null: pass # safe now
  if mystery != null: pass # safe now
  if mystery is Node: pass # safe now
  

Those will be safe for variant - returning or passing as an argument with explicitly specified Variant type, comparing to null, type checking.

[vonagam]

Marked is, == and != operators as safe for usage with variants and typed them as booleans. Currently:

if var_e == null: pass # not safe, bad
if var_e != null: pass # not safe, bad
if var_e is Node: pass # not safe, bad

[vonagam]

Hm... so comparing two Variants is unsafe, have not expected that.
Limited blank safety only to comparing to null.

Since arrays and now dictionaries are compared by contents, usual comparison may include a conversion and also does not work between two arbitrary types, starting to think that something like === from js might make sense here - operator that checks that types are equal and compares inputs only by reference if possible, so no conversions, no contents checks and safe for all type combinations. Or at least a global helper method is_equal_strict for that.

[vonagam]

Updated PR to allow inference of hard Variants for variables:

var var_i := variant() # error -> safe

Removed some inconsistencies between resolving member variables and local ones.

Added a test.

[anvilfolk]

Just went through the PR. Didn't really do a deep dive, but all the code changes make sense to me, as well as the conceptual changes to what is safe and isn't safe :)

Just had a question above for something that I noticed afterward was already this way, so I guess no harm in leaving it this way either :)

[anvilfolk]

Needs rebase!

[adamscott]

Looks good to me.

[vnen]

Updated PR to allow inference of hard Variants for variables:

var var_i := variant() # error -> safe

I'm not sure if this is a good idea. Generally one uses type inference expecting to become of that specific type. Since Variant is not really an specific type, it should not be valid for inference, even if the original value is hard typed to be Variant. I fear it's quite easy to do this mistakenly by accident.

In this case it would be better to either not use static type at all or explicitly type it as Variant.

[vonagam]

var var_i := variant() was part of unification, currently in master.

I consider the line itself as type safe. Unsafe lines would be those where var_i is used as anything other than variant. So if var_i is used just as a dictionary key, that's pretty type safe. (After type parameters for dictionaries are introduced one will be able to communicate if they really expect variants as key/value or not and those lines might become type unsafe.)

not use static type

In my view there is a difference between hard variant type and weak variant (untyped) type. With hard variant it should be safe to both read and write variant while with weak one you are told that the value is of some unspecified type, you can expect anything but it does not mean that you can safely set it to anything, so:

var a := variant()
a = another_variant() # type safe

var b = variant()
b = another_variant() # type unsafe

And it is also strange to punish properly typing things as there is no other way to type "anything" other than with Variant.

Also important to highlight that I'm talking about type safety, not simply safety, even type safe code can be wrong and unsafe. This is strictly about types, so maybe a = another_variant() is not actually safe, but from perspective of types it should be.

[vonagam]

Updated description to show what is left in this PR.

[akien-mga]

Thanks!