Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler] Unnecessary cast warning is not detected when casting a bound generic type to its super type #2471

Merged
merged 2 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.eclipse.jdt.core.dom.IAnnotationBinding;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;

import com.sun.mirror.declaration.AnnotationMirror;
import com.sun.mirror.declaration.Modifier;
Expand Down Expand Up @@ -76,7 +75,7 @@ public Collection<Modifier> getModifiers()
else if( _astNode instanceof SingleVariableDeclaration )
modBits = ((SingleVariableDeclaration)_astNode).getModifiers();
else{
ASTNode parent = ((VariableDeclarationFragment)_astNode).getParent();
ASTNode parent = _astNode.getParent();
if( _astNode instanceof BodyDeclaration )
modBits = ((BodyDeclaration)parent).getModifiers();
}
Expand Down Expand Up @@ -148,7 +147,7 @@ private IAnnotationBinding[] getAnnotationInstancesFromAST()
extendsMods = ((SingleVariableDeclaration)_astNode).modifiers();
break;
case ASTNode.VARIABLE_DECLARATION_FRAGMENT:
final ASTNode parent = ((VariableDeclarationFragment)_astNode).getParent();
final ASTNode parent = _astNode.getParent();
if( parent instanceof BodyDeclaration )
extendsMods = ((BodyDeclaration)parent).modifiers();
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ private IAnnotationBinding[] getAnnotationInstances()
switch( binding.getKind() )
{
case IBinding.TYPE:
instances = ((ITypeBinding)binding).getAnnotations();
instances = binding.getAnnotations();
break;
case IBinding.METHOD:
instances = ((IMethodBinding)binding).getAnnotations();
instances = binding.getAnnotations();
break;
case IBinding.VARIABLE:
instances = ((IVariableBinding)binding).getAnnotations();
instances = binding.getAnnotations();
break;
case IBinding.PACKAGE:
// TODO: support package annotation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ private TestCodeUtil() {
}

public static boolean isTestCode(ICompilationUnit cu) {
IPackageFragmentRoot packageFragmentRoot = (IPackageFragmentRoot) ((IJavaElement) cu)
IPackageFragmentRoot packageFragmentRoot = (IPackageFragmentRoot) cu
.getAncestor(IJavaElement.PACKAGE_FRAGMENT_ROOT);
if (packageFragmentRoot != null) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public <R, P> R accept(ElementVisitor<R, P> visitor, P param) {
}
@Override
protected AnnotationBinding[] getAnnotationBindings() {
return ((ModuleBinding) this._binding).getAnnotations();
return this._binding.getAnnotations();
}

abstract class PackageDirectiveImpl {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
boolean analyseResources = currentScope.compilerOptions().analyseResourceLeaks;
boolean hasResourceWrapperType = analyseResources
&& this.resolvedType instanceof ReferenceBinding
&& ((ReferenceBinding)this.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable);
&& this.resolvedType.hasTypeBit(TypeIds.BitWrapperCloseable);
for (int i = 0, count = this.arguments.length; i < count; i++) {
Expression argument = this.arguments[i];
flowInfo =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,13 +430,13 @@ public static void analyseCloseableAllocation(BlockScope scope, FlowInfo flowInf
if (flowInfo.reachMode() != FlowInfo.REACHABLE)
return;
// client has checked that the resolvedType is an AutoCloseable, hence the following cast is safe:
if (((ReferenceBinding)allocation.resolvedType).hasTypeBit(TypeIds.BitResourceFreeCloseable)) {
if (allocation.resolvedType.hasTypeBit(TypeIds.BitResourceFreeCloseable)) {
// remove unnecessary attempts (closeable is not relevant)
if (allocation.closeTracker != null) {
allocation.closeTracker.withdraw();
allocation.closeTracker = null;
}
} else if (((ReferenceBinding)allocation.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable)) {
} else if (allocation.resolvedType.hasTypeBit(TypeIds.BitWrapperCloseable)) {
boolean isWrapper = true;
if (allocation.arguments != null && allocation.arguments.length > 0) {
// find the wrapped resource represented by its tracking var:
Expand Down Expand Up @@ -511,7 +511,7 @@ public static FlowInfo analyseCloseableAcquisition(BlockScope scope, FlowInfo fl
return flowInfo;
}
// client has checked that the resolvedType is an AutoCloseable, hence the following cast is safe:
if (((ReferenceBinding)acquisition.resolvedType).hasTypeBit(TypeIds.BitResourceFreeCloseable)
if (acquisition.resolvedType.hasTypeBit(TypeIds.BitResourceFreeCloseable)
&& !isBlacklistedMethod(acquisition)) {
// remove unnecessary attempts (closeable is not relevant)
if (acquisition.closeTracker != null) {
Expand Down Expand Up @@ -1093,7 +1093,7 @@ protected boolean handle(FakedTrackingVariable closeTracker, FlowInfo flow, ASTN
/** Answer wither the given type binding is a subtype of java.lang.AutoCloseable. */
public static boolean isAnyCloseable(TypeBinding typeBinding) {
return typeBinding instanceof ReferenceBinding
&& ((ReferenceBinding)typeBinding).hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable);
&& typeBinding.hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable);
}

/** Answer wither the given type binding is a subtype of java.lang.AutoCloseable. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,9 @@ public static TypeBinding getCollectionElementType(BlockScope scope, TypeBinding
if (collectionType.isArrayType()) { // for(E e : E[])
return ((ArrayBinding) collectionType).elementsType();
} else if (collectionType instanceof ReferenceBinding) {
ReferenceBinding iterableType = ((ReferenceBinding)collectionType).findSuperTypeOriginatingFrom(T_JavaLangIterable, false /*Iterable is not a class*/);
ReferenceBinding iterableType = collectionType.findSuperTypeOriginatingFrom(T_JavaLangIterable, false /*Iterable is not a class*/);
if (iterableType == null && isTargetJsr14) {
iterableType = ((ReferenceBinding)collectionType).findSuperTypeOriginatingFrom(T_JavaUtilCollection, false /*Iterable is not a class*/);
iterableType = collectionType.findSuperTypeOriginatingFrom(T_JavaUtilCollection, false /*Iterable is not a class*/);
}
if (iterableType == null) return null;

Expand Down Expand Up @@ -570,22 +570,22 @@ public void resolve(BlockScope upperScope) {
this.collection.computeConversion(this.scope, expectedCollectionType, collectionType);
}
} else if (collectionType instanceof ReferenceBinding) {
ReferenceBinding iterableType = ((ReferenceBinding)collectionType).findSuperTypeOriginatingFrom(T_JavaLangIterable, false /*Iterable is not a class*/);
ReferenceBinding iterableType = collectionType.findSuperTypeOriginatingFrom(T_JavaLangIterable, false /*Iterable is not a class*/);
if (iterableType == null && isTargetJsr14) {
iterableType = ((ReferenceBinding)collectionType).findSuperTypeOriginatingFrom(T_JavaUtilCollection, false /*Iterable is not a class*/);
iterableType = collectionType.findSuperTypeOriginatingFrom(T_JavaUtilCollection, false /*Iterable is not a class*/);
}
checkIterable: {
if (iterableType == null) break checkIterable;

this.iteratorReceiverType = collectionType.erasure();
if (isTargetJsr14) {
if (((ReferenceBinding)this.iteratorReceiverType).findSuperTypeOriginatingFrom(T_JavaUtilCollection, false) == null) {
if (this.iteratorReceiverType.findSuperTypeOriginatingFrom(T_JavaUtilCollection, false) == null) {
this.iteratorReceiverType = iterableType; // handle indirect inheritance thru variable secondary bound
this.collection.computeConversion(this.scope, iterableType, collectionType);
} else {
this.collection.computeConversion(this.scope, collectionType, collectionType);
}
} else if (((ReferenceBinding)this.iteratorReceiverType).findSuperTypeOriginatingFrom(T_JavaLangIterable, false) == null) {
} else if (this.iteratorReceiverType.findSuperTypeOriginatingFrom(T_JavaLangIterable, false) == null) {
this.iteratorReceiverType = iterableType; // handle indirect inheritance thru variable secondary bound
this.collection.computeConversion(this.scope, iterableType, collectionType);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
import org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment;
import org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
import org.eclipse.jdt.internal.compiler.lookup.MethodScope;
import org.eclipse.jdt.internal.compiler.lookup.MethodVerifier;
import org.eclipse.jdt.internal.compiler.lookup.MissingTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.ParameterizedGenericMethodBinding;
import org.eclipse.jdt.internal.compiler.lookup.ParameterizedMethodBinding;
Expand Down Expand Up @@ -807,25 +808,14 @@ public TypeBinding resolveType(BlockScope scope) {
if (this.constant != Constant.NotAConstant) {
this.constant = Constant.NotAConstant;
long sourceLevel = scope.compilerOptions().sourceLevel;
boolean receiverCast = false;
if (this.receiver instanceof CastExpression) {
this.receiver.bits |= ASTNode.DisableUnnecessaryCastCheck; // will check later on
receiverCast = true;
}
this.actualReceiverType = this.receiver.resolveType(scope);
if (this.actualReceiverType instanceof InferenceVariable) {
return null; // not yet ready for resolving
}
this.receiverIsType = this.receiver.isType();
if (receiverCast && this.actualReceiverType != null) {
// due to change of declaring class with receiver type, only identity cast should be notified
TypeBinding resolvedType2 = ((CastExpression)this.receiver).expression.resolvedType;
if (TypeBinding.equalsEquals(resolvedType2, this.actualReceiverType)) {
if (!scope.environment().usesNullTypeAnnotations() || !NullAnnotationMatching.analyse(this.actualReceiverType, resolvedType2, -1).isAnyMismatch()) {
scope.problemReporter().unnecessaryCast((CastExpression) this.receiver);
}
}
}
// resolve type arguments (for generic constructor call)
if (this.typeArguments != null) {
int length = this.typeArguments.length;
Expand Down Expand Up @@ -977,6 +967,12 @@ public TypeBinding resolveType(BlockScope scope) {
return null;
}

if (this.receiver instanceof CastExpression castedRecevier) {
// this check was suppressed while resolving receiver, check now based on the resolved method
if (isUnnecessaryReceiverCast(scope, castedRecevier.expression.resolvedType))
scope.problemReporter().unnecessaryCast(castedRecevier);
}

if (compilerOptions.isAnnotationBasedNullAnalysisEnabled) {
ImplicitNullAnnotationVerifier.ensureNullnessIsKnown(this.binding, scope);
if (compilerOptions.sourceLevel >= ClassFileConstants.JDK1_8) {
Expand Down Expand Up @@ -1096,6 +1092,26 @@ public TypeBinding resolveType(BlockScope scope) {
: null;
}

protected boolean isUnnecessaryReceiverCast(BlockScope scope, TypeBinding uncastedReceiverType) {
if (uncastedReceiverType == null || !uncastedReceiverType.isCompatibleWith(this.binding.declaringClass)) {
return false;
}
if (uncastedReceiverType.isRawType() && this.binding.declaringClass.isParameterizedType()) {
return false;
}
MethodBinding otherMethod = scope.getMethod(uncastedReceiverType, this.selector, this.argumentTypes, this);
if (!otherMethod.isValidBinding()) {
return false;
}
if (scope.environment().usesNullTypeAnnotations()
&& NullAnnotationMatching.analyse(this.actualReceiverType, uncastedReceiverType, -1).isAnyMismatch()) {
return false;
}
return otherMethod == this.binding
|| MethodVerifier.doesMethodOverride(this.binding, otherMethod, scope.environment())
|| MethodVerifier.doesMethodOverride(otherMethod, this.binding, scope.environment());
}

protected TypeBinding handleNullnessCodePatterns(BlockScope scope, TypeBinding returnType) {
// j.u.s.Stream.filter() may modify nullness of stream elements:
if (this.binding.isWellknownMethod(TypeConstants.JAVA_UTIL_STREAM__STREAM, TypeConstants.FILTER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
boolean analyseResources = currentScope.compilerOptions().analyseResourceLeaks;
boolean hasResourceWrapperType = analyseResources
&& this.resolvedType instanceof ReferenceBinding
&& ((ReferenceBinding)this.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable);
&& this.resolvedType.hasTypeBit(TypeIds.BitWrapperCloseable);
for (int i = 0, count = this.arguments.length; i < count; i++) {
flowInfo = this.arguments[i].analyseCode(currentScope, flowContext, flowInfo);
if (analyseResources && !hasResourceWrapperType) { // allocation of wrapped closeables is analyzed specially
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public TypeBinding resolveTypeEnclosing(BlockScope scope, ReferenceBinding enclo
if (!memberType.isValidBinding()) {
hasError = true;
scope.problemReporter().invalidEnclosingType(this, memberType, enclosingType);
memberType = ((ReferenceBinding)memberType).closestMatch();
memberType = memberType.closestMatch();
if (memberType == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public void addPattern(RecordPattern rp, int i) {
TypeBinding ref = SwitchStatement.this.expression.resolvedType;
if (!(ref instanceof ReferenceBinding))
return;
RecordComponentBinding[] comps = ((ReferenceBinding)ref).components();
RecordComponentBinding[] comps = ref.components();
if (comps == null || comps.length <= i) // safety-net for incorrect code.
return;
if (this.next == null)
Expand Down Expand Up @@ -425,12 +425,12 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
this.scope.enclosingCase = this.cases[caseIndex]; // record entering in a switch case block
caseIndex++;
if (prevCaseStmtIndex == i - 1) {
if (((CaseStatement) this.statements[prevCaseStmtIndex]).containsPatternVariable())
if (this.statements[prevCaseStmtIndex].containsPatternVariable())
this.scope.problemReporter().illegalFallthroughFromAPattern(this.statements[prevCaseStmtIndex]);
}
prevCaseStmtIndex = i;
if (fallThroughState == FALLTHROUGH && complaintLevel <= NOT_COMPLAINED) {
if (((CaseStatement) statement).containsPatternVariable())
if (statement.containsPatternVariable())
this.scope.problemReporter().IllegalFallThroughToPattern(this.scope.enclosingCase);
else if ((statement.bits & ASTNode.DocumentedFallthrough) == 0) { // the case is not fall-through protected by a line comment
this.scope.problemReporter().possibleFallThroughCase(this.scope.enclosingCase);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ public static final Constant computeConstantOperationEQUAL_EQUAL(Constant left,
if (rightId == T_JavaLangString) {
//String are interned in th compiler==>thus if two string constant
//get to be compared, it is an equal on the vale which is done
return BooleanConstant.fromValue(((StringConstant)left).hasSameValue(right));
return BooleanConstant.fromValue(left.hasSameValue(right));
}
break;
case T_null :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ public Object[] getEmulationPath(ReferenceBinding targetEnclosingType, boolean o
if (enclosingArgument != null) {
FieldBinding syntheticField = sourceType.getSyntheticField(enclosingArgument);
if (syntheticField != null) {
if (TypeBinding.equalsEquals(syntheticField.type, targetEnclosingType) || (!onlyExactMatch && ((ReferenceBinding)syntheticField.type).findSuperTypeOriginatingFrom(targetEnclosingType) != null))
if (TypeBinding.equalsEquals(syntheticField.type, targetEnclosingType) || (!onlyExactMatch && syntheticField.type.findSuperTypeOriginatingFrom(targetEnclosingType) != null))
return new Object[] { syntheticField };
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1178,8 +1178,8 @@ public String toString() {
} else {
buf.append('\n');
for (Entry<ParameterizedTypeBinding, ParameterizedTypeBinding> entry : this.captures.entrySet()) {
String lhs = String.valueOf(((TypeBinding)entry.getKey()).shortReadableName());
String rhs = String.valueOf(((TypeBinding)entry.getValue()).shortReadableName());
String lhs = String.valueOf(entry.getKey().shortReadableName());
String rhs = String.valueOf(entry.getValue().shortReadableName());
buf.append('\t').append(lhs).append(" = capt(").append(rhs).append(")\n"); //$NON-NLS-1$ //$NON-NLS-2$
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public boolean isSubtypeOf(TypeBinding other, boolean simulatingBugJDK8026527) {
if (TypeBinding.equalsEquals(this, other))
return true;
if (other instanceof ReferenceBinding) {
TypeBinding[] rightIntersectingTypes = ((ReferenceBinding) other).getIntersectingTypes();
TypeBinding[] rightIntersectingTypes = other.getIntersectingTypes();
if (rightIntersectingTypes != null && rightIntersectingTypes.length > 1) {
int numRequired = rightIntersectingTypes.length;
TypeBinding[] required = new TypeBinding[numRequired];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ public boolean hasMemberTypes() {
public boolean hasTypeBit(int bit) {
TypeBinding erasure = erasure();
if (erasure instanceof ReferenceBinding)
return ((ReferenceBinding) erasure).hasTypeBit(bit);
return erasure.hasTypeBit(bit);
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1599,7 +1599,7 @@ protected boolean isSubTypeOfRTL(TypeBinding other) {
return (lower != null && isSubtypeOf(lower, false));
}
if (other instanceof ReferenceBinding) {
TypeBinding[] intersecting = ((ReferenceBinding) other).getIntersectingTypes();
TypeBinding[] intersecting = other.getIntersectingTypes();
if (intersecting != null) {
for (TypeBinding binding : intersecting) {
if (!isSubtypeOf(binding, false))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1656,7 +1656,7 @@ public MethodBinding findMethod(ReferenceBinding receiverType, char[] selector,
if (method != null && method.isValidBinding() && method.isVarargs()) {
TypeBinding elementType = method.parameters[method.parameters.length - 1].leafComponentType();
if (elementType instanceof ReferenceBinding) {
if (!((ReferenceBinding) elementType).erasure().canBeSeenBy(this)) {
if (!elementType.erasure().canBeSeenBy(this)) {
return new ProblemMethodBinding(method, method.selector, invocationSite.genericTypeArguments(), ProblemReasons.VarargsElementTypeNotVisible);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2444,6 +2444,7 @@ public void test041() {
"public abstract class X extends J implements I {\n" +
" void bar() {\n" +
" String s = ((I) this).foo(0.0f);\n" +
" s = this.foo(0.0f);\n" + // without cast a different overload is selected, returning String
" }\n" +
"}"
},
Expand Down
Loading
Loading