Enum.valueOf() support?

[niloc132]

Enum.valueOf() (and the corresponding Class.getEnumConstants()) isn't supported in j2cl, but there are TODOs in the code suggesting that this might be feasible, and perhaps would be accepted as an external contribution?

j2cl/jre/java/java/lang/Enum.java

Line 56 in d555a33

throw new AssertionError("not supported");

j2cl/jre/java/java/lang/Class.java

Lines 100 to 103 in d555a33

	// TODO(b/30745420): implement
	public T[] getEnumConstants() {
	return null;
	}

Having not yet tried, my hunch was that this might have been left out due to its interaction with @JsEnum

j2cl/transpiler/java/com/google/j2cl/generator/JavaScriptImplGenerator.java

Lines 597 to 601 in d555a33

	} else if (targetTypeDescriptor.isEnum() && !targetTypeDescriptor.isJsEnum()) {
	// TODO(b/117525773): targetTypeDescriptor.isEnum should already be false for JsEnums,
	// making the second part of the condition unnecessary.
	sourceBuilder.appendln(
	utilAlias + ".$setClassMetadataForEnum(" + className + ", " + obfuscatableName + ");");

? Otherwise, it looks like the array literal returned by SomeEnum.values() (via com.google.j2cl.frontend.common.EnumMethodsCreator#createValuesMethod) could possibly be passed made into an additional argument for Util.$setClassMetadataForEnum so it is available on the Class itself?

[rluble]

The main issue is not the ability to implement it but rather how to implement it without losing optimization. As with other reflective features, if not done very carefully the result in a big loss of optimizations. In this case it, a naive implementation will result in all enum values for all enums being kept and not optimized away.

To implement this properly might require some restrictions and compiler transformations to ensure optimizability.

[niloc132]

Thanks Roberto.

Can you outline briefly what your acceptance criteria for a feature like this might look like? Keeping in mind of course that some parts of //transpiler/javatests can't be built in open source (com/google/j2cl/transpiler/readable and com/google/j2cl/transpiler/regression), plus all of //jre/javatests.

At least in my head, it seems plausible that this could be as simple as adding a simple test to com.google.j2cl.transpiler.optimization.EnumOptimizationTest to ensure getUnusedEnumValues still passes for an enum with unreferenced values, even if OtherEnum.valueOf(...) and Enum.valueOf(OtherEnum.class,...) is called, so that we are sure that only this specific enum gets its values array retained through the whole optimization process?

[rluble]

Without restrictions (or a specific global optimizations in jscompiler) a single use of an utility method like

void myMethod(Class c) {
  .....
  x =   Enum.valueOf(c, "...");
 ...
}

would retain all unreferenced values on all enums.

As I before said implementing the functionality is trivial and is not blocking this API. We need to decide on a set of restrictions and/or specific jscompiler optimizations before implementing it. We have already had some discussions around this issue but no concrete design nor plan yet.

[niloc132]

To confirm, you are saying that if Enum.valueOf was implemented as something like return c.getEnumConstants(), which in turn would be assigned to the Class instance in generated JS (perhaps right after Util.$setClassMetadataForEnum is called), closure-compiler would not notice that the field is never read on most class literals and prune those assignments, and that some kind of pass would be required to enable closure to chase them down and confirm which Class instances could be passed in?

This has come up a few times in open source, so it might be something we're going to pursue, even if we just maintain it in our fork after confirming that some basic usage like I described works.

[gkdn]

It is complicated to tackle this in JsCompiler. Our plan is to support this only for limited cases and do some rewriting on J2CL.

If we take an example use case: EnumSet.allOf which needs to call getEnumConstants.

First EnumSet will be updated to use some annotation like @ClassLiteral:
static <E extends Enum<E>> EnumSet<E> allOf(@ClassLiteral Class<E> clazz);

So it means that it could only be called like:
EmumSet.allOf(MyEnum.class);

If J2CL sees such definition, it will allow the call for clazz.getEnumConstants() inside that method.

Then J2CL will re-write such methods to look like following instead:
EnumSet allOf(Class<E> clazz, E[] clazz_enum_constants);

Then it will replace clazz.getEnumConstants() inside the method with clazz_enum_constants parameter.

And finally the call site will be re-written to look like following:
EmumSet.allOf(MyEnum.class, MyEnum.values());

[niloc132]

Got it, thank you. I imagine this will be an internal-only annotation, akin to the other javaemul-internal annotations?

I get the impression you aren't looking for an external contribution for this - is that accurate, or am I misreading?

[gkdn]

The annotation will be public so Guava and user utilities could use it as well.

We will be happy to have an external contribution. If you are planning to do, I recommend writing a short design doc first to get early feedback.

[mP1]

Can i make a suggestion that along with an @MagicAnnotation on the actual enum to opt in, a txt file holding fq class names of enums is also supported. This is of course to support 3rd party libs that are not yours and have not had their enums tagged(updated), but you want/need em in j2cl.

[xamde]

If others stumble over this, here is the best current workaround I found.

Given an enum like

    enum Color { red, green, blue, white }

and this helper

    public static <T extends Enum<T>> T getValue( String name, T ... enumConstants) {
        for(T enumConstant : enumConstants) {
            if(enumConstant.name().equals(name))
                return enumConstant;
        }
        return null;
    }

Usage is

        String fromSomewhere = "red";
        Color color = getValue(fromSomewhere, Color.values());

[niloc132]

Consider instead just using Color.fromString(fromSomewhere) - if you already can reference the enum type directly (instead of its Class<? extends Enum<?>> to pass to Enum.valueOf(type, name) itself), you can just call the static method.

J2CL's tests confirm that this method will be emitted, and will work as expected:

j2cl/transpiler/javatests/com/google/j2cl/readable/java/enums/output_closure/Enum1.impl.java.js.txt

Lines 30 to 37 in 20e8f01

	/** @return {!Enum1} */
	static m_valueOf__java_lang_String(/** string */ name) {
	Enum1.$clinit();
	if ($Equality.$same(Enum1.f_namesToValuesMap__enums_Enum1_, null)) {
	Enum1.f_namesToValuesMap__enums_Enum1_ = /**@type {Map<?string, !Enum1>}*/ ($Enums.createMapFromValues(Enum1.m_values__()));
	}
	return /**@type {Enum1}*/ ($Enums.getValueFromNameAndMap(name, Enum1.f_namesToValuesMap__enums_Enum1_));
	}