Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

WIP: External null annotations via jar from maven central #4513

Closed

Conversation

triller-telekom
Copy link
Contributor

@triller-telekom triller-telekom commented Nov 7, 2017

This PR is similar to #4217 but instead of creating a project that
contains the annotations in a directory it sets up the IDE to read the
annotations from a jar file.

The reason behind this is that the IDE should be for the every day
developer who will not create external annotations. Nevertheless he
should benefit from them which is why they are provided in a zip file.

This jar file should be hosted externally, maybe together with
lastnpe.org (on maven central as a maven dependency). That is also
the place to add new annotations, i.e.
download the project from there, change the IDE to use this project as a
directory, add your annotations and afterwards the project releases a
new jar file on maven central.

In this PR I created a setup task for the IDE to download the
annotations file and place it as "eeas.jar" in the targetplatform
folder via a maven task. For maven I am using the maven-dependency-plugin
which enables downloading a maven dependency project and copying
its jar file into a place where the IDE an access it. The compile task
of maven uses the CLASSPATH as "annotationpath", because the annotations
are added as a maven dependency and put on the classpath.

Also I reactivated the "Unchecked conversion from non-annotated type to
@nonnull type" check by setting it to warning level, otherwise (if set
to ignore) external annotations are not needed.

As an example I updated the ".classpath" file of the yahoo binding to
use the downloaded jar file.

The parts which are still valid from PR #4217 are:

  • all created external annotations
  • all updated .classpath files (IF adjusted to use the zip file instead
    of a directory)

We still need one central project on maven central that contains all
external annotations which is why this PR is WIP for now.

Signed-off-by: Stefan Triller stefan.triller@telekom.de

This PR is similar to eclipse-archived#4217 but instead of creating a project that
contains the annotations in a directory it sets up the IDE to read the
annotations from a zip file.

The reason behind this is that the IDE should be for the every day
developer who will not create external annotations. Nevertheless he
should benefit from them which is why they are provided in a zip file.

This zip file should be hosted externally, maybe together with
lastnpe.org. That is also the place to add new annotations, i.e.
download the project from there, change the IDE to use this project as a
directory, add your annotations and afterwards the project releases a
new zip file.

In this PR I created a setup task for the IDE to download the
annotations file and place it in a "eea" directory in the targetplatform
folder. For maven there is a new dependency to the wagon-maven-plugin
which enables downloading a single file to be placed where the IDE
stores the file too. The compile task uses this zip file as
"annotationpath".

Also I reactivated the "Unchecked conversion from non-annotated type to
@nonnull type" check by setting it to warning level, otherwise (if set
to ignore) external annotations are not needed.

As an example I updated the ".classpath" file of the sonos binding to
use the downloaded zip file.

The parts which are still valid from PR eclipse-archived#4217 are:
* all created external annotations
* all updated .classpath files (IF adjusted to use the zip file instead
of a directory)

The URL where the file will be hosted is still left open, which is why
this PR is WIP.

Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@maggu2810
Copy link
Contributor

@triller-telekom Are there any news?

@triller-telekom
Copy link
Contributor Author

@maggu2810 No there are no news.

The latest things I have done were to trying to put the jar file containing the annotations on the eclipse classpath. The goal was to have the annotations in a bundle located in a p2 repository that is in our targetplattform.

However, I failed to to set the annotationpath for the rt.jar of the JDK within eclipse to "CLASSPATH". It only allowed me to choose a jar file from the workspace or a path in the file system. For "Headless Consumption" this works, see https://wiki.eclipse.org/JDT_Core/Null_Analysis/External_Annotations and as implemented in the maven pom file of this PR.

For now I won't put any more effort into this since the benefit which we get (mainly some less warnings on collections) is not worth it for now.

I heard there are plans to MAYBE include external annotations for JDK classes into the IDE just like the jdt.nonnull, nullable, etc. so the null analysis can pick them up automatically, but I don't know the status of if its being actively developed.

Long story short: I am not actively pursuing this PR anymore.

@maggu2810
Copy link
Contributor

Hm, so I still don't understand why we haven't use #4217 to get rid of all that warnings as long as there is no other solution. Reverting the approach #4217 shouldn't be a problem if there are better solutions.

@triller-telekom
Copy link
Contributor Author

@maggu2810 That is because #4217 included the need for a specific project in the workspace containing the external annotation files. And we do not want to require developers to have such a project in their IDE.

Binding developers in OH2 for example only have to checkout and import their binding projects to work with, everything else comes from the targetplatform.

@maggu2810
Copy link
Contributor

Isn't the official way to develop for Eclipse SmartHome to use the Eclipse SmartHome IDE setup?
So the projects in the IDE are controllable by the Oomph setup.

If openHAB does not want to add an additional project to its workspace by its Oomph setup I don't see the relationship here.
So, it is better to have the warnings in a downstream IDE and in the upstream project then solve it for the upstream project?

@kaikreuzer
Copy link
Contributor

@triller-telekom This is marked WIP, but I understood that you are not actively pursuing this anymore - I'd therefore close this PR to avoid the impression that this is being worked on.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants