phax / jcodemodel

A heavily extended fork of the com.sun.codemodel (from 2013/09)
Other
93 stars 34 forks source link

Enum values in switch() #41

Closed fbaro closed 7 years ago

fbaro commented 8 years ago

When generating a switch on an enum, the case labels should not include the enum class name, but only the enum label.

    public enum E { A, B }
    ...
    switch (e) {
        case A: // Not case E.A
        ...
    }

It's an old issue with the original codemodel. Unfortunately, I have no idea how to fix this with the generate() method architecture. I solved it on my local codemodel copy with an ugly instanceof in JCase.

phax commented 8 years ago

Being on vacation. Fixing this afterwards. Should be easy.

phax commented 8 years ago

Please verify if it works for you as well. I remove emitting the type in the JEnumConstant class only.

mcoblenz commented 7 years ago

Unfortunately this change breaks code generation in my project because it results in emitting an unscoped enum name in a case where the fully-qualified name is required. The generated code looks like this:

public class Simple4 {
  private State_Simple4 __state;

 private void __conserveFields(State_Simple4 nextState) {
        if (getState() == S1) { // need to be State_Simple4.S1
           …
        }
    }
    public enum State_Simple4 {
        S1,
        S2;
    }
}
phax commented 7 years ago

Agreed - had the same issue myself in other scenarios.

phax commented 7 years ago

Do you need it in the 2.8.x series, or can you use Java 8 with upcoming V3?

mcoblenz commented 7 years ago

V3 is probably fine. For now I'm using a locally-modified V3 and it seems to work okay otherwise. Thanks!

phax commented 7 years ago

So you basically reverted the changes mentioned in the above referenced commit - right?

mcoblenz commented 7 years ago

Yes, though I realize that's not the right fix. Presumably we need to check to see what scope the enum was defined in and emit the right thing?

phax commented 7 years ago

Yep :)

phax commented 7 years ago

@mcoblenz So the current solution should suffice both requirements - the difference is done in JCase for enum constants :|

mushaorc commented 6 years ago

3.0.0 apparently is back to fully qualified enumeration constants in switch.

phax commented 6 years ago

I added a quick test and it prints:

-----------------------------------issue41.Issue41Test.java-----------------------------------
package issue41;

public class Issue41Test {

    void dummy(Issue41Test.MyEnum enumParam) {
        switch (enumParam) {
            case A:
            {
                break;
            }
            case B:
            {
                break;
            }
            default:
            {
                break;
            }
        }
    }

    public enum MyEnum {
        A,
        B,
        C;
    }
}

==> so to me it looks good...

mushaorc commented 6 years ago

ok, I probably need to minimize it, but Im switching version 2.8.6 and 3.0.0 for existing project and I have case A: w 2.8.6 and case Enum.A: for 3.0.0

phax commented 6 years ago

Are you using it like this:

...
  final JEnumConstant ca = jEnumClass.enumConstant ("A");
 ...
  final JSwitch s = method.body ()._switch (p);
...
    s._case (ca).body ()._break ();
...

or do you use the _case with a generic JExpr.ref?

mushaorc commented 6 years ago

JEnumConstant enumEntry(JDefinedClass enumClass, String valueName, int value, String description) { JEnumConstant enumConstant = enumClass.enumConstant(valueName); enumConstant.arg(JExpr.lit(value)); return enumConstant; }

    JSwitch switchVF = valueOfIntMethod.body()._switch(numericValueParamVFInt);

    for (Entry<Integer, String> e : usedValues.entrySet()) {
      String valueName = "A";
      JEnumConstant enumConstant = enumClass.enumConstant(valueName);
      switchVF._case(JExpr.lit(e.getKey())).body()._return(enumConstant);
    }

ie as u say, but with return instead break. also it calls enumConstant() twice, plus its having argument.

phax commented 6 years ago

Okay, so you have a method where the case is based on an int and the return value is an enum constant. Working on a test case

mushaorc commented 6 years ago

correct, sorry for being unclear in the begining.

phax commented 6 years ago

Test code:

    final JCodeModel cm = new JCodeModel ();

    final JDefinedClass c2 = cm._package ("issue41")._class ("Issue41Test2");

    final JDefinedClass jEnumClass = c2._enum ("MyEnum");
    final JEnumConstant ca = jEnumClass.enumConstant ("A");
    final JEnumConstant cb = jEnumClass.enumConstant ("B");
    jEnumClass.enumConstant ("C");

    final JMethod m = c2.method (0, jEnumClass, "dummy");
    final JVar p = m.param (cm.INT, "val");
    final JSwitch s = m.body ()._switch (p);
    s._case (JExpr.lit (1)).body ()._return (ca);
    s._case (JExpr.lit (2)).body ()._return (cb);
    s._default ().body ()._return (JExpr._null ());

output:

package issue41;

public class Issue41Test2 {

    Issue41Test2 .MyEnum dummy(int val) {
        switch (val) {
            case  1 :
            {
                return Issue41Test2 .MyEnum.A;
            }
            case  2 :
            {
                return Issue41Test2 .MyEnum.B;
            }
            default:
            {
                return null;
            }
        }
    }

    public enum MyEnum {
        A,
        B,
        C;
    }
}