[project] Move to Java 8 for Neon

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 470034 | | Status | RESOLVED FIXED | | Importance | P3 major | | Reported | Jun 12, 2015 04:18 EDT | | Modified | May 22, 2016 06:41 EDT | | Depends on | 470033, 485108, 485114 | | See also | 470122 | | Reporter | Ed Willink |

Description

The forced move to Java 7 during Mars was unpleasant and enquiries at the time suggested that nobody opposed a move to Java 8 for Neon.

The major motivation, apart from currency, is to move on from the interim Java 7 @NonNull annotations semantics that force us to distribute our own Java 7-specific org.eclipse.jdt.annotation. With Java 8 the code should be less complicated and we have the option to annotate collection elements as @NonNull rather than suppressing associating warnings.

Moving the Classic OCL plugins to Java 7 revealed a conflicting functions signature that would mandate an API breakage. So Classic OCL plugins will stay as Java 5 classes built with Java 8.

[eclipse-ocl-bot]

By Ed Willink on Jun 12, 2015 04:25

A first experiment gives many tens of thousands of errors; not entirely surprising.

The main problem is Bug 470033 which requires recoding of all qualified typenames such as

@NonNull org.eclipse.ocl.pivot.Class

as

org.eclipse.ocl.pivot.@NonNull Class

If this is really necessary then so be it, we will have to make the autogenerators smarter too. But it seems like a bug to me so see how Bug 470033 pans out.

---

If the autogenerators have to change, attempting to autogenerate Java 7 and Java 8 compatible source is probably impossible. Java 7 is a dinosaur so we should ignore it and do just Java 8. Our current Java 7-only generators are probably already not very useful to the wider world.

[eclipse-ocl-bot]

By Ed Willink on Jun 13, 2015 04:02

(In reply to Ed Willink from comment #1)

If this is really necessary then so be it, we will have to make the autogenerators smarter too. But it seems like a bug to me so see how Bug 470033 pans out.

The Java 8 syntax appears to be necessary.

The incompatibility of the Java 8 annotations wrt Java 7 is very disconcerting; I'm not even sure that it is ethical Eclipse-wise. But it highlights that org.eclipse.jdt.annotation is an unstable Eclipse-proprietary facility. Should we even be using such facilities?

I see three choices for Neon. +pro, -con.

a) Eliminate all use of @NonNull, @Nullable

• we are clean and future compatible
• we lose API documentation
• we lose many NPE detection opportunities\
• we are not deceived into a false sense of NPE-free security

b) Continue with Java 7 @NonNull, @Nullable

• no code change today
• we are stuck in an obsolete proprietary rut
• our use of Java 7 @NonNull is unhelpful to our Java 8 consumers
• we fail to detect many NPEs unless the new annotation libraries work on Java 7
• klunky null suppressions for collection elements
• we use a facility not supported by EMF (fortunately JMerge kind of works ok anyway)

c) Migrate to Java 8 @NonNull, @Nullable

• significant trivial code adjustments
• inconvenient adjustments to autogenerators
• our use of Java 8 @NonNull is unhelpful to our Java 7 consumers
• no guarantee that it won't be all change again for Java 9
• still using a proprietary source facility
• using a proprietary run-time facility too\
• we detect many NPEs\
• we detect many more NPEs through annotation libraries for EMF/platform/Xtext APIs (not yet evaluated)\
• we detect collection element NPEs (not yet evaluated)
• we use a facility not supported by EMF (will JMerge like "X.Y.@NonNull Z" ?)

---

The above assessment of pros and cons seems to firmly rule out continuing with Java 7 annotations.

I guess we need to experiment further to see whether JMerge can tolerate Java 8 annotations and whether EMF Collections can be consistently annotated as null-free.

At any rate if we go for c) a migration to a) is pretty easy. The reverse is really hard.

[eclipse-ocl-bot]

By Ed Willink on Jun 13, 2015 04:36

(In reply to Ed Willink from comment #2)

org.eclipse.jdt.annotation is an unstable Eclipse-proprietary facility. Should we even be using such facilities?

See Bug 470122 for a suggestion that the SDK should be @NonNull free.

[eclipse-ocl-bot]

By Ed Willink on Jun 13, 2015 15:53

Another option:

d) Erased @NonNull, @Nullable

We already use /@NonNull/ where warnings are too painful to suppress and /@LazyNonNull/ since it has yet to be implemented.

Perhaps we change all @NonNull, @Nullable to /@NonNull/, /@Nullable/ thereby ensuring that we have clean sources with API documentation.

We can have an Erased2Java7Annnotation and Erased2Java8Annotation conversion that we run automatically to get the benefit of the additional warnings.

Rather than Erased2Java8Annotation, we might be able to autogenerate an annotation library for our own code.

[eclipse-ocl-bot]

By Ed Willink on Jun 13, 2015 16:33

(In reply to Ed Willink from comment #4)

d) Erased @NonNull, @Nullable

Rather than Erased2Java8Annotation, we might be able to autogenerate an annotation library for our own code.

This is a new option

e) Invisible @NonNull, @Nullable

Most, hopefully all, annotations are supplied by an overlay from an annotation library.

We ought to autogenerate an annotation library for our EMF declarations anyway, so doing it for all code is not much more development effort. Just more uto-generation time.

If an autogenerated library works, we should retain the helpful near instant edit warnings. However the turnaround from save through autogeneration builder to updated edit may be sluggish and in danger of stressing unanticipated control flows.

The autogenerated library probably only handles public API. We may have to ask nicely to extend support to private fields.

Autogeneration of a library is probably already in progress/available. We may be able to tailor something to exploit the stronger underlying declarations from a *.ecore model. Only the builder to impose the library on own code may be novel.

[eclipse-ocl-bot]

By Ed Willink on Jun 27, 2015 11:49

Still using Java 7 @NonNull.

JDT has got a little better. About 10 warning suppressions can now be eliminated where JDT now gets it right. However we still have two significant sources of false @NonNull warnings from unfixed JDT bugs.

This highlights a further hazard with auto-generated @NonNull; the auto-generator is empirically tuned to a particular JDT milestone release. Every time JDT gets better, we track it and become backward incompatible. If JDT gets worse, albeit temporarily, we are in a mess.

JDT's @NonNull analysis has been quite helpful in providing an independent check on the OCL non-null analysis, but it is not suitable for export into an API compatible world beyond the development environment.

[eclipse-ocl-bot]

By Ed Willink on Jul 06, 2015 09:46

Discussion on Bug 470033 is encouraging and discouraging.

Encouraging: the klunkiness of org.eclipse.ocl.pivot.@NonNull Class is recognized.

Discouraging: something better might happen; all change again.

Therefore:

Use of today's Java 8 annotations in our API is not tenable.\ Use of yesterday's Java 7 annotations in our API is not tenable.

(Xtext has a few methods annotated in comments).

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 06, 2015 12:17

Hi Ed,

Adding different thoughts:

AFAIU, the major change that needs to be done to the annotations to be Java 8 compliant (as of JDT) is supported by the JSL specification, so I think that doing that movement is justified (even if we had to change something with Java9)

The more worrying concern, is what happen with those clients working with a lower 8 Java (java 7 or even less), because it could be very annoying if they start seeing errors/warnings in their IDE. With some retrospective, I see this "issue" similar to the one we could observe in the Java5/Java6 transition, in which Java6 users added @Override annotation to the methods when implementing interfaces. Then, if a Java5 user downladed that source code in his Java 5 environment, JDT produced an irritating error in those cases. I don't think that people desiring to move to Java6, were really taking into account this side effect in order to take such a decision.

We could even think of the opposite scenario. Clients who wanted to move to Java8 (due to any cool Java8 feature), and some of the java7 annotations can cause some troubles to them.

My opinion is that we are talking about source code facilities, which under this kind of issues clients can deactivate, or even add additional source code facilities (@supresswarnings) to overcome such issues (in case that by any reason they don't want to move to Java8). To me the questions we should address are:

• Do we want to lose final users who stick to old JVMs which can't run java8 code. I think, this was not a concern for you.
• What java8 provides us. The better null/non-null analysis seems a good reason, since we might produce better source code.
• and which is the effort to achieve the transition. It seems to me that with some regular expressions usage, changing the annonations to be Java8 compliant doesn't seem to be very hard (CG can cause more troubles as you mentioned).

[eclipse-ocl-bot]

By Ed Willink on Jul 06, 2015 12:59

I want the OCL source code to be useable by Java 7/8/9/... (and if possible 6/5/...) users. The 7/8 users are of top concern today.

(Many of my criticisms of the JDT @NonNull efforts are distressingly applicable to my efforts with the Pivot model; I'm sympathetic but also critical.) Realistically the JDT @NonNull technology is not ready for prime time. AFAIK Eclipse OCL is the only Eclipse project shipping @NonNull in their sources, and we had to ship a legacy variant.

The migration to use Java 8 annotations is indeed governed by JSL 8 facilities, but these define what/how a type annotation behaves; not how @NonNull behaves.

IMHO there has been a lack of discipline and imagination in providing Java 7 compatibility while also exploiting Java 8. I suspect that, aided by quickfix tooling, the new Java 8 could be augmented by the old Java 7. Not trivial, but some prioritization of resolutions should be possible.

Today the only compatible Java 7/8 source has no uncommented @NonNull. We could introduce our own @NonNull / annotation processor to adjust the behavior of JDT, but I do not have time to develop it, and even if I did, a rival within Eclipse would be at best confusing.

We have done our best alpha testing @NonNull. Our concerns have been ignored, fixes deferred. Sadly, while very useful, it is also deeply flawed. It has no specification. It is implementation driven leading to very complex defaults for template parameters that I cannot comprehend.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 07, 2015 14:55

Hi Ed,

[I've not heard about the parallelism with the java5/6 @Override annotation issue.]

I think I'm missing something here, because I don't follow such a pessimism. Let's discuss it in Italy with the laptop in front and some plugins to play with.

As a last thought. You say that at the best of your knowledge, nobody in Eclipse is using jdt.annotation null analysis, and it seems that you are worried by a source code incompatibility issue which implies:

• Interested in that null analysis and jdt.annotations,
• More specifically, the jdt.annotations provided by the ocl bundle rather than the more recent jdt.annotations provided by Java8

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 07, 2015 15:04

Hi Ed,

I've done an experiment, which seems to demonstrate that the source code compatibility issue can be easily avoided.

a) I have o.e.ocl.pivot (java8) depends on org.eclipse.jdt.annotation (NB!! as a source dependency rather than a genuine plugin dependency)\ b) create a plugin (java7) which depends on o.e.ocl.pivot (normal plugin dependency) and o.e.ocl.jdt.annotation7 (as a source dependency rather than a genuine plugin dependency).

Observations:

• In a), the IDE reports bad located @NonNull annotation when writing method(@NonNull java.lang.String s)
• In b), the IDE doesn't report any issue when writing method(@NonNull java.lang.String s)
• b) properly reports null analysis error when passing null to @NonNull API from a)

Please, give it a try.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jul 08, 2015 05:35

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #11)

I've done an experiment, which seems to demonstrate that the source code compatibility issue can be easily avoided.

Where?

I cannot see how you can possibly bridge the incompatibility that the Java 8 support mandates "java.lang.@NonNull String" while the Java 7 support mandates "@NonNull java.lang.String". There is no compromise without added tooling.

The Java 8 support uses run-time annotations. It is therefore mandatory to have a run-time dependency. This is a further major incompatibility with the Java 7 support which was source.

Until you do a build on Hudson and do CG you cannot tell what you are achieving.

A wierd problem I had a couple of days ago revealed that ASM was failing to load org.eclipse.jdt.annotation because of a source reference and consequently emitting redundant casts and null enforcements. ASM provides another mechanism under which source code should load in as conventional a fashion as possible.

A sure indication that things are not right, and so risky, is to go-to-definition on an @NonNull in our source code. Since only our org.eclipse.jdt.annotation is available on the build path, it should go to our org.eclipse.jdt.annotation. It doesn't it goes to the Java 8 one.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 08, 2015 07:07

Created attachment 255057 Demonstrating SS

Capture.PNG

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 08, 2015 07:09

Hi Ed,

It's obviously a local experiment (easy and quick to create, though). See attachment. The tooling manages to properly report the errors depending on its java source setup, as long as the dependency is declared as a plugin development time one. The go-to-definition also goes as expected, going either to the jdt.annotation (v2.0.0) or to the ocl.jdt.annotation7 one.

I've just done another experiment. Having a runtime dependency on jdt.annotation (with and without [2.0.0,3.0.0) bounds) at o.e.ocl.pivot doesn't make the error appear in Test.java neither. I've also restarted the IDE, just in case. So the compatibility issue doesn't appear even in that case. The compatibility issue ONLY comes back when the dependency on jdt.annotation is re-exported.

In principle, I'd be happy with the behaviour I reported yesterday, that is, having those dependencies as development time one, because I've always seen those java annotations as a development time facility that the JDT tooling understands and that's all. A different history is that now you have to add a runtime dependency to keep the IDE or the builds happy. If that's case, I'm not sure that the change is so good, but as usual you can still go for the Java8 movement, because I don't think we have java7/8 compatilibity issues with respect the NonNull annotations. The only drawback is that the runtime dependency on jdt.annotation needs to be individually defined in every plugin.

[N.B I've never understood your comments around ASM. I don't even know which is the role it plays, or why we need it.]

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jul 08, 2015 07:34

ASM: The Java 7 annotations have @NonNull as a source annotation, therefore the source must be examined to discover if a return is @NonNull; the class file does not contain the required information. ASM is a convenient toolkit that supports this access to the source.

Your experiment seems to confirm that our Mars distribution with Java 7 variant annotations enforced via build.properties works usefully, if not perfectly. If you have something where go-to-definition works, then please provide it. It doesn't work for me.

The Java 7 @NonNull annotations as source annotations cannot annotate the elements of collections, so every collection element requires an @NonNull 'cast' or redundant if test. The Java 7 @NonNull annotations can only co-exist with java 8 on Hudson with our variant distribution. NB source dependencies cannot be versioned so you cannot load both 1.1.0 and 2.0.0. In fact I don;t think you can load 1.1.0 at al; Buckminster is sure that 2.0.0 is better for you.

The Java 8 @NonNull annotations are much better in terms of their technical capability unfortunately their concrete syntax throws up compatibility issues.

Again please provide a demonstration of Java 8 @NonNull source code that can be compiled by a Java 7 user and exhibit Java 8 annotation functionality. Seems like it must be impossible.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 08, 2015 08:08

(In reply to Ed Willink from comment #15)

Again please provide a demonstration of Java 8 @NonNull source code that can be compiled by a Java 7 user and exhibit Java 8 annotation functionality. Seems like it must be impossible.

First at all, my java compiler is normally the IDE :). In any case, I understand that a java compiler needs to compile some code, in hudson in our case, but there are many things to clarify in your request, e.g. "Java 7 user" is very ambiguous.

Does the following would be enough for you ?:

• Create a branch from master
• Move one OCL plugin to Java 8.
• Adjust annotations in that plugin.
• Build in hudson.

so that we would be compiling java 7 code (with ocl.jdt.annotations7 style), and java 8 code (with jdt.annotation -2.0.0- style).

I presume that we have to configure the build to make it use the Java 8 compiler, if it has not already been done.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 08, 2015 10:09

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #16)\

Does the following would be enough for you ?:

• Create a branch from master
• Move one OCL plugin to Java 8.
• Adjust annotations in that plugin.
• Build in hudson.

Well, apparently, tests job succeed on asanchez/java8annotations

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 08, 2015 10:23

Just kicked off another build, in which I've replaced the runtime dependency by the development time one to check the outcome

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 08, 2015 10:40

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #18)

Just kicked off another build, in which I've replaced the runtime dependency by the development time one to check the outcome

The compilation phase has passed (currently running some tests). So apparently, the build doesn't need the runtime dependency. You mentioned another use case on CG, which is better if you test it, because I don't know which one is.

(In reply to Ed Willink from comment #15)

Your experiment seems to confirm that our Mars distribution with Java 7 variant annotations enforced via build.properties works usefully, if not perfectly. If you have something where go-to-definition works, then please provide it. It doesn't work for me.

In the own asanchez/java8annotations:

• If you press ctrl + left click on @NonNull annotation from o.e.ocl.pivot.ui plugin, you will go to the NonNull @interface of o.e.jdt.annotation_2.0.100 installation plugin
• If you press ctrl + left click on @NonNull annotation from any other ocl plugin, you will go to the NonNull @interface of o.e.ocl.jdt.annotation7 workspace plugin

Regards,\ Adolfo

[eclipse-ocl-bot]

By Ed Willink on Jul 08, 2015 15:58

I presume that you are using Java 8 as the ocl-branch-tests job does too.

I'm using Java 7. Your example at least shows a little promise, no errors. This is because we are actually concerned with JDT 7/8 rather than Java 7/8 and Mars JDT is JDT 8.

Unfortunately your example has precisely zero @NonNull functionality on Java 7.

a) hover text over signatures does not show "@NonNull" on returns. On parameters it is not in bold; this suggests an ignored annotation.

b) returning null through @NonNull gives no error/warning.

So long as Eclipse gives a Java crash every couple of hours when I use Java 8, moving to a Java 8 JVM is not an option.

But when it is, it looks as if Java 8 annotations will work for Java 8 developers/users and be ignored for Java 7 users.

[eclipse-ocl-bot]

By Ed Willink on Jul 10, 2015 02:42

(In reply to Ed Willink from comment #20)

So long as Eclipse gives a Java crash every couple of hours when I use Java 8, moving to a Java 8 JVM is not an option.

See Bug 465693. I'll try the workaround.

[eclipse-ocl-bot]

By Ed Willink on Oct 02, 2015 15:33

So long as OCL pivot uses Java 7 annotations derived classes are forced to use no-annotations or Java-7 annotations. As soon as they enable Java 8 annotations they get warnings about failure to comply with the Java 7 annotations.

[eclipse-ocl-bot]

By Ed Willink on Oct 09, 2015 06:03

M2 forces us to use Java 8 for all execution purposes; i.e. test and build launches. This is most easily done by changing the BREEs of build and test plugins to Java 8. But this then causes numerous @NonNull warnings since all references from test/build code to non-test/build code crosses the Java 7/8 boundary and uses the wrong jdt.annotation. Looks like the can't-decide-so-do-nothing-practice will not work for much longer.

But moving to Java 8 annotations will be a mega-source ripple. Must close out all active branches first.

[eclipse-ocl-bot]

By Ed Willink on Oct 15, 2015 06:35

(In reply to Ed Willink from comment #23)

This is most easily done by changing the BREEs of build and test plugins to Java 8.

Plugin launches have an explicit JRE that must be chosen. These must all be changed to Java 8 regardless of BREEs.

Standalone launches can track the host plugin BREE, so moving to 8 is easiest, but causes @NonNull problems, so for now stay at BREE 7, and set all standalone launches to Java 8 explicitly.

[eclipse-ocl-bot]

By Ed Willink on Oct 16, 2015 05:29

An experiment converting just the org.eclipse.ocl.pivot plugin

Initially a couple of thousand errors\ Reduced to 300 by correcting systemic positioning bugs with search and replace\ Then needs individual attention\ Ultimately about 30 new hazards are diagnosed.

The current coding practice of qualifying rather than importing nested classes significantly aggravates the number of @NonNull placement corrections needed.

The add imports quickfix unfortunately doesn't work for @NonNull, requiring nearly 100 manual fixes.

Template parameters used as variables must now be explicitly @NonNull/@Nullable. This may cause a problem with EMF for which derived XXXSwitch needs an @Nullable to be added.

Over 350 files are affected.

[eclipse-ocl-bot]

By Ed Willink on Jan 04, 2016 02:46

The org.eclipse.ocl.common plugin is a bit of a problem. Since it contributes to the Classic OCL, we don't want to move it to Java 8, but without annotations we get tricked by Bug 485093. The attempt to introduce a shared Legacy/Pivot common layer is a pain, but we can't remove it without API breakage.

[eclipse-ocl-bot]

By Ed Willink on Jan 04, 2016 02:54

Using Java 8 annotations on Hudson is also a pain. A number of inconsistent errors appear. These can be reproduced locally by using an Eclipse 4.4 installation. Clearly using Buckminster 4.4 implies using JDT 4.4 to do the build and JDT 4.4 predates/is very early for Java 8 and its annotations.

Buckminster 4.5 requested: Bug 485108

In the meantime, Buckminster has an 'install' command so installing a newer JDT might be possible, but this has a a too-high lower bound on org.eclipse.ant.launching that is a core contribution that cannot be uninstalled by a feature reference. Install fails with a P2 conflict.

The Buckminster 'build' command seems to have no options as to what does the build.

Try a Buckminster newsgroup query.

[eclipse-ocl-bot]

By Ed Willink on Jan 04, 2016 05:40

(In reply to Ed Willink from comment #27)

Buckminster 4.5 requested: Bug 485108

Now available.

Doesn't work ... Bug 471115.

With "-noverify", builds but 16 compile errors.

[eclipse-ocl-bot]

By Ed Willink on Jan 04, 2016 05:50

(In reply to Ed Willink from comment #28)

With "-noverify", builds but 16 compile errors.

Correction 15 errors, reproducible locally using Eclipse 4.5.

The errors all seem to the in the Bug 485093 area where an inherited unannotated getRegistry()/getAdapter() returns a null T with a Class parameter. This clearly a very smelly area which we can afford to hit with some very explicit @Nullable/@NonNull casts. getRegistry() could be avoided by not using ocl.common.

[eclipse-ocl-bot]

By Ed Willink on Jan 04, 2016 16:49

(In reply to Ed Willink from comment #29)

parameter. This clearly a very smelly area which we can afford to hit with some very explicit @Nullable/@NonNull casts. getRegistry() could be avoided by not using ocl.common.

A coupleof wrapper methods can hide the hazrad in OCLDE;legateDomain, and changes to ocl.common can be reverted to just an erasure of @NonNull/@Nullable.

Only one @NonNull X @Nullable[] needs heavy null-warning suppression to avoid an error on JDT 4.5. The handling of Class and @Nullable[] is clearly much better in JDT M4.

Need the Bug 485114 fix to allow JDT M4 to be installed in Buckminster.

[eclipse-ocl-bot]

By Ed Willink on Jan 06, 2016 13:56

(In reply to Ed Willink from comment #30)

Need the Bug 485114 fix to allow JDT M4 to be installed in Buckminster.

The external annotations work on 4.5 so annotating getAdapter() elimnates the major hazard that needed null-suppression fudges.

But JDT 4.6M4 will be better to avoid build surprises.

pushed to master for M5.

[eclipse-ocl-bot]

By Ed Willink on May 22, 2016 06:41

Moving to Java 8 seems to limit how far back we have platform compatibility.

Experimenting at Neon RC2 suggests that the Neon release is ok on Luna/Mars/Neon.

Kepler fails to even start.

Juno at least starts and installs but OCL functionality is invisible.