JsEnum documentation vs j2cl implementation

[niloc132]

The Javadoc for JsEnum specifically calls out that this is only implemented in J2CL, but omits some specific decisions that the J2CL implementation made. I've spent a few hours this weekend trying to build the same feature in GWT2, and my experience with JsEnum in J2CL is quite limited, so I was surprised by some of these decisions.

I went a little deeper into J2CL, reading docs, tests, and implementation specifics, and have found myself with many questions that possibly should go into the JsEnum javaodc, and possibly somewhere more like the J2CL docs where those decisions were deliberately unspec'd by jsinterop-annotations. I'm happy to try to make those changes myself, but the line of where some of them ought to end up isn't always clear.

I posted this originally as Q&A, but it could easily fall under another category, or possible even as a set of bugs? Please direct me if there is a better way to handle this.

Comparable

The Javadoc for JsInteropRestrictionsChecker includes this block:

j2cl/transpiler/java/com/google/j2cl/transpiler/passes/JsInteropRestrictionsChecker.java

Lines 294 to 313 in 0e13b9a

	/**
	* Checks that the JsEnum complies with all restrictions.
	*
	* <p>There are three flavors of JsEnum:
	*
	* <ul>
	* <li>(1) JsEnum that does not customize the value type.
	* <li>(2) JsEnum that customizes the value type.
	* <li>(3) native JsEnum.
	* </ul>
	*
	* <pre>
	* | has value field | has constructor | implements Comparable | can call ordinal() |
	* (1) | | | x | x |
	* (2) | x | x | | |
	* (3) | x | | | |
	*
	* </pre>
	*/
	private void checkJsEnum(Type type) {

The truth table suggests that ordinal() and Comparable should both only function for JsEnums with hasCustomValue=false. However, the tests in https://github.com/google/j2cl/blob/0e13b9a49b68b8f048734b2330cbdfcd89681972/transpiler/javatests/com/google/j2cl/integration/java/jsenum/Main.java seem to disagree with that - StringNativeEnum as the name implies is both a native JsEnum and has a String custom value, but tests confirm that instances correctly implement Comparable:

j2cl/transpiler/javatests/com/google/j2cl/integration/java/jsenum/Main.java

Lines 213 to 219 in 0e13b9a

	Serializable se = (Serializable) o;
	StringNativeEnum sne = (StringNativeEnum) o;
	NativeEnum ne = (NativeEnum) o;
	Comparable ce = (Comparable) o;
	Comparable seAndC = (StringNativeEnum & Comparable<StringNativeEnum>) o;
	assertUnderlyingTypeEquals(String.class, seAndC);

.

Both implementations of JsInteropUtils agree with the tests, rather than the documentation:

j2cl/transpiler/java/com/google/j2cl/transpiler/frontend/jdt/JsInteropUtils.java

Lines 100 to 101 in 0e13b9a

	.setSupportsComparable(!hasCustomValue || isJsNativeType(typeBinding))
	.setSupportsOrdinal(!hasCustomValue && !isJsNativeType(typeBinding))

j2cl/transpiler/java/com/google/j2cl/transpiler/frontend/javac/JsInteropUtils.java

Lines 100 to 101 in 0e13b9a

	.setSupportsComparable(!hasCustomValue || isJsNativeType(type))
	.setSupportsOrdinal(!hasCustomValue && !isJsNativeType(type))

I added the "supports ordinal" line in both to contrast with the truth table showing that they follow the same logic.

Boxing

It appears that in j2cl, an enum can be "boxed", where the underlying number/string/boolean is wrapped in an object. Among other things, this makes it possible to tell that an enum cast to Object is not an instanceof Double/String/Boolean, but is only an instance of its own class. This does have the tradeoff that most generic jsinterop-annotated methods will break when given a JsEnum - consider for example a JsArray<MyEnum>, where you call JsArray.of(MyEnum.FOO, MyEnum.BAR), or use arr.push(MyEnum.FOO) to populate the collection before returning it to JS.

(Additionally, arrays of enums aren't supported, so this example can't have a "JsArray vs MyEnum[]" compare/contrast.)

Is there a way to unbox consistently to avoid this issue? Generated .impl.java.js for MyEnum getValue() and JsMap<String, MyEnum> getMap() methods:

  /**
   * @return {?MyEnum}
   */
  static getValue() {
    //...
  }
  /**
   * @return {Map<?string,$Enums_BoxedLightEnum<?MyEnum>>}
   */
  static getMap() {
    //...
  }

If not, shouldn't BoxedLightEnum.value be marked as a JsProperty, so that JS is hypothetically able to unbox?

Note that boxing doesn't occur with native enums, which almost certainly should be documented as well. What benefits do non-native enums get from being able to be boxed to be worth these tradeoffs?

Extends Enum

Somewhat surprisingly, the previous point does not make a JsEnum actually extend java.lang.Enum, unlike other boxed java primitives. It seems like it would be clearer to document this directly in JsEnum as one of the main points, and discuss details about name()/ordinal()/values() as a subpoint.
Related, valueOf() isn't documented as not supported in JsEnum, but doesn't appear to be supported.

Enum instance method support?

I can't find any tests that verify this, nor any logic that implements it, and the JsEnum docs don't say they isn't allowed. What's up with enum instance methods? The enum type could handle this by implementing the abstract method with a switch/case to delegate to (or inline) the actual final/static implementation, right?

[rluble]

RE: Enum instance method support?

Instance methods are allowed treated as if they were @JsOVerlay. That is possible because it is guaranteed that there are no subclasses of the particular enum and because JsEnums are not allowed to implement interfaces. The only potentially polymorphic methods are those inherited from Object or Enum. The ones from Enum are actually final and special cased (ordinal is only allowed on non custom valued enums, name is disallowed in all, etc). I don't recall which approach we took for Object, but those are trampolines to handle JS values, but we could be boxing also, not sure.

Re: Extends Enum
JsEnums don't extend Enum, even in their boxed version. That was a design choice, and the main reason is that the contract for enum does not apply to JsEnum (there is no name(), ordinal() only on some as well as compareTo()). Nevertheless the boxed versions of JsEnums (implemented in the runtime by BoxedLightEnum) could extend Enum and throw for the unsupported methods, but would only have benefits for enums that support Comparable. In general we prefer to fail at compile time.

Enum.valueOf is not supported in J2CL for any Enum. The reason being is that using value of would prevent stripping of unused Enum values.

Re: Boxing
Boxing is mostly done so that the JsEnums can be passed as Objects, put in collections and not be confused with the values. Currently arrays of JsEnum are not supported.