go: enums not working - Expected enum value, got "<string>"

[eladb]

🐛 Bug Report

Affected Languages

• Golang
• TypeScript or Javascript
• Python
• Java
• .NET (C#, F#, ...)

General Information

• JSII Version: 1.20.0
• Platform: any

What is the problem?

Enums are not working.

Consider the following jsii code packaged as github.com/eladb/go-enum-repro/enumrepro:

export enum MyEnum {
  MY_VALUE = 'my',
  YOUR_VALUE = 'your'
}

export class Hello {
  public sayHello(x: MyEnum) {
    return `hello, ${x}!`;
  }
}

And the consuming go code:

package main

import (
	"github.com/eladb/go-enum-repro/enumrepro"
)

func main() {
	var hello = enumrepro.NewHello()
	hello.SayHello(enumrepro.MyEnumYourValue)
}

When running this program, we get the following error:

panic: "Expected enum value, got \"YOUR_VALUE\""

goroutine 1 [running]:
github.com/aws/jsii-runtime-go.Invoke(0x1124220, 0x1376d60, 0x11461c8, 0x8, 0xc00015bdf0, 0x1, 0x1, 0x1376d01, 0x111c400, 0xc000104720, ...)
        /Users/benisrae/go/pkg/mod/github.com/aws/[email protected]/runtime.go:87 +0x305
github.com/eladb/go-enum-repro/enumrepro.(*Hello).SayHello(0x1376d60, 0x11466ca, 0xa, 0x111e4a0, 0xc000102058)
        /Users/benisrae/playground/play-0204133257/enum-repro/dist/go/enumrepro/enumrepro.go:34 +0x165
main.main()
        /Users/benisrae/playground/play-0204133257/enum-repro/dist/go/test/main.go:9 +0x4d

• blocked by Go runtime type registration #2561

[eladb]

The jsii runtime expects that enum values will be marshaled in the following form { "$jsii.enum": "fqn/member" }. For example, { "$jsii.enum": "jsii-calc.StringEnum/A" }.

The go code generator renders enums like this:

type StringEnum string

const (
	StringEnumA StringEnum = "A"
	StringEnumB StringEnum = "B"
	StringEnumC StringEnum = "C"
)

The current bindings implementation simply passes the values of enums through to the javascript code.

This approach has the following downsides:

1. There is no strong-typing - any string can be passed for any enum. The enum type (in the above example StringEnum is merely an alias for string).
2. The value passed to the jsii runtime is just a string, not the { "$jsii.enum": ... } token. We need to somehow enforce the full FQN of the type.

It seems like the idiom of using an alias and constants for enums is common in Go, so #1 is likely less of an issue for Go developers, and maybe we don't have to solve for it.

In order to solve #2, we need two things:

1. The full type information of the enum member (FQN/Member).
2. Convert it from a string to a JSON object in the form { "$jsii.enum": "... }.

We can address the type information problem by simply rendering the fully qualified member name in the consts:

const (
	StringEnumA StringEnum = "jsii-calc.StringEnum/A"
	StringEnumB StringEnum = "jsii-calc.StringEnum/B"
	StringEnumC StringEnum = "jsii-calc.StringEnum/C"
)

But then, in the generated bindings of all code paths that can accept/return enums (methods, properties and structs), we will need to use the jsii type information to identify that an enum is passed in, and serialize it appropriately.

Alternative

Would it make sense to model enums like this:

type StringEnum struct {
	StringEnum string `json:"$jsii.enum"`
}

func StringEnumA() StringEnum {
	return StringEnum{StringEnum: "jsii-calc.StringEnum/A"}
}

func StringEnumB() StringEnum {
	return StringEnum{StringEnum: "jsii-calc.StringEnum/B"}
}

func StringEnumC() StringEnum {
	return StringEnum{StringEnum: "jsii-calc.StringEnum/C"}
}

This will address both #1 and #2, but seems less idiomatic for Go.

[MrArnoldPalmer]

So we can keep the existing approach and fix number downside #2 by code generating custom json serialization implementations alongside the enum types like so:

type JsiiEnum struct {
	StringEnum string `json:"$jsii.enum"`
}

type MyEnum string

const (
	Val1 MyEnum = "Val1"
	Val2 MyEnum = "Val2"
)

var myEnumFqn = "jsii-calc.MyEnum"
var myEnumToId = map[string]MyEnum{
	"jsii-calc.MyEnum/Val1": Val1,
	"jsii-calc.MyEnum/Val2": Val2,
}

// Custom marshal serializes MyEnum vals as "{jsii.enum: fqn}" format
func (s MyEnum) MarshalJSON() ([]byte, error) {
        fqn := fmt.Sprintf("%s/%s", myEnumFqn, string(s))
	return json.Marshal(JsiiEnum{StringEnum: fqn})
}

// Custom unmarshal deserializes "{jsii.enum: fqn}" format
func (s *MyEnum) UnmarshalJSON(b []byte) error {
	var j JsiiEnum
	err := json.Unmarshal(b, &j)
	if err != nil {
		return err
	}

        val, ok := myEnumToId[j.StringEnum]
        if !ok {
          return errors.New("Invalid enum value")
        }

        *s = val
	return nil
}

Playground link

I think this would be my preferred solution as it feels the most idiomatic though does require some significant increase in generated code. Most of the logic for the bodies of Unmarshal/Marshal implementations could be abstracted.

If we decide that this is too complex or has other problems I'm not considering, I believe option #2 (the free floating functions for each option) would be my preference here. I don't see a lot of functional difference between the two besides the actual "values" of each option within the Enums feels more obscured by the functions and therefore more hidden from the user. That feels better to me when these values are FQNs are otherwise something the user may not know about or understand. This probably is largely inconsequential though.

[eladb]

@MrArnoldPalmer thanks for the inputs. I'll try to take the approach you proposed.

[eladb]

@MrArnoldPalmer I've implemented your suggestion in #2535. I did have to modify the function castAndSetToPtr so that it will attempt to unmarshal with the specific return pointer. Curious what you think about this.

[eladb]

@RomainMuller brought up that the custom marshaller approach won't work if we set/get the enum through an any type. In that case, the Go marshaller won't invoke the marshaller because it doesn't know the source/target type.

I recon this means that we will need to take the approach described in "Alternative" above in which enum members are rendered as functions that return objects that intrinsically include the $jsii.enum field. This will solve the marshaling, but we will still need to implement customized general-purpose unmarshaling to identify those enum objects when we receive them from the runtime & recover the Go type from it somehow.

Okay, continuing to explore...

[RomainMuller]

I think it'll end up something along the lines of what is done for object references:

jsii/packages/@jsii/go-runtime/jsii-runtime-go/runtime.go

Lines 204 to 224 in 0368d42

	func castValToRef(data interface{}) (objref, bool) {
	ref := objref{}
	ok := false
	dataVal := reflect.ValueOf(data)
	if dataVal.Kind() == reflect.Map {
	for _, k := range dataVal.MapKeys() {
	// Finding values type requires extracting from reflect.Value
	// otherwise .Kind() returns `interface{}`
	v := reflect.ValueOf(dataVal.MapIndex(k).Interface())
	if k.Kind() == reflect.String && k.String() == "$jsii.byref" && v.Kind() == reflect.String {
	ref.InstanceID = v.String()
	ok = true
	}
	}
	}
	return ref, ok
	}

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.