Community
Participate
Working Groups
TestMap.java ------------ > import java.util.*; > import org.eclipse.jdt.annotation.*; > > @NonNullByDefault > public abstract class TestMap extends AbstractMap<String,@Nullable String> { > > } Test.java ------------ > import java.util.*; > import org.eclipse.jdt.annotation.*; > > @NonNullByDefault > public class Test { > > public static final void test(TestMap testMap) { > testMap.putAll(new HashMap<String,@Nullable String>()); // Error: Contradictory null annotations: method was inferred as 'void putAll(Map<? extends @NonNull String,? extends @NonNull @Nullable String>)', but only one of '@NonNull' and '@Nullable' can be effective at any location > } > > } eclipse.buildId=4.5.0.I20141029-2000 If I do a "Project->Clean..." the error disappears and won't show up until I modify and save Test.java. Sorry if this is a dup, but all the others are marked verified fixed in earlier releases.
(In reply to Chris Hubick from comment #0) > If I do a "Project->Clean..." the error disappears and won't show up until I > modify and save Test.java. Maybe related to Bug 444024?
I can reproduce: TestMap must be read from .class while compiling Test.java. I assume that merging default & explicit annotations just doesn't work for type arguments of a BinaryTypeBinding's super type. Apparently a bug in NonNullDefaultAwareTypeAnnotationWalker.getAnnotationsAtCursor(), which blindly merges annotations without checking for overriding.
See https://www.eclipse.org/forums/index.php?t=msg&th=896396&goto=1514995&#msg_1514995 for a related example - perhaps same root cause?
Released for 4.5 M5 via commit 7c493ed6b34d98f52ff4fc8520fcd8662e5ffd91. @Jay: I believe this would be good to have in 4.4.2. Reading null default annotations from binary types didn't follow our rules for null defaults: - @NNBD was wrongly applied despite explicit @Nullable (comment 0) - @NNBD was wrongly applied to wildcards and type variable use (see comment 3), violating the javadoc of DefaultLocation. Let me know if you'd like a description of the fix, too.
(In reply to Stephan Herrmann from comment #4) > Released for 4.5 M5 via commit 7c493ed6b34d98f52ff4fc8520fcd8662e5ffd91. > > @Jay: I believe this would be good to have in 4.4.2. Stephan, sorry I missed out this one. Yes, We can include this one. I tried a quick attempt at back porting along with other ones, but ran into merge conflicts. Will take this up little later.
(In reply to Jay Arthanareeswaran from comment #5) > (In reply to Stephan Herrmann from comment #4) > > Released for 4.5 M5 via commit 7c493ed6b34d98f52ff4fc8520fcd8662e5ffd91. > > > > @Jay: I believe this would be good to have in 4.4.2. > > Stephan, sorry I missed out this one. n.p. > Yes, We can include this one. I tried > a quick attempt at back porting along with other ones, but ran into merge > conflicts. Will take this up little later. I can handle this tomorrow, unless you beat me to it :)
I see why this doesn't cleanly apply: it depends on commit 63231f253dc3aaae18caf57a7f77da85f8cefe96 (1st part of bug 439516). I'm running tests against this pair of commits on top of R4_4_maintenance. If tests pass, I'll come back and comment on risks with / without those changes.
Created attachment 249951 [details] Combined patch to consider for 4.4.2 RunJDTCoreTests are green. Sorry this came piecemeal. Both patches in question are actually closely related: (from bug 439516 comment #4) > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=63231f253dc3aaae18caf57a7f77da85f8cefe96 > Don't apply nonnull-default on <T extends Object> when read from .class. and (In reply to Stephan Herrmann from comment #4) > Released for 4.5 M5 via commit 7c493ed6b34d98f52ff4fc8520fcd8662e5ffd91. > > Reading null default annotations from binary types didn't follow our rules > for null defaults: > - @NNBD was wrongly applied despite explicit @Nullable (comment 0) > - @NNBD was wrongly applied to wildcards and type variable use (see comment > 3), violating the javadoc of DefaultLocation. The first patch may look big, but it's not complex: we just need to feed one more piece of information into TypeAnnotationWalker - from several places. While null type annotations are still new, I see the risk that adopters might wrongly adjust their code in response to these bugs. OTOH, users that don't enable null type annotations should not be affected, since the main change is in NonNullDefaultAwareTypeAnnotationWalker, which isn't even instantiated in their case. For your convenience, I've attached a patch combined from both commits under consideration. OK to release both changes to 4.4.2? If you're unsure, it's probably OK to tell adopters that they should switch to Mars at their earliest convenience, because quite a few bugs are only fixed in master, in particular most from the cluster of bug 438458.
(In reply to Stephan Herrmann from comment #8) > OK to release both changes to 4.4.2? +1 for back porting. > If you're unsure, it's probably OK to tell adopters that they should switch > to Mars at their earliest convenience, because quite a few bugs are only > fixed in master, in particular most from the cluster of bug 438458. I share the same thoughts. We can still back port this. But this should be our line or response for those we can't accommodate in 4.4.2 :)
Thanks, pushed to R4_4_maintenance: Prerequisite patch #1 from bug 439516 comment #4 via commit e0b75d21a26e97c9a42f6cf8d270088b82d36613 Patch from this bug via commit ecc644906c7fc73a8306ba44f6555680f46dcb51
Verified for 4.4.2 RC1 using build M20150121-0900.
Verified for 4.5 M5 using build N20150121-2000.
Reverted target milestone the 4.4.2 as that is the lowest version containing the fix.
Verified for Eclipse Mars 4.5 M5 Build id: I20150126-2000
