eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[project] Accommodate risks and inadequacies of JDT's @NonNull / @Nullable annotations #2119

Open eclipse-ocl-bot opened 1 day ago

eclipse-ocl-bot commented 1 day ago

| --- | --- | | Bugzilla Link | 564014 | | Status | NEW | | Importance | P3 normal | | Reported | Jun 06, 2020 02:55 EDT | | Modified | Jan 12, 2021 12:34 EDT | | See also | 414237, 509397, 462412, 486868, 526011, 569379 | | Reporter | Ed Willink |

Description

Bug 414237#c28 provides the very sad news that Stephan Herrmann is leaving JDT. Since he was perhaps solely responsible for the Bye Bye NPE initiative we must reassess use of @NonNull / @Nullable annotations now that they move from a secondary priority of a primary committer to perhaps an embarrassment for the residual committers struggling to implement the incessant sequence of next-Javas.

Clearly none of the outstanding bugs regarding external annotations will be fixed. Potentially the embarrassment may be categorized as not-JLS and just deprecated wholescale.

What do @NonNull / @Nullable do for us (we never yse @NonNullByDefault)?

1) Provide formal documentation of interface design intent 2) Synchronize interface design intent over an inheritance hierarchy 3) Provide stronger null hazard analysis with functions 4) Identify redundant null checks within functions

But @NonNull / @Nullable do not help and sometimes even fail when a function from an unannotated project is used.

5) Support provision of annotations for unannotated functions

what would we do if there was no @NonNull / @Nullable / we deleted them all?

1) We have to add numerous informal Javadoc comments for the intent.\ -- too painful, we refactor org.eclipse.jdt.annotation.NonNull as org.eclipse.ocl.annotation.NonNull with commont-only functionality.

2) No checking without JDT\ -- painful. A custom post-builder could check the org.eclipse.jdt.annotation.NonNull annotations.

3) JDT has null analysis\ -- hopefully this won't go. Maybe there will be a JLS standard

4) JDT has null analysis\ -- painful we have to put back all the asserts that we thought were redumndant

5) External annotations have never worked proper;y. After 7 years Bug 414237 is WONTFIX. Clearly the necessary PDE support to migrate the classpath annotations to the MANIFEST so that they are visible to code calling plugins will never happen.

There is no point tolerating inadquate code in the hope of bug fixes that will never happen. We must eliminate the very inadequate dependency on external annotations now. The rest may survive in JDT for a while.

eclipse-ocl-bot commented 1 day ago

By Ed Willink on Jun 06, 2020 03:23

The problem with external annotations is that the original declaration is tweaked zero or more times by tweaks that may not be consistently visible to tooling and consequently tooling may easily neglect to respect the tweaks (which could be conflicting or even wrong). Another problem is that the *.eea files are only semi-readable.

If we instead use wrappers, there is a consistent outer declaration that all tooling should use without difficulty and which is available for access from installed plugins and is even available for use in inheritance hierarchies of wrapper classes.

Since the wrappers are thin static functions, one might hope that JIT will see them as final and optimize them away. A static function can be declared final so adding final should do no harm; it might even encourage JIT.

Just need a consistent naming policy.

?? java.lang.Class.getName is wrapped by AnnotationWrappers.Class_getName(Class) ??

overloading may avoid the need for extended package qualification for clashes.

eclipse-ocl-bot commented 1 day ago

By Sergey Olefir on Jun 06, 2020 11:25

FWIW, I've been using null-analysis (including @NonNullByDefault) and external annotations (to annotate JDK stuff & to annotate all the Maven libraries that are used by the projects) extensively in production for years.

I used 20171005-1200 EE for years and recently migrated to 2019-12 EE -- 2019-12 appears to have one very minor regression but otherwise seems to be 100% compatible with 20171005-1200.

I consider this existing null-analysis fully usable in full-production code (for standard projects, I can't comment on PDE at all) with only very minor nitpicks with easy workarounds such as discussed in Bug 414237 (see e.g. my comments there about fakeNonNull()).

I further consider null-analysis an essential piece of my development process; it is the major reason that I'm sticking with Eclipse for all my Java needs.

The potential loss of this functionality (both null-analysis itself and external annotations in some usable form are critical) would be pretty devastating for my projects.

And on that note -- eclipse-jee-2020-03-R-incubation-win32-x86_64.zip does appear to have significant regressions in null-analysis / external annotations (reporting nullability errors where it shouldn't); I assumed that this is because of 'incubation' status and will be eventually fixed. Now I am less sure.

eclipse-ocl-bot commented 1 day ago

By Ed Willink on Jun 06, 2020 12:37

(In reply to Sergey Olefir from comment #2)

I've been using ... external annotations

Provided you only have one suite of projects with one annotation path you're probably ok. I have two annotation paths for two suites of projects and would like them to work even when one is installed as plugins.

eclipse-ocl-bot commented 1 day ago

By Ed Willink on Dec 08, 2020 08:49

Bug 569379 reports that @NonNull can give strange failures with Tycho 2.1.0 but not 1.6.0.

For the for org.eclipse.ocl.examples.xtext.{build,idioms,idioms.ui,serializer} plugins that might need to migrate to the Xtext project, I experimented with no EEA but still used @NonNull as before. This was differently painful since all the INFOs from Map.get() etc had to be wrapped in maybeNull helpers.

eclipse-ocl-bot commented 1 day ago

By Ed Willink on Dec 29, 2020 14:10

(In reply to Ed Willink from comment #4)

Bug 569379 reports that @NonNull can give strange failures with Tycho 2.1.0 but not 1.6.0.

Setting deriveReleaseCompilerArgumentFromTargetLevel false and ensuring build-wide consistent org.eclipse.jdt.core.prefs content made the problem go away allowing Tycho 2.1.0 to be used.

For the for org.eclipse.ocl.examples.xtext.{build,idioms,idioms.ui,serializer} plugins that might need to migrate to the Xtext project, I experimented with no EEA but still used @NonNull as before. This was differently painful since all the INFOs from Map.get() etc had to be wrapped in maybeNull helpers.

On balance "differently painful" is worse. There are so many asserts/casts/@SuppressWarnings as to make the concept of non-null integrity a joke.

Full EEA or no @NonNull seems the only real choice.

At least the Bug 509397 fix suggests that there may yet be progress.

eclipse-ocl-bot commented 1 day ago

By Ed Willink on Jan 12, 2021 12:34

(In reply to Ed Willink from comment #5)

For the for org.eclipse.ocl.examples.xtext.{build,idioms,idioms.ui,serializer} plugins that might need to migrate to the Xtext project, I experimented with no EEA but still used @NonNull as before. This was differently painful since all the INFOs from Map.get() etc had to be wrapped in maybeNull helpers.

Changing to use EEA allowed over 100 gratuitous maybeNull calls to be eliminated and also associated @SuppressWarnings compensation. Some nonNullSate also could be removed. And a couple of null hazards were then identified.