Bugzilla – Full Text Bug Listing
| Summary: | [compiler][null] inheritance of null annotations as an option | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Stephan Herrmann <stephan.herrmann> | ||||||
| Component: | Core | Assignee: | Stephan Herrmann <stephan.herrmann> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | anchakrk, daniel_megert, jarthana, loskutov, markus.kell.r, sebastian.zarnekow, srikanth_sankaran, tobias.hammerschmidt | ||||||
| Version: | 3.8 | ||||||||
| Target Milestone: | 4.3 M4 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Stephan Herrmann
Hi Stephan, thank you for great description - this is exactly what I would expect how the JDT would handle the null annotations inheritance - a clear +1 from the user point of view. Regarding the P1 and P2... Let assume we will store inherited "null" annotations into the class file (and only those). I guess you were just adding them to the overriding methods in the C type. So from the JLS point of view compiler would generate something to the .class which were not directly coming from corresponding .java file. Why would this be bad? There were no "real" binary code incompatibility introduced, as nobody (except tools like FindBugs ?) relies on "null" annotations, and FindBugs would never complain about those extra hints. "Inject" type of annotations are for sure completely different story, but I doubt that "Nonnull" can be dangerous. What one can do is to add an extra "Classfile generation" compiler option (similar to "Inline finally blocks - large class files but improved performance"). The option could be named "Store inherited annotations used for null analysis (larger class files but better NPE analysis results)". I think this could be a good practical solution. You want strict JLS compatibility - switch the option off. You want better NPE analysis results - switch the option on. As the resulting bytecode would unlikely break anyone, I guess the majority of users would just agree to turn the option on if this would be proposed by the JDT, similar as today's proposal of enabling the error level for NPE warnings. (In reply to comment #1) > Hi Stephan, thank you for great description - this is exactly what I would > expect how the JDT would handle the null annotations inheritance - a clear > +1 from the user point of view. Thanks for confirming. > Regarding the P1 and P2... Let assume we will store inherited "null" > annotations into the class file (and only those). I guess you were just > adding them to the overriding methods in the C type. So from the JLS point > of view compiler would generate something to the .class which were not > directly coming from corresponding .java file. > > Why would this be bad? I've used all my arguments why this is NOT bad in bug 366063, yet I failed to convince the component leads. If you have arguments that haven't been raised in that bug, feel free to re-open the discussion. I for my part can only investigate the other strategy (re-analysing method overriding at compile-time) since I can't but consider the persisting strategy as dead. Since bug 366063 has grown quite long I'd like to highlight just one proposal that was dropped in the course: In bug 366063 comment 55 I proposed to record in a class file if it has been created by ecj and if so with which compiler options. This would allow much more qualified reporting in the dreaded case of mixed compilers. Would that be useful? I made some measurements for the costs of analysing overriding for binary methods. I started by a rather naive implementation of the additional analysis and measured times by compiling the org.eclipse.jdt.ui project with null annotations enabled and in a somewhat controlled setting. The first experiment showed a performance penalty of well over 2%, which is already better than what I expected but still worse than what we typically accept in JDT as the price for any new feature. Fortunately, that penalty could quite easily be reduced below 1%. This is now in a tolerable range I'd say. This means I can start some serious implementation for this feature. One situation that needs to be addressed: A method may specify a null annotation for only one parameter, leaving other parameters and the result unspecified. What happens, if this method overrides a method which already has null annotations? I decided *not* to mix explicit annotations with inherited annotations, i.e., when specifying any null annotations although an overridden method already declares nullness, then the overriding method must specify nullness for *all* parameters and the result. If you fail to do so the compiler will complain about incompatible overriding as before. I hope this makes sense to everybody. Hi Stephan, could you please explain why you decided to not inherit null annotations as soon as a single null-related annotation is given? What I would expect is that overriding methods may weaken the contract of their super methods (e.g. by marking the return type as non-null or any parameter as nullable). Why should one bother to repeat all the annotations for the parameters just to be able to define the return type as non-null? That does not make any sense to me. The same question arises when one thinks about parameters. Why would one bother to enumerate all the annotations for all parameters just to mark one as optional which was something non-nullish before? (In reply to comment #6) > could you please explain why you decided to not inherit null annotations as > soon as a single null-related annotation is given? OK, so I should put my gut feeling into clear arguments :) > What I would expect is > that overriding methods may weaken the contract of their super methods (e.g. > by marking the return type as non-null or any parameter as nullable). Right. Only, I'd call it strengthening of the contract: the method requires less and promises more, so it has to do more work. But certainly contracts are always a matter of perspective (provider vs. consumer). But for the main question: I see the option to inherit null annotations mainly as a means for migration: allow existing code to remain unchanged, even when a super type is updated with null annotations. Now, as soon as you add null annotations to the sub type, the argument of keeping it unchanged no longer holds. Next, I want to avoid confusion in situations like this: interface I { @Nullable Object m(@NonNull Object a1, Object a2); } @NonNullByDefault class C implements I { Object m(@Nullable Object a1, Object a2) { ... } } Now, what's the effective signature of C.m??? With your proposal you'd need to do three searches: - the return type is @Nullable by inheritance - a1 is @Nullable by explicit annotation - a2 is @NonNull by the default annotation of C I don't want you to write this kind of code, so the least I can do here is force you to repeat the return annotation, so there's only two things to consider to find out the effective signature. Finally, I believe overriding inherited nullness will be rather infrequent. OTOH, it's easy to add a null annotation to a method and forget that the overridden method already has some. With an all-or-nothing strategy to inheritance (per method) I can report a mix that might be by accident as an error - which is easy to fix, no matter if it really was a mistake or by intention. Makes more sense now? (In reply to comment #7) > (In reply to comment #6) > > could you please explain why you decided to not inherit null annotations as > > soon as a single null-related annotation is given? > > OK, so I should put my gut feeling into clear arguments :) > > > What I would expect is > > that overriding methods may weaken the contract of their super methods (e.g. > > by marking the return type as non-null or any parameter as nullable). > > Right. > Only, I'd call it strengthening of the contract: the method requires less > and promises more, so it has to do more work. But certainly contracts are > always a matter of perspective (provider vs. consumer). I used the term weakening since implementations should not be allowed to introduce additional constraints but only be more tolerant (reduce the burden for clients). But that's a matter of wording and not too relevant here :-) > > > But for the main question: > > I see the option to inherit null annotations mainly as a means for > migration: allow existing code to remain unchanged, even when a super type > is updated with null annotations. Now, as soon as you add null annotations > to the sub type, the argument of keeping it unchanged no longer holds. > That's an intresting viewpoint that I generally do not share. I'd rather always use inherited annotations since I usually don't want to repeat all the stuff that was already defined in the contract (the interface). In that sense I'd expect that the tool will support that nicely by means of hover hints or something. So in your example > > Next, I want to avoid confusion in situations like this: > > interface I { > @Nullable Object m(@NonNull Object a1, Object a2); > } > > @NonNullByDefault > class C implements I { > Object m(@Nullable Object a1, Object a2) { ... } > } > > Now, what's the effective signature of C.m??? > I'd want to hover over C.m and see the effective signature. The tools should perform the necessary analysis. It can do that far more efficient than I ever could. > With your proposal you'd need to do three searches: > - the return type is @Nullable by inheritance > - a1 is @Nullable by explicit annotation > - a2 is @NonNull by the default annotation of C > > I don't want you to write this kind of code, so the least I can do here is > force you to repeat the return annotation, so there's only two things to > consider to find out the effective signature. > > > Finally, I believe overriding inherited nullness will be rather infrequent. I think so, too. But if I want my implementation to be more tolerant, I don't want to repeat the stuff that was already defined in the interface / super method. > OTOH, it's easy to add a null annotation to a method and forget that the > overridden method already has some. With an all-or-nothing strategy to > inheritance (per method) I can report a mix that might be by accident as an > error - which is easy to fix, no matter if it really was a mistake or by > intention. > > > Makes more sense now? I see what you mean but I'm not convinced. From my point of view it's all about tool support especially in the hover hints. I hope that I could clarify my point, too. (In reply to comment #8) > (In reply to comment #7) > > I see the option to inherit null annotations mainly as a means for > > migration: allow existing code to remain unchanged, even when a super type > > is updated with null annotations. Now, as soon as you add null annotations > > to the sub type, the argument of keeping it unchanged no longer holds. > > > > That's an intresting viewpoint that I generally do not share. I'd rather > always use inherited annotations since I usually don't want to repeat all > the stuff that was already defined in the contract (the interface). I see your point. However, without the migration aspect I wouldn't even work on this RFE, because bending Java semantics (annotations on anything other than classes are never inherited in Java) is not justified just by convenience of use. It will be difficult enough to get sufficient support for this feature even when emphasizing that adoption will not be possible without due to those migration issues. That's why those get the focus. > In that > sense I'd expect that the tool will support that nicely by means of hover > hints or something. So in your example > > > > > Next, I want to avoid confusion in situations like this: > > > > interface I { > > @Nullable Object m(@NonNull Object a1, Object a2); > > } > > > > @NonNullByDefault > > class C implements I { > > Object m(@Nullable Object a1, Object a2) { ... } > > } > > > > Now, what's the effective signature of C.m??? > > > > I'd want to hover over C.m and see the effective signature. The tools should > perform the necessary analysis. It can do that far more efficient than I > ever could. Sounds good. I just can't make any promises for JDT/UI. And people keep emphasizing that there are situations when you read code without having the IDE to aid you. > I see what you mean but I'm not convinced. From my point of view it's all > about tool support especially in the hover hints. I hope that I could > clarify my point, too. Summarizing: My design is driven by urgent need in migration scenarios, not by convenience. Code readability cannot be solved by hovers alone, because you don't have them in all situations. My final point about detecting accidentally mixed annotation-and-inheritance stands unchallenged. New: if convenience is the driver, we'll talk about a variant of an existing quick fix :) (In reply to comment #9) > New: if convenience is the driver, we'll talk about a variant of an existing > quick fix :) It's about convenience and conciseness. A quickfix would repeat all the information each and everywhere thus that's probably not an option (that's what I take from the discussions). Regarding the inheritance + default annotation thing: I'd argue the strongest annotation wins (inheritance declares nullable return type - default is non-null: the contract would be non-null, inheritance declares non-null parameter where default annotation states nullable is ok - nullable wins). (In reply to comment #10) > A quickfix would repeat all the > information each and everywhere thus that's probably not an option (that's > what I take from the discussions). Why so pessimistic? I think it should be easily possible to design a quick fix in a way that it will make handling of this rare case more convenient. May not be worth the effort, though, dunno. > Regarding the inheritance + default annotation thing: I'd argue the > strongest annotation wins (inheritance declares nullable return type - > default is non-null: the contract would be non-null, inheritance declares > non-null parameter where default annotation states nullable is ok - nullable > wins). I didn't think these precedence rules were at stake, but as you're asking let's make them explicit (I think we already had this in bug 186342 but can't find it right now). Your's sure is an academically sound solution, but let's not make things so complicated. I see several situations where this would create quite surprising results. I'm implementing the rule "inheritance beats default", which should be easier to explain and easier to use. To avoid any ambiguities: we first compute the effective contract for the super method (incl. transitive inheritance and defaults), then this is applied to the overriding method. Only if this leaves any elements unspecified a default at the current site will be applied. After playing a bit with my current implementation on real world code I see that the proposed solution of annotation inheritance can still interact with nonnull defaults in surprising ways.
So, here's a new strategy:
Goals:
- avoid asymmetry between default and inheritance
- avoid situations where effective nullness is difficult to see without the IDE
- try to make Sebastian happier :)
New rules:
- partially annotated signatures are generally supported
- if only inheritance affects the current method, unannotated params/return
are filled in from the overridden method
- if only a nonnull default affects the current method,
unannotated params/return are filled in from the default
- if potentially both inheritance and a nonnull default influence
the same method, for each param/return it is checked:
- if inheritance and default both say @NonNull
-> OK, use it
- if inheritance and default are in conflict
-> NOK, require an explicit annotation
Advantages:
- we don't even need a precedence rule for conflicting annotations
(default vs. inheritance)
- when searching nullness for an unannotated param/return it suffices to
find either a matching annotation in the overridden method or an
applicable default. At that point searching can stop.
- @Nullable has to be made visible in more situations than @NonNull,
which is a good message to the developer (this remaining asymmetry
results from not having a @NullableByDefault).
- we can support annotation inheritance for partially annotated overrides
without too much magic.
While it might be possible to guess the intended semantics in even more cases, this would make the semantics more complex. OTOH, the workaround for those few situations of conflict is simple enough: completely annotate the affected method.
comments welcome
As my implementation is now pretty complete I redid my measurements.
Method of measurement:
- Use project org.eclipse.jdt.ui with new compiler option enabled
- Always do one warm-up build which is not measured
- Then do 10 full builds in a loop enabling the following measurement:
- Accumulate the time spent in top-level invocations of the new implementation
Using org.eclipse.jdt.ui as the test candidate yielded the following numbers:
160750 methods analysed (total)
12870 of these where binary (from .class)
This proportion is relevant because the new implementation specifically incurs costs for those binary methods.
Times for three total runs (in milliseconds, measurements were accumulated using nanoTime()):
total time | time spent in new impl | percentage
#1: 52881 | 1110 | 2.1%
#2: 42906 | 1070 | 2.5%
#3: 41876 | 1014 | 2.4%
So the percentage is kind of stable. It is more than in my initial measurements, because the implementation is much more elaborate now.
I'd say it's still tolerable for folks who really want this feature, and the overhead occurs only if the corresponding compiler option is enabled.
On the long run and if this feature will be officially released, it will be worth investigating a mixed strategy that uses some synthetic annotation or proprietary bytecode attribute just for optimization, falling back to the full analysis if this attribute is not present. This should cut down the penalty which is caused by re-analyzing overriding for binary methods, information that is naturally available while compiling those methods in the first place.
I've pushed my current implementation into a new feature branch http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/log/?h=sherrmann/NullAnnotationsForFields_381_inherit This branch starts from current 3.8.1 candidate, adds null annotations for fields as previously published for early access and finally adds the impl from this bug. Some implementation notes of what's in the branch: - add compiler option, FIXME: not yet for batch compiler - on-demand collection of implicit annotations (super/default) - new tagBit for methods: IsNullnessKnown - introduce new superclass of MethodVerifier15: ImplicitNullAnnotationVerifier (pull-up a few utilities) - remove unused old implementations from MethodVerifier - analyze overriding for all methods incl. binary - detect conflicts between (indirectly) inherited annotations plus conflict between inherited and default - individually select which of three possible actions is required: - apply nullness default - apply inheritance of null annotations - check and complain about incompatibilities - change compile order to provide required info when needed - need to call checkImplicitNullAnnotations() as early as during STB.resolveTypesFor() (where we want the nullness defaults) - split BTB.scanMethodForNullAnnotation to defer handling default - integrate analysis of inheritance & defaults & conflicts to support all desired combinations I will prepare a new Beta (http://wiki.eclipse.org/JDT_Core/Null_Analysis/Beta) for Juno SR1. I'm soliciting feedback for what should go into that new Beta. For that purpose, please add your comments to bug 383371. As a summary of the proposed solution in this bug you may refer to comment 0 (top half) plus the strategy update in comment 12. Corresponding tests are here: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/diff/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java?h=sherrmann/NullAnnotationsForFields_381_inherit&id=94c43252a9a5062c721d012dcb43778bd7ae765d I'll certainly need more tests, hints on what should be tests are welcome. Created attachment 220779 [details]
Patch ported to master
Here's an updated patch proposed for inclusion in master, as a basis for high-level review.
@Srikanth, let me know if you want me to push to gerrit for detailed review.
Created attachment 220780 [details]
Proposed changes for JDT/UI
Here's a proposal how this option could be surfaced in JDT/UI
@Markus: inheritance of null annotations had been discussed in bug 186342 and from a conceptual p.o.v. we decided for explicit annotations in overrides. Feedback in bug 385440 signals that from a pragmatic p.o.v. the other option is highly desirable. Do you see any problems with including [ ] Enable inheritance of null annotations as a sub-option of annotation based null analysis? (see patch in comment 17). Default will be DISABLED. Conceptually the main challenge is in making this play well with @NonNullByDefault. I've outlined my strategy in comment 12. See bug 372768 comment 21: we would like to enable [E/W/I] for "Conflict between null annotation and reference" instead of [E/W] as of today. Is this considered in context of this feature? I agree that inheritance of null annotations would be nice, but it also has its downsides. Without tooling, each and every reader of annotated code needs to know and apply all the inheritance rules when reading code. In generated Javadocs, the nullness is very hard to determine. The underlying problem is that the @Inherited meta-annotation is not defined for annotations on methods and on method parameters. I recently stumbled upon the same problem in bug 386410 comment 3. I really think annotation inheritance should be defined by the Java language, and not by individual annotations. That way, the inheritance is also generically accessible for tooling, and we wouldn't need custom solutions at all. Caveats: - @Inherited would need to be extended to consider interfaces as well - the Javadoc tool in Java SE 7 doesn't render @Documented @Inherited annotations from superclasses in the subclass doc. This is a bug. We could start to write extra tooling (e.g. Javadoc hover) for custom annotation inheritance, but this only solves a small part of the problems and adds dependencies to specific annotations into the platform. (In reply to comment #18) > Do you see any problems with including > > [ ] Enable inheritance of null annotations Yes. The problems are that this "inheritance" is not well-defined and easily understood, and it carries UX problems with it. As I said in the original discussion about null annotations in JDT, JDT's master branch is not a playing field to try new language features. Whatever is added to JDT and shipped in version n needs to be sufficiently established, so that we can be sure we don't have to break existing clients in version n+1. If JSR308 would specify @Inherited for methods and parameters and from interfaces, then I would agree with adding an implementation that adheres to that spec. IIRC JSR308 leaves it explicitely to the implementor of the annotation and the respective processing logic to define the semantics in case of inheritance. This allows for greater flexibility since not all annotations may / can possibly agree on the very same semantics when it comes to inheritance. It is quite unlikely that @Inherited will be available for methods in the future. From my point of view, it would be much more helpful for clients and users of JDTs null analysis, if the annotations would define a reasonable semantics for inheritance. One could argue that other annotations (from findbugs, et al) that could be used instead of JDTs own null annotations define another semantics but in that case one could really limit the inheritance of null semantics to JDTs annotations. Does that make sense? Regarding the readability of code: That one is already not possible without navigating the file system due to @NonNullByDefault on the package-info level and I assume that it is much more likely to browse to super types instead of the package-info thing. Hi Markus, thanks for sharing your view. (In reply to comment #20) > Without tooling, each and every reader of annotated code needs to know and > apply all the inheritance rules when reading code. In generated Javadocs, > the nullness is very hard to determine. I've raised pretty much the same concerns in the discussion, but those who requested annotation inheritance unanimously stated that the existing problems without annotation inheritance are far greater than those you mention. > The underlying problem is that the @Inherited meta-annotation is not defined > for annotations on methods and on method parameters. I know and I have raised this question when talking to several folks at JavaOne. People are aware of this question but I don't see any quick movement here (< 3 years). I can try to get a more definite statement. Sebastian do you remember the exact answer? > I recently stumbled > upon the same problem in bug 386410 comment 3. I really think annotation > inheritance should be defined by the Java language, I fully agree. > Caveats: > - @Inherited would need to be extended to consider interfaces as well I think that's why nobody has a quick and general solution here. As Sebastian mentions, this is easier solved for one set of annotations than for all annotations in general. > (In reply to comment #18) > > Do you see any problems with including > > > > [ ] Enable inheritance of null annotations > > Yes. The problems are that this "inheritance" is not well-defined and easily > understood, What do you mean by not well-defined? Among all people with whom I've discussed this (incl. e.g., the 308 spec lead), there was very much agreement. The *only* point that raised a little controversy was how potential conflicts between defaults and inheritance are addressed and I'm convinced the strategy in comment 12 is the most careful strategy that has been proposed to date. As you mention interfaces, they shall certainly be included in the same strategy: if conflicts could arise force the user to be explicit. I'm not planning to accept any dubious code that in any future definition of annotation inheritance would reasonably become illegal. More of the contrary, rejecting code that could potentially become legal, later. What difficulties exactly do you see? > As I said in the original > discussion about null annotations in JDT, JDT's master branch is not a > playing field to try new language features. You know what the Oracle folks answered when I asked them about the future of JSR 305? They are waiting until everybody is already using these annotations before they believe those can be standardized. Seems we're in a deadlock here, if we don't provide the tool so people can really use annotations even before they're officially blessed by a standard. Null annotations are those that are best understood from this group of annotations. As mentioned, I'm not recommending annotation inheritance as any kind of default, it should be a disabled-by-default sub-option of a disabled-by-default option. I'll let others comment on the usability question. (In reply to comment #22) > I know and I have raised this question when talking to several folks at > JavaOne. People are aware of this question but I don't see any quick > movement here (< 3 years). I can try to get a more definite statement. > Sebastian do you remember the exact answer? > The answer to the explicit question about the widening the scope of @Inherited to method-annotations was 'No'. The semantic cannot be consistently for all sorts of annotations in case of diamond inheritance of interface methods and default methods etc. The oracle guys explicitly stated that the semantics for those cases is subject to the implementor of the annotation itself. Thanks for the additional background. It really sounds like we are stuck. I see the usability problems for null annotations without any inheritance. If the only way forward is to accept the proliferation of separate inheritance strategies for annotations, then we'll have to accept that for now. I give my "go" for the compiler option / checkbox, but I don't plan to add special code to make inherited annotations visible in the Javadoc hover etc. We should probably revise bug 353472 then and stop all copying of annotations to overriding/implementing methods. The copying seems to do more harm than good. (In reply to comment #24) > I give my "go" for the compiler option / checkbox, ... Thanks for reconsidering. I appreciate. > We should probably revise bug 353472 then and stop all copying of annotations to > overriding/implementing methods. The copying seems to do more harm than good. I'd propose: make the implementation of bug 353472 depend on the new option: - *either* apply inheritance (this bug) - *or* copy annotations (bug 353472) I agree that both together feel wrong, but while I'll keep recommending to use the explicit variant its good that the UI supports this by copying annotations. I've refreshed my patch, added more checking for methods inherited from different super types: - reject even more situations where implicit annotations are in conflict and request the user to add explicit annotations here This further reduces the danger that we'd accept code today, that will need to be rejected at a later point. OIOW: annotation inheritance only works if really free from any doubt of what it would mean. With this change I released it to master via commit 4ac6f89083748b9c4fc37b738ed82ea1a7c9c63b. Here's a list of open action items: JDT/Core: - doc changes reflecting the new constant in JavaCore - add the new option to the batch compiler - Srikanth wanted to do some level of code review when he finds the time JDT/UI: - I will propose updates for the quick fixes - please apply my patch from comment 17 (hopying it still applies cleanly). (In reply to comment #17) > Here's a proposal how this option could be surfaced in JDT/UI Thanks. I've changed the label a bit and released as http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=c37f380b5ac6dcf5659e6a49704570892980beb1 I've also fixed the annotation copying along the lines of comment 25, see bug 386410. (In reply to comment #26) > (In reply to comment #17) > > Here's a proposal how this option could be surfaced in JDT/UI > > Thanks. I've changed the label a bit and released as > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=c37f380b5ac6dcf5659e6a49704570892980beb1 Thanks! Now we're down to 4 open action items (see comment 25), three are for me, one for Srikanth. I found an oversight: given the new option is enabled and:
class Super {
void m(@NonNull String s) {}
}
class Sub extends Super {
void m(String s) {
super.m(s);
s = null;
}
}
the assignment from null should not be allowed, but it is. Apparently the inherited annotation isn't fully effective yet. The fact that the super call is accepted shows that we know about s being nonnull, but somehow information got lost that s must *always* remain nonnull.
One could argue the current behavior is correct, because in Sub s is not annotated, we only know that it is bound to a non-null value at method entry. While semantically correct, this will confuse users, I'm afraid. It's much easier to treat s as if it was actually annotated even in Sub.
I'll take a look.
(In reply to comment #28) > I found an oversight: [...] Test and fix for this issue have been released via commit 132fd2e863d712144f01b19d7de00f006508d11a. (In reply to comment #25) > Here's a list of open action items: > JDT/Core: > - add the new option to the batch compiler This part has been released via commit 4360ed1bd069f31a09271878d78479e26473ff4a. The new option is "inheritNullAnnot" and recognized as a token for -warn or -err. While this fits fairly well I was puzzled by two observations: (1) Is it really appropriate to model options that influence the semantics of analysis as a sub-option of -warn? A similar case can be made, e.g., for includeAssertNull, which doesn't control a kind of warning but controls the analysis that will eventually cause or not cause a warning (of one kind or other) further down. (2) One related option is already defined at top level: -missingNullDefault. In this case I would have expected the opposite, actually. This option introduces a whole new kind of warning and thus would fit well into -warn, IMHO (except that the intention was originally different from adding a new warning). If it is not just me seeing inconsistencies, then we should file a follow-up bug for cleanup - unless we feel bound by the current design being API, at what point only the new option from this bug is at stake: top level option or token for -warn / -err? The new test was failing on windows environment and I have fixed it via commit http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=4113a7eeb51ee4f116e5cc9ef2de2b6b6e78e544 Stephan, I don't expect this to affect other environments, but it would be nice if you can run the test in our environment and confirm things are fine. (In reply to comment #31) > The new test was failing on windows environment and I have fixed it via > commit > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=4113a7eeb51ee4f116e5cc9ef2de2b6b6e78e544 Thanks, Jay, I looked at the failure this morning and couldn't make any sense from the test log. > Stephan, I don't expect this to affect other environments, but it would be > nice if you can run the test in our environment and confirm things are fine. On Linux the test is happy, too. Thanks again. (In reply to comment #25) > JDT/Core: > - doc changes reflecting the new constant in JavaCore I have released corresponding doc changes via http://git.eclipse.org/c/platform/eclipse.platform.common.git/commit/?id=aa72871d7ce718437c24309888007913174ee850 > [...] With Bug 395555 filed for updating the quick fixes, all planned changes have been / are being taken care of. Closing as fixed for 4.3 M4. Hello, I verified that only if "Inherit null annotations" checkbox is checked, the null annotations are inherited. Verified for: 4.3 M4 with build ID : I20121210-2000 Thanks Anirban |