How to make EEA easier to review, diff & merge

[vorburger]

While working on https://github.com/lastnpe/eclipse-null-eea-augments/issues/10 and merging some EEA java. from an existing project (tracecompass) with some EEA I had already created in this project before, I realized that diff & merge of .eea is quite a PITA... e.g. Git is often wrong and would have lost things, because it's algorithm for finding dupes assume you want to change things when really you want to add.

As (or perhaps even more...) important than diff & merge is simple "review-ability" - unless you are really used to the EEA syntax (which e.g. I currently am not yet..), it's kind of really hard actually to immediately understand exactly what gets changed in e.g. something like the Map get() and remove() of PR https://github.com/lastnpe/eclipse-null-eea-augments/pull/21.

The goal of this issue to collect community feedback is there's anything that can be done to help with this:

• Git settings for *.eea ?!
• Minor changes to the EEA file format (probably too late already)
• Alternative friendlier syntax with generator/converter to EEA?

@svenefftinge once suggested the idea of an Xtext grammar for this... that could be cool, some day! ;-) Imagine representing EEA as a very Java-like Xbase validating DSL language, with the respective @Nullable etc. annotations as if it was Java (but without any Method bodies), and then a little EEA generator off that - that could be neat. BUT one problem that deserves more thought before jumping into such a DSL (which isn't really THAT hard to put together...) is the workflow... The Eclipse IDE integration using which developers maintain EEA files directly (which is how EEA get created today) would become much MORE cumbersome - after you Quick Fix an EEA, how do you get the EEA changed? In ideal world, that would have to be some fancy bi-directional thingie - but that's a little less immediate easy like pie out of the box, e.g. with Xtext.

[ctron]

EEA Compiler

I played a bit around with writing some "compiler" for EEAs. My idea was to write an interface/class like:

class java.time.Instant {

public static Instant now();
public static @NonNull _now();

}

So that you have the original methods and the annotated methods prefixed with _. The code to parse the Java stuff is easy, as you can use JDT for that. However I wasn't able to quickly find out how the generate the method signatures from an AST.

I think having such an approach would make it simpler to write EEAs.

EEA validation

I guess having some sort of unit tests would be necessary as well. However the unit test would be some piece of Java code which simply doesn't compile, due to null analysis. So going with plain maven surefire probably won't work. I am not sure if there is a concept in maven for testing the compilability of java source code.

[vorburger]

My idea was to write an interface/class like:

Hm... I was imagining something like that as well, and on further reflection wonder why it couldn't simply be:

package java.time;

import org.eclipse.jdt.annotation.NonNull;

interface Instant {
    @NonNull Instant now();
}

so just forget my earlier Xtext rambling - why not just good old plain valid Java syntax real .java files? And then a lil' "thingie" which transpiles this new "human readable EEA" .java to & fro the current "unreadable almost like binary" .eea... imagine that.

IMHO for this to real work and keep this new "source" .java in line with what then become purely a "generated output" EEA, you would need BOTH a Maven plugin which during the build would transform this src/.java into target/.eea (you know what I mean), AND a (new) Eclipse plug-in which which works bi-directionally, so it would do the "generation" from .java to .eea (so that you can edit the .java in the workspace while you code and refine the EEA; that part alone you can probably even get with M2E integration just kicking off the same Maven plugin) but, in an ideal world, ALSO the reverse, a Builder which when the target/.eea gets touched correspondingly updates the src/.java (preserving any formatting and comments in it...). The latter would be useful to be able to still use the JDT Quick Fix stuff which creates/modifies the EEA. Or maybe that's just a nice to have later phase 27 could have idea, not critical for a first version... perhaps one would just get used to only directly editing the .java with this approach, and forget about letting the JDT quick fix update the EEA?

All this unless, of course, someone were to put in the work to change JDT to support the syntax above directly; but that's probably a long shot - and actually not required - we could well offer something "on top".

some sort of unit tests

Originally I wasn't even thinking that far, but more imagining that some... small new Maven plugin-in would be able to use JDT headless JAR to validate even just the syntax of EEA - that they are valid and loadable and not "corrupt", not necessarily "correct", yet. But if they were generated from .java (as in above), then that would be a non-issue, anyway.

unit test would be some piece of Java code which simply doesn't compile, due to null analysis

But perhaps we could as a convention start the other way around? Positive not negative testing... if you add a new EEA, it is (should be) because you are getting rid of a warning or error? So one would just put an example of that in examples/ ... we may have to change the example' config to fail for warnings, that should be possible.

not sure if there is a concept in maven for testing

yeah, I dunno either. Or if just a Maven trick / plugin that tests that a compilation fails. But then you would have to have a lot of very small individual Maven artifact projects, one for each annotation.. that's not going to scale.

[maggu2810]

I don't know about discussions and things that are related to the creation of JDT nullness analysis and EEA files. But the format you suggest is IMHO the format the Checker Framework already uses for their astub files. So, why reinvent the wheel and not using stuff that already exist?

[ctron]

What is better than 13 standards? 😁

So, why reinvent the wheel and not using stuff that already exist?

Do you have a pointer on it? So that we can have a look.

[maggu2810]

https://checkerframework.org/manual/#stub-using

[maggu2810]

There are also tools to create a stub file from a class, so you only need to add your annotations and that in a readable text format...

[maggu2810]

Under nullness you can find the annotated JDK: https://github.com/typetools/checker-framework/tree/master/checker/jdk

[vorburger]

@maggu2810 @ctron re. "There are also tools to create a stub file from a class" and all, see also new https://bugs.eclipse.org/bugs/show_bug.cgi?id=525352. The link above has a full JDK with annotation - do you happen to know, or would be willing to dig to find out, if the Checker project has these Stub files e.g. for the JDK somewhere? Or how one produces those from that?

[vorburger]

==> https://github.com/lastnpe/external-annotations-esperanto :alien:

[Bananeweizen]

I have just done a big diff/merge on EEA files, and therefore imagine something more simple actually: Can't we implement a simple editor without xtext, that derives from the builtin text editor, and just adds the following 2 features:

• tooltip on text lines showing the actual meaning (i.e. the same you would see when hovering the same method in the Java editor or displaying it in the Javadoc view)
• hyperlinking (from the EEA method signatures to the Java method declarations).

I imagine that to be sufficient for reviewing and merging. I left out more advanced editing on purpose, since I'm not sure that will ever be needed.

[vorburger]

Guys, in https://github.com/lastnpe/eclipse-null-eea-augments/pull/72 @rPraml contributes something which addresses one aspect we have previously discussed here - his work seems to cover the 'some sort of unit tests / imagining that some... small new Maven plugin-in would be able to use JDT headless JAR to validate even just the syntax of EEA - that they are valid and loadable and not "corrupt"' aspect!

[sebthom]

I created an alternative EEAs repo with an advanced generator at https://github.com/vegardit/no-npe/. The generator can sort existing EEA files. PRs are validated to comply with the expected sorting order and that the declared method signatures match the bytecode. This makes PR reviewing very easy.

This is a commit where I locally bumped the version of one dependency and then executed the generator to add new message signatures https://github.com/vegardit/no-npe/commit/9d615fe85d9eea597a11558715ed469cf73099b7 This is a PR created by the generator bot configured in the repo: https://github.com/vegardit/no-npe/pull/49/files

[vorburger]

@sebthom cool! Would you be interested in being invited to be a commiter and maintainer on this lastnpe org?

[sebthom]

@vorburger thanks for the offer. While I am not interested in modifying the existing lastnpe project (our repo works fundamentally different) I would be willing to donate the content of our repo and refactor/rebrand it as lastnpe 3.x (ideally with some minor help from you regarding infrastructure stuff) and continue working from there.

[vorburger]

@vorburger thanks for the offer. While I am not interested in modifying the existing lastnpe project (our repo works fundamentally different) I would be willing to donate the content of our repo and refactor/rebrand it as lastnpe 3.x (ideally with some minor help from you regarding infrastructure stuff) and continue working from there.

==> https://github.com/lastnpe/eclipse-null-eea-augments/issues/166

[vorburger]

168 states that PR will solve this. I'm trying to better understand how, in https://github.com/vegardit/no-npe/issues/268 and here:

That contributes a sort of "validator" of EEA files, which can update them e.g. to remove deleted methods. It can also "extract" annotations from existing libraries to store them in EEA? (But Eclipse would have seen them on / in those libraries, right, so... I'm not entirely sure I get it. Or is the main point that it can "unify" different nullness annotations?)

It does not, yet, switch to having external nullness annotations for existing libraries in ("fake") *.java instead of in *.eea syntax, to generate that from it, but it could be basis for future work like that.

@sebthom is this accurate, and fair?

[sebthom]

the eea validator ensures the methods defined in eea files actually exist with the given signatures. it also ensures that the annotated signature is valid and does not contain typos (which I found a few in the current eea files of lastnpe).

the generator creates/updates eea files in a stable/sorted form. it creates eea files containing the signatures of all protected and public fields/methods of public/protected classes.

if you want we can have an online meeting and dive a bit into the details.