Skip to content

Commit d9a0989

Browse files
stephan-herrmannstephan-herrmann
authored
"var" type inferrence bug with generic wrapping (#5054)
+ Ignore type bound of "? extends Object" + Resolve new "unnecessary @SuppressWarnings" with new test as a witness + Preserve null info from erased bound '? extends @nonnull Object' + normalize eea signature to fold +Ljava/lang/Object; to * + encode variants with null annotations as *1 or *0 resp. + Test adjustments Fixes #5028
1 parent 3a7e978 commit d9a0989

31 files changed

Lines changed: 367 additions & 253 deletions

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/classfmt/ExternalAnnotationProvider.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ void initialize(InputStream input) throws IOException {
150150

151151
// discard optional meta data (separated by whitespace):
152152
annotSig = trimTail(annotSig);
153+
rawSig = normalizeWildcards(rawSig, false);
154+
annotSig = normalizeWildcards(annotSig, true);
153155
if (isSuper) {
154156
if (this.supertypeAnnotationSources == null)
155157
this.supertypeAnnotationSources = new HashMap<>();
@@ -167,6 +169,23 @@ void initialize(InputStream input) throws IOException {
167169
}
168170
}
169171

172+
private String normalizeWildcards(String signature, boolean isAnnotated) {
173+
signature = signature.replaceAll("\\+Ljava/lang/Object;", "*"); //$NON-NLS-1$ //$NON-NLS-2$
174+
if (isAnnotated) {
175+
// '1' on either the wildcard or its Object bound or both => treat like '@NonNull ?'
176+
final String nonNullWildcard = "*1"; //$NON-NLS-1$
177+
signature = signature.replaceAll("\\+1L1java/lang/Object;", nonNullWildcard); //$NON-NLS-1$
178+
signature = signature.replaceAll("\\+L1java/lang/Object;", nonNullWildcard); //$NON-NLS-1$
179+
signature = signature.replaceAll("\\+1Ljava/lang/Object;", nonNullWildcard); //$NON-NLS-1$
180+
// '0' on either the wildcard or its Object bound or both => treat like '@Nullable ?'
181+
final String nullableWildcard = "*0"; //$NON-NLS-1$
182+
signature = signature.replaceAll("\\+0L0java/lang/Object;", nullableWildcard); //$NON-NLS-1$
183+
signature = signature.replaceAll("\\+L0java/lang/Object;", nullableWildcard); //$NON-NLS-1$
184+
signature = signature.replaceAll("\\+0Ljava/lang/Object;", nullableWildcard); //$NON-NLS-1$
185+
}
186+
return signature;
187+
}
188+
170189
private boolean isValidSignature(String trim, boolean expectTypeArguments) {
171190
if (trim.length() > 0) {
172191
char first = trim.charAt(0);

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/AnnotatableTypeSystem.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.eclipse.jdt.internal.compiler.lookup;
1818

1919
import org.eclipse.jdt.core.compiler.CharOperation;
20+
import org.eclipse.jdt.internal.compiler.ast.Wildcard;
2021
import org.eclipse.jdt.internal.compiler.util.Util;
2122

2223
/* AnnotatableTypeSystem: Keep track of annotated types so as to provide unique bindings for identically annotated versions identical underlying "naked" types.
@@ -187,6 +188,13 @@ public WildcardBinding getWildcard(ReferenceBinding genericType, int rank, TypeB
187188
if (genericType.hasTypeAnnotations())
188189
throw new IllegalStateException();
189190

191+
long objectBoundNullTagBits = 0;
192+
if (boundKind == Wildcard.EXTENDS && bound != null && bound.id == TypeIds.T_JavaLangObject && otherBounds == null) {
193+
objectBoundNullTagBits = bound.tagBits & TagBits.AnnotationNullMASK;
194+
boundKind = Wildcard.UNBOUND;
195+
bound = null;
196+
}
197+
190198
WildcardBinding nakedType = null;
191199
boolean useDerivedTypesOfBound = bound instanceof TypeVariableBinding || (bound instanceof ParameterizedTypeBinding && !(bound instanceof RawTypeBinding)) ;
192200
TypeBinding[] derivedTypes = getDerivedTypes(useDerivedTypesOfBound ? bound : genericType);
@@ -197,20 +205,22 @@ public WildcardBinding getWildcard(ReferenceBinding genericType, int rank, TypeB
197205
continue;
198206
if (derivedType.boundKind() != boundKind || derivedType.bound() != bound || !Util.effectivelyEqual(derivedType.additionalBounds(), otherBounds)) //$IDENTITY-COMPARISON$
199207
continue;
200-
if (Util.effectivelyEqual(derivedType.getTypeAnnotations(), annotations))
201-
return (WildcardBinding) derivedType;
208+
WildcardBinding derivedWildcard = (WildcardBinding) derivedType;
209+
if (Util.effectivelyEqual(derivedType.getTypeAnnotations(), annotations) && derivedWildcard.hasNullTagBits(objectBoundNullTagBits))
210+
return derivedWildcard;
202211
if (!derivedType.hasTypeAnnotations())
203-
nakedType = (WildcardBinding) derivedType;
212+
nakedType = derivedWildcard;
204213
}
205214

206215
if (nakedType == null)
207216
nakedType = super.getWildcard(genericType, rank, bound, otherBounds, boundKind);
208217

209-
if (!haveTypeAnnotations(genericType, bound, otherBounds, annotations))
218+
if (!haveTypeAnnotations(genericType, bound, otherBounds, annotations) && objectBoundNullTagBits == 0)
210219
return nakedType;
211220

212221
WildcardBinding wildcard = new WildcardBinding(genericType, rank, bound, otherBounds, boundKind, this.environment);
213222
wildcard.id = nakedType.id;
223+
wildcard.nullTagBitsFromErasedObjectBound = objectBoundNullTagBits;
214224
wildcard.setTypeAnnotations(annotations, this.isAnnotationBasedNullAnalysisEnabled);
215225
return (WildcardBinding) cacheDerivedType(useDerivedTypesOfBound ? bound : genericType, nakedType, wildcard);
216226
}

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CaptureBinding.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public CaptureBinding(WildcardBinding wildcard, ReferenceBinding sourceType, int
6565
// propagate from wildcard to capture - use super version, because our own method propagates type annotations in the opposite direction:
6666
super.setTypeAnnotations(wildcard.getTypeAnnotations(), wildcard.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled);
6767
this.tagBits |= wildcard.tagBits & (TagBits.HasNullTypeAnnotation|TagBits.HasMissingType);
68+
this.tagBits |= wildcard.nullTagBitsFromErasedObjectBound;
6869
} else {
6970
computeId(this.environment);
7071
this.tagBits |= wildcard.tagBits & (TagBits.AnnotationNullMASK|TagBits.HasNullTypeAnnotation|TagBits.HasMissingType);

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.HashMap;
2323
import java.util.function.Supplier;
2424
import org.eclipse.jdt.internal.compiler.ast.ASTNode;
25+
import org.eclipse.jdt.internal.compiler.ast.Wildcard;
2526
import org.eclipse.jdt.internal.compiler.util.SimpleLookupTable;
2627
import org.eclipse.jdt.internal.compiler.util.Util;
2728

@@ -373,6 +374,10 @@ public RawTypeBinding getRawType(ReferenceBinding genericType, ReferenceBinding
373374
public WildcardBinding getWildcard(ReferenceBinding genericType, int rank, TypeBinding bound, TypeBinding[] otherBounds, int boundKind) {
374375
if (genericType == null) // pseudo wildcard denoting composite bounds for lub computation
375376
genericType = ReferenceBinding.LUB_GENERIC;
377+
if (boundKind == Wildcard.EXTENDS && bound != null && bound.id == TypeIds.T_JavaLangObject && otherBounds == null) {
378+
boundKind = Wildcard.UNBOUND;
379+
bound = null;
380+
}
376381

377382
ReferenceBinding unannotatedGenericType = (ReferenceBinding) getUnannotatedType(genericType);
378383
int otherBoundsLength = otherBounds == null ? 0: otherBounds.length;

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java‎

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public class WildcardBinding extends ReferenceBinding implements HotSwappable{
5252
ReferenceBinding[] superInterfaces;
5353
TypeVariableBinding typeVariable; // corresponding variable
5454
LookupEnvironment environment;
55+
long nullTagBitsFromErasedObjectBound = 0; // stores null info from '? extends @NonNull Object'
5556

5657
/**
5758
* When unbound, the bound denotes the corresponding type variable (so as to retrieve its bound lazily)
@@ -161,46 +162,46 @@ public long determineNullBitsFromDeclaration(Scope scope, Wildcard wildcard) {
161162
}
162163
}
163164
}
164-
if (this.bound != null && this.bound.isValidBinding()) {
165-
long boundNullTagBits = this.bound.tagBits & TagBits.AnnotationNullMASK;
166-
if (boundNullTagBits != 0L) {
167-
if (this.boundKind == Wildcard.SUPER) {
168-
if ((boundNullTagBits & TagBits.AnnotationNullable) != 0) {
169-
if (nullTagBits == 0L) {
170-
nullTagBits = TagBits.AnnotationNullable;
171-
} else if (wildcard != null && (nullTagBits & TagBits.AnnotationNonNull) != 0) {
172-
Annotation annotation = wildcard.bound.findAnnotation(boundNullTagBits);
173-
if (annotation == null) { // false alarm, implicit annotation is no conflict, but should be removed:
174-
// may not be reachable, how could we have an implicit @Nullable (not via @NonNullByDefault)?
175-
TypeBinding newBound = this.bound.withoutToplevelNullAnnotation();
176-
this.bound = newBound;
177-
wildcard.bound.resolvedType = newBound;
178-
} else {
179-
scope.problemReporter().contradictoryNullAnnotationsOnBounds(annotation, nullTagBits);
180-
}
165+
long boundNullTagBits = this.bound != null && this.bound.isValidBinding()
166+
? this.bound.tagBits & TagBits.AnnotationNullMASK
167+
: this.nullTagBitsFromErasedObjectBound;
168+
if (boundNullTagBits != 0L) {
169+
if (this.boundKind == Wildcard.SUPER) {
170+
if ((boundNullTagBits & TagBits.AnnotationNullable) != 0) {
171+
if (nullTagBits == 0L) {
172+
nullTagBits = TagBits.AnnotationNullable;
173+
} else if (wildcard != null && (nullTagBits & TagBits.AnnotationNonNull) != 0) {
174+
Annotation annotation = wildcard.bound.findAnnotation(boundNullTagBits);
175+
if (annotation == null) { // false alarm, implicit annotation is no conflict, but should be removed:
176+
// may not be reachable, how could we have an implicit @Nullable (not via @NonNullByDefault)?
177+
TypeBinding newBound = this.bound.withoutToplevelNullAnnotation();
178+
this.bound = newBound;
179+
wildcard.bound.resolvedType = newBound;
180+
} else {
181+
scope.problemReporter().contradictoryNullAnnotationsOnBounds(annotation, nullTagBits);
181182
}
182183
}
183-
} else {
184-
if ((boundNullTagBits & TagBits.AnnotationNonNull) != 0) {
185-
if (nullTagBits == 0L) {
186-
nullTagBits = TagBits.AnnotationNonNull;
187-
} else if (wildcard != null && (nullTagBits & TagBits.AnnotationNullable) != 0) {
188-
Annotation annotation = wildcard.bound.findAnnotation(boundNullTagBits);
189-
if (annotation == null) { // false alarm, implicit annotation is no conflict, but should be removed:
190-
TypeBinding newBound = this.bound.withoutToplevelNullAnnotation();
191-
this.bound = newBound;
192-
wildcard.bound.resolvedType = newBound;
193-
} else {
194-
scope.problemReporter().contradictoryNullAnnotationsOnBounds(annotation, nullTagBits);
195-
}
184+
}
185+
} else {
186+
if ((boundNullTagBits & TagBits.AnnotationNonNull) != 0) {
187+
if (nullTagBits == 0L) {
188+
nullTagBits = TagBits.AnnotationNonNull;
189+
} else if (wildcard != null && (nullTagBits & TagBits.AnnotationNullable) != 0) {
190+
Annotation annotation = wildcard.bound.findAnnotation(boundNullTagBits);
191+
if (annotation == null) { // false alarm, implicit annotation is no conflict, but should be removed:
192+
TypeBinding newBound = this.bound.withoutToplevelNullAnnotation();
193+
this.bound = newBound;
194+
wildcard.bound.resolvedType = newBound;
195+
} else {
196+
scope.problemReporter().contradictoryNullAnnotationsOnBounds(annotation, nullTagBits);
196197
}
197198
}
198-
if (nullTagBits == 0L && this.otherBounds != null) {
199-
for (TypeBinding otherBound : this.otherBounds) {
200-
if ((otherBound.tagBits & TagBits.AnnotationNonNull) != 0) { // can this happen?
201-
nullTagBits = TagBits.AnnotationNonNull;
202-
break;
203-
}
199+
}
200+
if (nullTagBits == 0L && this.otherBounds != null) {
201+
for (TypeBinding otherBound : this.otherBounds) {
202+
if ((otherBound.tagBits & TagBits.AnnotationNonNull) != 0) { // can this happen?
203+
nullTagBits = TagBits.AnnotationNonNull;
204+
break;
204205
}
205206
}
206207
}
@@ -300,7 +301,9 @@ public char[] constantPoolName() {
300301

301302
@Override
302303
public TypeBinding clone(TypeBinding immaterial) {
303-
return new WildcardBinding(this.genericType, this.rank, this.bound, this.otherBounds, this.boundKind, this.environment);
304+
WildcardBinding clone = new WildcardBinding(this.genericType, this.rank, this.bound, this.otherBounds, this.boundKind, this.environment);
305+
clone.nullTagBitsFromErasedObjectBound = this.nullTagBitsFromErasedObjectBound;
306+
return clone;
304307
}
305308

306309
@Override
@@ -881,4 +884,10 @@ TypeBinding propagateNonConflictingNullAnnotations(TypeBinding type) {
881884
return type;
882885
return this.environment.createAnnotatedType(type, annots);
883886
}
887+
888+
public boolean hasNullTagBits(long nullTagBits) {
889+
if (nullTagBits == this.nullTagBitsFromErasedObjectBound)
890+
return true;
891+
return (this.tagBits & TagBits.AnnotationNullMASK) == nullTagBits;
892+
}
884893
}

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
package org.eclipse.jdt.internal.compiler.problem;
8282

8383
import java.io.CharConversionException;
84+
import java.util.Arrays;
8485
import java.util.HashMap;
8586
import java.util.List;
8687
import java.util.Map;
@@ -10080,7 +10081,7 @@ public void anonymousDiamondWithNonDenotableTypeArguments(TypeReference type, Ty
1008010081
type.sourceStart,
1008110082
type.sourceEnd);
1008210083
}
10083-
public void redundantSpecificationOfTypeArguments(ASTNode location, TypeBinding[] argumentTypes) {
10084+
public void redundantSpecificationOfTypeArguments(TypeReference location, TypeBinding[] argumentTypes) {
1008410085
int severity = computeSeverity(IProblem.RedundantSpecificationOfTypeArguments);
1008510086
if (severity != ProblemSeverities.Ignore) {
1008610087
int sourceStart = -1;
@@ -10090,10 +10091,36 @@ public void redundantSpecificationOfTypeArguments(ASTNode location, TypeBinding[
1009010091
} else {
1009110092
sourceStart = location.sourceStart;
1009210093
}
10094+
String problemArguments;
10095+
String messageArguments;
10096+
class FindWildcard extends TypeBindingVisitor {
10097+
boolean found;
10098+
@Override
10099+
public boolean visit(WildcardBinding wildcardBinding) {
10100+
this.found = true;
10101+
return false;
10102+
}
10103+
}
10104+
FindWildcard find = new FindWildcard();
10105+
TypeBindingVisitor.visit(find, argumentTypes);
10106+
TypeReference[] typeArguments = null;
10107+
if (find.found) {
10108+
// when wildcards are in the mix then prefer showing type references, rather than processed bindings:
10109+
if (location instanceof ParameterizedSingleTypeReference pstr)
10110+
typeArguments = pstr.typeArguments;
10111+
else if (location instanceof ParameterizedQualifiedTypeReference pqtr)
10112+
typeArguments = pqtr.typeArguments[pqtr.typeArguments.length-1];
10113+
}
10114+
if (typeArguments != null) {
10115+
problemArguments = messageArguments = Arrays.stream(typeArguments).map(TypeReference::toString).collect(Collectors.joining(", ")); //$NON-NLS-1$
10116+
} else {
10117+
problemArguments = typesAsString(argumentTypes, false);
10118+
messageArguments = typesAsString(argumentTypes, true);
10119+
}
1009310120
this.handle(
1009410121
IProblem.RedundantSpecificationOfTypeArguments,
10095-
new String[] {typesAsString(argumentTypes, false)},
10096-
new String[] {typesAsString(argumentTypes, true)},
10122+
new String[] {problemArguments},
10123+
new String[] {messageArguments},
1009710124
severity,
1009810125
sourceStart,
1009910126
location.sourceEnd);

‎org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/parser/ComplianceDiagnoseTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2917,7 +2917,7 @@ public void testBug399778a() {
29172917
"1. WARNING in X.java (at line 4)\n" +
29182918
" List<String> l = (List<String>) (null == null ? Arrays.asList() : Arrays.asList(\"Hello\",\"world\"));\n" +
29192919
" ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
2920-
"Type safety: Unchecked cast from List<capture#1-of ? extends Object> to List<String>\n" +
2920+
"Type safety: Unchecked cast from List<capture#1-of ?> to List<String>\n" +
29212921
"----------\n";
29222922

29232923
runComplianceParserTest(

‎org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractRegressionTest.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -945,9 +945,6 @@ Excuse excuseFor(JavacCompiler compiler) {
945945
new EclipseHasABug(MismatchType.JavacErrorsEclipseWarnings),
946946
EclipseBug421922 = // https://bugs.eclipse.org/bugs/show_bug.cgi?id=421922
947947
new EclipseHasABug(MismatchType.EclipseErrorsJavacNone),
948-
EclipseBug428061 = // https://bugs.eclipse.org/bugs/show_bug.cgi?id=428061
949-
new EclipseHasABug(MismatchType.JavacErrorsEclipseNone |
950-
MismatchType.JavacErrorsEclipseWarnings),
951948
EclipseBug510528 = // https://bugs.eclipse.org/bugs/show_bug.cgi?id=510528
952949
new EclipseHasABug(MismatchType.JavacErrorsEclipseNone),
953950
EclipseBug531531 = // https://bugs.eclipse.org/bugs/show_bug.cgi?id=531531

0 commit comments

Comments
 (0)