Community
Participate
Working Groups
Problem: -------- When there is generated code in the project, e.g. from JAXB or ANTLR and you want to work with annotation based null analysis, there is no clear way how to to it. Adding the annotations to the generated code makes no sense, because for each regenerate run, they disappear. What i want: ------------ I think, there is "something" needed to use the EEA mechanism for selected sources as well. One possibility to implement this: ---------------------------------- Perhaps, just check, if beside the .java a .eea file exists and take it into account? Then when i want to use EEA for a generated source, i create an empty .eea file. Then, when I press Ctrl-1 in the generated source, i get the possibility to annotate.
I understand your request. The technical design for external annotations, though, isn't quite ready for annotating arbitrary sources. Luckily, in bug 488494 we added general capability to superimpose .eea information over source types (in that case for project dependencies), but still when I configure .classpath and .eea exactly as desired, the compiler doesn't leverage this information during analysis. So, let's first investigate what needs to be done to fully support this scenario in JDT/Core, before discussing gestures how this can be configured and used in the UI. The first gap I see is in the call from NameEnvironment.computeClasspathLocations(..) to ClasspathLocation.forSourceFolder(..) where we don't pass the externalAnnotationPath. Need to test more, to see if there are more gaps to fill. Just to double-check the intended scenario: do you have a dedicated source folder for your generated sources? I think this would be necessary for making this work. Additionally, I'd assume that those generated sources contain *no* null annotations, i.e., we are not speaking of merging annotations from different sources, but for those it's onle .eea, right?
Question: would it be acceptable to require that a source folder for generated sources compiles into a separate output folder, in order to use external annotations? Reason: when we find a binary type in - say - bin, it would otherwise be impossible to determine if it belongs to the [src & bin] or to the [src-gen & bin] classpath entry, hence we wouldn't know whether or not to apply the externalAnnotationPath from [src-gen & bin] (internally: ClasspathMultiDirectory).
(In reply to Stephan Herrmann from comment #2) > Question: would it be acceptable to require that a source folder for > generated sources compiles into a separate output folder, in order to use > external annotations? I think this overlaps significantly with Bug 486868. I'm unclear why you start introducing a profusion of paths. As a user I use x.y.Z and so I want to annotate x.y.Z. I do not want to have to use a variety of different annotations according to complex annotation path fragmentation policies. Let them all appear to be a uniform annotationpath, just as the Java classpath on a good day is just a uniform provider of classes. In the same way that the uniform classpath is computed from the manifest required bundles, let the uniform annotationpath be computed from the annotation entries of the classpath of the required bundles.
(In reply to Ed Willink from comment #3) > (In reply to Stephan Herrmann from comment #2) > > Question: would it be acceptable to require that a source folder for > > generated sources compiles into a separate output folder, in order to use > > external annotations? > > I think this overlaps significantly with Bug 486868. Frankly, due to contradictory answers in bug 486868 I lost track as to what you are requesting there (after bug 488494 has been resolved). > I'm unclear why you start introducing a profusion of paths. As a user I use > x.y.Z and so I want to annotate x.y.Z. I do not want to have to use a > variety of different annotations according to complex annotation path > fragmentation policies. Let them all appear to be a uniform annotationpath, > just as the Java classpath on a good day is just a uniform provider of > classes. We decided against this the very moment we decided annotationPaths to be a property of a classpath *entry*, not of the classpath as such. And yes, annotation paths are different from the classpath as they enrich existing entries on the classpath rather than adding entirely new stuff. It seems you want the "-annotationpath CLASSPATH" workaround for the batch compiler (bug 440687) to be applicable in the IDE, too. If so, that would be a distinct new RFE. OTOH, my question in comment 2 only concerned the configuration of output folders per source folder. And given that I'm not planning to support mixing "internal" and "external" annotations in the same class, the "one big annotationpath" approach would cause a lot of complications. Proposed here: specific source folder with specific output folders can be combined with external annotations. This only *moves* the line between normal and "foreign" (externally annotated) code: previously that line was between own code and library code. With this proposal sources in specific source folders are considered "foreign", too. And we still need to decide what happens, when a generated source file with external annotations attached has a null annotation in the source. It seems we need a new analysis & error to protect against this unsupported situation. PS: with Java 9 around the corner, "classpath" doesn't have a bright future anyway.
New Gerrit change created: https://git.eclipse.org/r/87380
(In reply to Eclipse Genie from comment #5) > New Gerrit change created: https://git.eclipse.org/r/87380 Some experiments to interpret annotationPath also for source folders, assuming they have a separate output folder (otherwise we can't recognize from a .class file what configuration we should apply). With inserting external annotations early on, also these generated sources will report problems if analysis finds that the implementation doesn't match the external annotations. This could be very interesting, it can also be annoying, since you won't be able to rephrase the sources to make errors/warnings go away. One may or may not want to ignore optional problems on such a folder. One should probably experiment a bit more with this proposed change. => Suspending work on this for now.
Ran out of time for 4.7. Bulk move to 4.8.
bulk move out of 4.8
Bulk move to 4.13. Hopefully I can dedicate a sprint during 4.13 just to static analysis (null and resource).
*** Bug 485088 has been marked as a duplicate of this bug. ***
Working with EMF in a null-annotated project. EMF supports merging and keeps the annotation on the generated source -.. somehow. For the return value of a method this works, for the arguments not. So having this issue fixed would really help.
@Frank. I use null-annotated EMF and sometimes wonder whether it is actually worth it because with four year old bugs like this and Bug 486868 continually pushed into oblivion by Java-N+1, it appears that Bye-Bye-NPE is never going to be fulfilled. Unfortunately wherever you hit a crack in the null-analysis you introduce an opportunity for the compiler to fool you into thinking you are NPE-safe when you are not. I get NPEs in null-safe code some due to the cracks, some due to the number of assets that I need to silence 'bigus' warnings. In regards to EMF, which is strictly speaking still at Java 5 with a few later tweaks, it is amazing how much sort of works, but it is only sort of. I would caution against adding annotations to EMF, since bad things happen, particularly when you mix projects. For instance IIRC if you annotate Resource.getContents() as having a non-null return, you can get in a mess if your derived Resource overrides getContents. That said, annotations on classes suuch as URI that you do not override are probably ok. I certainly would not dream of annotating EMF's arguments.
Bug 564014 discusses derisking the ongoing use of @NonNull / @Nullable annotations for the OCL (and QVTd) projects.
.
Hi having the EEA path be configured with the output path seems strange. It forces the user to re-arrange the output path just because of the EEA. What about if the Java project can have a single EEA dir, valid for the all source dirs? Regardless how many source and output dirs are configured. Frank
Regarding my EMF use-case, this can be solved by having a processor in the MWE2 workflow, patching the generated sources. - Returned lists always non-null - Returned values non-null if cardinality 1-1, same for setters - Returned values nullable if cardinality 0-1 - Factory return non-null - Switch case-signatures with non-null Nevertheless there are many other use cases for this ticket. So this is just a notice for EMF users.
https://bugs.eclipse.org/bugs/buglist.cgi?classification=Modeling&component=Core&list_id=19639782&product=EMF&query_format=advanced&short_desc=Java%208%20annotations&short_desc_type=allwordssubstr identifies Bug 485089 as WONTFIX for the switch/adapter factories identifies Bug 485091 as WONTFIX for the non-null collections You might be interested in the modified templates in /org.eclipse.ocl.examples.codegen/templates/model that fix the annotations for Switch and AdapterFactory. You might be interested in the /org.eclipse.ocl.examples.codegen/src/org/eclipse/ocl/examples/codegen/oclinecore/OCLinEcoreGeneratorAdapterFactory.java approach that enables an OCL-enriched Ecore to be loaded, modified to a Java-enriched Ecore in memory and then genmodelled to pure Java. I've played with @NonNull lists and such like. A better generator could be good but you have to rely on EEA to sort out the EMF inheritance roots else you get some really strange diagnostics. Unfortunately putting EEA somewhere better in Java is not really the solution if your code may be installed rather than checked out. Once installed the .classpath is lost; PDE must put the EEA in the MANIFEST.MF. And JDT needs to understand how MyClass inheriting HerClass inheriting HisClass inheriting EMFClass may have conflicting His and Her external annotations for EMFClass (and I might be a non-annotation user). Given that the EEA PDE bug has been outstanding for years and there is no JDT-committer support and the problem is too-hard/too-low-down on my priority list, I have reluctantly resolved that for my installed functionality purposes there must be NO external annotations. Even if your functionality is solely in one family of checked out projects today, I recommend you consider whether you can be confident that that will always be so. Making EMF better is a worthy goal, but when that involves a fight with EMF's basically Java 5 vintage, you may find that you are in a losing battle. The correct solution is to update the JET templates with annotation helper calls so that the useNonNullAnnotations genmodel option provides the additional declarations.
I played a bit more with the change (after rebase & conflict resolution) and found it quite useful already. Contrary to what I wrote above (comment 4), it seems to work even for different source folders compiling into the same output folder. The UI is "almost" capable to configure this, already: If an annotation path has been defined in .classpath, it is shown in the Build Path dialog and can be changed accordingly. Only initial definition of an annotation path is not yet possible in the UI. Since the change is quite straight forward I'm going to merge it for 4.19.
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/87380 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=ca5f41b9281c66c20874cdd0c498256a910212b7
(In reply to Eclipse Genie from comment #19) > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/87380 was > merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=ca5f41b9281c66c20874cdd0c498256a910212b7 Released to master for 4.19 M1 (In reply to Stephan Herrmann from comment #18) > The UI is "almost" capable to configure this, already: If an annotation path > has been defined in .classpath, it is shown in the Build Path dialog and can > be changed accordingly. Only initial definition of an annotation path is not > yet possible in the UI. Over to bug 569953
(In reply to Stephan Herrmann from comment #18) > I played a bit more with the change (after rebase & conflict resolution) and > found it quite useful already. Could you clarify exactly what the improved functionality is? Your mention of .classpath suggests that it does not solve the 'PDE' issue whereby the external annotations need to be in MANIFEST.MF to contribute to the calling code experience.
(In reply to Ed Willink from comment #21) > (In reply to Stephan Herrmann from comment #18) > > I played a bit more with the change (after rebase & conflict resolution) and > > found it quite useful already. > > Could you clarify exactly what the improved functionality is? "Enable EEA for generated sources" :) > Your mention of .classpath suggests that it does not solve the 'PDE' issue > whereby the external annotations need to be in MANIFEST.MF to contribute to > the calling code experience. Correct. Comment 0 does not mention PDE nor MANIFEST.MF, ergo that situation wasn't changed here.
One more note: while the bug requests EEA for "generated sources", I chose not to test for IFile.isDerived(), because - I wasn't sure if all tools that generate sources actually mark their result as derived - evaluating EEA for sources is not logically connected to the derived status. Hence the implemented feature is actually "evaluate EEA for all source folders with an annotationpath configured".
Thanks Stephan, I am excited to have this available.
(In reply to Frank Benoit from comment #24) > Thanks Stephan, I am excited to have this available. Frankly, I was in need of this feature myself :) With the UI bugs (bug 569953 and bug 569955) also merged, it would be cool if you could pick up the next build for a test drive.
(In reply to Stephan Herrmann from comment #22) > > Could you clarify exactly what the improved functionality is? > > "Enable EEA for generated sources" > :) Sorry this means nothing to me. I have been using some EEA in association with auto-generated sources for at least a couple of years. For my own auto-generated sources I ensure that the auto-generator provides the correct @NonNull etc automatically, subject to a use/don't use generator preference. No need for EEA at all. For the platform I've tried fixing up getAdapter() but regretted it since as many warnings appeared as were fixed. For the Collection library I have fixed some things but I have an 'impossible' warning that some method in Set is not correctly overridden. I just ignore it. For EMF, I use EEA for some solid root methods such as ResourceSet.getResources(). But this isn't auto-generated. Annotating too much of EMF just moves the not-properly-checked boundary to more obscure places giving more obscure hello-again NPEs. In order to use @Nullable correctly to characterize the auto-generated Adapter and Switch classes, I was forced to fix the JET templates. See Bug 485089. For much of EMF, the functionality is two phase, may-be-null during construction / XMI loading, should-not-be-null thereafter. Something more powerful than @LazyNonNull is needed to handle this. Using EEA to prefer one phase ensures that it's hello-again NPE in the other phase. However EMF's EList-returning get-methods and many create-methods are solid candidates for an @NonNull EEA. However these are public API that fails to propagate correctly. For instance if my base OCL model has EEA, these EEAs cannot be inherited by derived QVTd models rather QVTd has to respecify all the EEAs and the code is then vulnerable to the fundamental intermittency, which is most easily understood by considering an EEA for Map.get() which is defined in OCL's EEAs and again in QVTd's EEAs. (An import declaration at the EEA root might help.) Normally we do not care which class loader loads Map since the loaded Map is the same regardless, unless of course we have the excitements of a mad classpath. However with EEA, my theory is that the utility of EEA depends on the EEAed class being loaded by a class loader with the correct EEAs declared, so if QVTd gets to use a class loaded by OCL the EEAs may be 'wrong'. Worse, if Map.class gets loaded by some platform/EMF functionality then the EEAs are omitted and all Map.get's are splattered with spurious infos or worse. Fundamentally I find EEA very useful but unsound. I'm unclear what use case this fix is going to make better. AFAICT the only way to reliably make generated code consistently better is to ensure that the generated source has the requisite @NonNull etc in the true source code, possibly requiring a post-generator to inject them. This is clearly not possible for distributed source code such as Java or the platform, but is only a minor post-generator problem for project generated sources.
Ed, for future maintenance our bugzilla threads are a crucial source for information, which is very hard to digest if one bug entry discusses too broad a range of issues. Hence I will respond here only to what I see as part of this bug: (In reply to Ed Willink from comment #26) > (In reply to Stephan Herrmann from comment #22) > > > Could you clarify exactly what the improved functionality is? > > > > "Enable EEA for generated sources" > > :) > > Sorry this means nothing to me. I have been using some EEA in association > with auto-generated sources for at least a couple of years. I'm not sure how to read this. Are you saying: (1) "I'm not interested in using EEA for generated sources" (2) "I don't understand what 'enable EEA for generated sources' should mean" (3) "I've been using EEA for generated sources for a long time" Please tell me, which comes closest to your intention. If it's (1) then please don't further comment on *this* bug. If it's (3) then please tell me how you did it, because to all my understanding this was not possible prior to http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=ca5f41b9281c66c20874cdd0c498256a910212b7 For all other concerns please either file a new bug, or (preferably) find an existing bug that exactly matches your concern, to pick up discussion there. If you look for a place for discussing strategies regarding the use of null annotations, please consider the JDT forum, or perhaps contact the community of http://www.lastnpe.org/ Thanks. Unless you encounter situations, where the mentioned fix does not work, or demonstrate that it was never relevant in the first place, this bug is resolved and no further discussion is needed.
(1) I'm very interested in using EEA, but I'm very wary that without PDE support EEA has created more problems for me than it solves. See Bug 486868. (2) I don't know what you mean by a generated source. I provided examples of my and EMF generated sources and what didn't work in the hope that you would identify what does work. I need to see an example of what the fix actually fixes for you. A specific example of a challenge. If EMF's EFactory.create has an @NonNull EEA, all derived generated code gets warnings until each derivation also annotates all its transitive createXXXX methods.
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/174355
Gerrit change https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/174355 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=6e4a9daa6c16fb92b28968c934d24451881f2feb
Verified for Eclipse Version: 2021-03 (4.19) M1 with Build id: I20210106-1800 [ref comment 2]
(In reply to Manoj Palat from comment #31) > Verified for Eclipse Version: 2021-03 (4.19) M1 with Build id: I20210106-1800 > [ref comment 2] Oops.. commented the wrong bug - was intended for bug 570014
Verified for 4.19 M3 using build I20210216-2020
