GDScript enum fixes & refactor

[anvilfolk]

This PR fixes a bunch of issues with enums. Also, 80% of it is unit tests ;)

Needs #71028 before it passes tests :)

At a glance

• Fix being able to assign to enum values.

• Fix unsafe enum assignments not marking nodes unsafe.

• Fix native enum exports generating warning. Fixes Enum initialized export generates incorrect warning #69118.

• Fix enum casts with values not existing in enum target not yielding error.

• Fix no errors being generated when using non-existent enum values. Fixes GDScript 2.0: When using named enums, accessing invalid members won't be caught while editing #60545.

• Fix accessing non-existent properties of native types (including enums) not yielding error.

• Fix no errors being generated when using non-existent methods on enums. Fixes Calling enum's nonexisting method causes a crash #61466.

• Fix being able to use non-const dictionary methods on enum types, e.g. MyEnum.clear().

• Fix code autocompletion for enums. Only enum values and const dictionary methods show up now. Fixes Named enums does not provide hints #61427.

• Fix (native & gdscript) enum values not being accessible in certain ways, e.g. NativeClass.EnumType.ENUM_VAL, etc.

• Fix enum and native enum types not being usable as variable, function parameter, function return types. Fixes GDScript 2.0: Function return type enum error (Trying to return value of type "int" from a function which the return type is "Dictionary") #57966.

• Fix Stack overflow when using @export variable with enum type and set, get #70915.

• Added comprehensive unit tests.

• Improved error messages with more information.

• Code refactor & other assorted improvements and fixes.

And yes, maybe I did name a lot of these "fixes" instead of "implements" because we are in feature freeze ;)

Fixes & features

It does a general refactor of how enums are handled internally and in GDScript, cleaning up the code & making it slightly more consistent across GDScript enums and native enums. It also aims to unify warnings&errors between in-editor, runtime, and export debug/release templates (native enum behavior on release templates now working due to #64253).

For native enums, e.g. with TileSet, you can now use TileSet.TILE_SHAPE_SQUARE or TileSet.TileShape.TILE_SHAPE_SQUARE, and a warning will be issued if the native type does not have an accessed property.

Typing with enums now works properly for variables, function parameters and function returns, and comprehensive unit tests have been added to check this. Example which is now possible, and will throw errors if the wrong enum is used.

class_name OuterClass

enum MyEnum {V0, V1}

class InnerClass:
  enum MyEnum {V0, V1}

func outer_inner_class_class(e : InnerClass.MyEnum) -> Outer.InnerClass.MyEnum:
  print(e)
  return e

func _ready()
  var boom :  = outer_inner_class_class(V1)
  var good :  = outer_inner_class_class(OuterClass.InnerClass.V1)

[rune-scape]

i know it's still a draft, but this looks great! nice job refactoring :)

[rune-scape]

what happens with this script?:

enum E1 {V0, V1}
enum E2 {V1, V0}

func _ready():
    print(V0, V1)

it doesn't look like the values are added as members, or checked for name conflicts,
i thought the way to use enum values at a class level was to do:

enum {
    V0,
    V1
}

[anvilfolk]

Oooooh, excellent comments, thanks @rune-scape! I am done for the day, but will think & tackle these when I can! I've implemented everything I wanted for this PR, so your comments are super timely :)

[anvilfolk]

what happens with this script?:

That's a really cool one!

I tried keeping things consistent & backwards compatible. Firstly, for native classes, NativeClass.EnumName.ENUM_VALUE wasn't possible, only NativeClass.ENUM_VALUE. On the other hand, GDScript was the opposite, with GDScriptClass.NAMED_ENUM_VALUE not working, only NamedEnum.NAMED_ENUM_VALUE.

So the two behaviors existed but were used inconsistently between native & gdscript types. I figured the only way to move forward without breaking existing code was to allow both types of access to both types of enums.

Good thought on maybe using Member to represent named enum values. I thought about it briefly, but I believe we would lose type information if we created ENUM_VALUE members from ENUM values, and would've had to check which enum it belonged to. I'm tempted to just add checking named enums wherever appropriate once I factor that out into a function.

But now you're right, it creates issues with shadowing that need some work. GDScript seems pretty lenient on allowing shadowing with warnings rather than errors, so I'm tempted to implement this:

1. Unnamed enum values shadow named enum values, since named enums can be accessed by adding enum name qualifiers.
2. Local enum values, unnamed & named, will shadow base_class & outer_class enum values of any kind, since "upper" enums can be accessed by adding class qualifiers.

I'm tempted to either not add any warnings at all, or to add warnings to 2 only, since it might be less obvious that you are doing it.

[anvilfolk]

I actually realized that I should be able to use reduce_identifier_from_base to find named enum values, rather than using my auxiliary methods.

So that + adding shadow warnings are on my list, but hopefully that's it?

[anvilfolk]

Alright, I think this PR is done :)

There's some issues with shadowing outer class members that needs work in check_class_member_name_conflict but I think that's a different PR.

[Maran23]

This may fixes #70915 as well. Can you verify that? :)

[anvilfolk]

It actually does - I confirmed the bug is still there on Beta 10, but doesn't happen in this PR :) Thanks for linking!

[Maran23]

It actually does - I confirmed the bug is still there on Beta 10, but doesn't happen in this PR :) Thanks for linking!

Glad to her!
Not sure if it works, but is it possible to add the buggy example in the ticket as test here?

[anvilfolk]

Not sure if it works, but is it possible to add the buggy example in the ticket as test here?

Not with the current test framework as far as I can tell. Currently it just attempts to parse scripts. The most you can do is preload, but I don't think there's a scene tree we can use. Maybe I'm wrong though :)

[vnen]

Looks okay to me.

[akien-mga]

Thanks! Outstanding work 🎉