JavaVersion: find a better way to keep up with Java SE release cadence

[cdietrich]

Adapting constants and switches at 50 places is bad. So we need a better way for that

see changes for eclipse/xtext-core#500

same for the consumption of asm

[cdietrich]

We all gonna die ...

[cdietrich]

places to look at

• org.eclipse.xtext.xbase.ui.builder.XbaseBuilderPreferenceAccess.fromCompilerSourceLevel(String)
• org.eclipse.xtext.xbase.compiler.InMemoryJavaCompiler.toClassFmt(JavaVersion)
• org.eclipse.xtext.xbase.testing.InMemoryJavaCompiler.toClassFmt(JavaVersion)
• org.eclipse.xtext.java.resource.JavaDerivedStateComputer.toJdtVersion(JavaVersion)
• org.eclipse.xtext.xbase.idea.facet.XbaseGeneratorConfigProvider.getTargetJavaVersion(XbaseGeneratorConfigurationState, Module)

maybe others

[cdietrich]

having an enum is bad as well. we would need to ship a new xtext version every 6 months

[cdietrich]

will do it the manual way at least for 9+10

[cdietrich]

@szarnekow the main different for the places is the error treatment

	private def long toClassFmt(JavaVersion version) {
		switch(version) {
			case JAVA5: return ClassFileConstants.JDK1_5
			case JAVA6: return ClassFileConstants.JDK1_6
			case JAVA7: return ClassFileConstants.JDK1_7
			case JAVA8: return ((ClassFileConstants.MAJOR_VERSION_1_7 + 1) << 16) + ClassFileConstants.MINOR_VERSION_0 // ClassFileConstants.JDK1_8
			case JAVA9: return ((ClassFileConstants.MAJOR_VERSION_1_7 + 2) << 16) + ClassFileConstants.MINOR_VERSION_0 // ClassFileConstants.JDK1_9
		}
	}

vs

protected def long toJdtVersion(JavaVersion version) {
        switch (version) {
            case JAVA5:
                ClassFileConstants.JDK1_5
            case JAVA6:
                ClassFileConstants.JDK1_6
            case JAVA7:
                ClassFileConstants.JDK1_7
            case JAVA8: {
                try {
                    ClassFileConstants.getField('JDK1_8').getLong(null)
                } catch (NoSuchFieldException e) {
                    ClassFileConstants.JDK1_7
                }
            }
            case JAVA9: {
                try {
                    ClassFileConstants.getField('JDK9').getLong(null)
                } catch (NoSuchFieldException e) {
                    ClassFileConstants.JDK1_7
                }
            }
        }
    }

do you remember why it is done this way. and if there is no reason what is the preferred way to unify the behaviours

[szarnekow]

Should be fine to do it without reflection.

[cdietrich]

unified the code a bit more => fewer places that need adaption

[ArneDeutsch]

Hi Christian. I checked the code you referenced. For me it seems you have enhanced the solution as far as possible. Doing it better then with the enum seems not possible to me. Even some configuration file with constants for new java versions would need some update. On the other hand the values can not be reliable be computed as the format has been changed in the past and probably will change in the future as well.

Is here anything left that we can do to improve the situation that I do not see?

[cdietrich]

this ticket is to think about a non enum based solution or a enum based solution that works with feature patches

[cdietrich]

we will support lts versions only. next likely 21