diff --git a/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java b/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java index 85a5f6fafed5..b064ea1022c4 100644 --- a/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java +++ b/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanVisitor.java @@ -117,7 +117,8 @@ protected boolean commonAssignmentCheck( } @Override - protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + protected TypecastKind isTypeCastSafe( + AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { AnnotationMirror exprLTAnno = exprType.getEffectiveAnnotationInHierarchy(atypeFactory.LESS_THAN_UNKNOWN); diff --git a/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java b/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java index eff70f42a864..c7e9bf19245c 100644 --- a/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java +++ b/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java @@ -984,9 +984,10 @@ private boolean overrides(ExecutableElement e, Class clazz, String method) { } @Override - protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + protected TypecastKind isTypeCastSafe( + AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { if (castType.getKind().isPrimitive()) { - return true; + return TypecastKind.SAFE; } return super.isTypeCastSafe(castType, exprType); } diff --git a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java index dc0f41b66241..2ad4f054a344 100644 --- a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java +++ b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java @@ -379,10 +379,11 @@ public Void visitCompoundAssignment(CompoundAssignmentTree tree, Void p) { } @Override - protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + protected TypecastKind isTypeCastSafe( + AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { if (!atypeFactory.maybeIntegral(castType)) { // If the cast is not a number or a char, then it is legal. - return true; + return TypecastKind.SAFE; } return super.isTypeCastSafe(castType, exprType); } diff --git a/checker/tests/initialization/CastInit.java b/checker/tests/initialization/CastInit.java index 631b7da1bc43..2f0040fac054 100644 --- a/checker/tests/initialization/CastInit.java +++ b/checker/tests/initialization/CastInit.java @@ -5,7 +5,7 @@ public class CastInit { public CastInit() { @UnknownInitialization CastInit t1 = (@UnknownInitialization CastInit) this; - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) @Initialized CastInit t2 = (@Initialized CastInit) this; } } diff --git a/checker/tests/lock/ChapterExamples.java b/checker/tests/lock/ChapterExamples.java index f06c1797da68..e9642e53d684 100644 --- a/checker/tests/lock/ChapterExamples.java +++ b/checker/tests/lock/ChapterExamples.java @@ -319,7 +319,7 @@ void someMethod() { o2 = o1; // {"lock"} and {} are not identical sets. } - @SuppressWarnings("lock:cast.unsafe") + @SuppressWarnings("lock:cast.incomparable") void someMethod2() { // A cast can be used if the user knows it is safe to do so. // However, the @SuppressWarnings must be added. @@ -567,7 +567,7 @@ public boolean compare(T[] a1, T[] a2) { private static final Object NULL_KEY = new Object(); // A guardsatisfied.location.disallowed error is issued for the cast. - @SuppressWarnings({"cast.unsafe", "guardsatisfied.location.disallowed"}) + @SuppressWarnings({"cast.incomparable", "guardsatisfied.location.disallowed"}) private static @GuardSatisfied(1) Object maskNull(@GuardSatisfied(1) Object key) { return (key == null ? (@GuardSatisfied(1) Object) NULL_KEY : key); } diff --git a/checker/tests/lock/Strings.java b/checker/tests/lock/Strings.java index c29200e1d47f..b9c4d8f15412 100644 --- a/checker/tests/lock/Strings.java +++ b/checker/tests/lock/Strings.java @@ -13,7 +13,9 @@ void StringIsGBnothing( @GuardSatisfied Object o3, @GuardedByBottom Object o4) { String s1 = (String) o1; + // :: error: (cast.incomparable) String s2 = (String) o2; + // :: error: (cast.incomparable) String s3 = (String) o3; String s4 = (String) o4; // OK } diff --git a/checker/tests/signedness/CastFromTtoT.java b/checker/tests/signedness/CastFromTtoT.java new file mode 100644 index 000000000000..45f535c98467 --- /dev/null +++ b/checker/tests/signedness/CastFromTtoT.java @@ -0,0 +1,41 @@ +import org.checkerframework.checker.signedness.qual.Signed; +import org.checkerframework.checker.signedness.qual.SignednessGlb; +import org.checkerframework.checker.signedness.qual.UnknownSignedness; + +class CastFromTtoT { + @SuppressWarnings("unchecked") + T bar(@UnknownSignedness Object p) { + // Seems to have no cast in terms of the qualifier (from @UnknownSignedness to + // @UnknownSignedness), but in instantiation, it could be a downcast. + // See method foo below. It's okay not to report downcast warnings as Javac will warn about + // casting object to 'T' (unchecked warning) + T x = (T) p; + return x; + } + + void foo(CastFromTtoT<@Signed Integer> s, @UnknownSignedness Object local) { + // Here, we passed in an @UnknownSignedness object and the method signature after + // substitution is @Signed Integer bar(@UnknownSignedness Object). This makes the typecast + // discussed earlier a downcast. + @Signed Integer x = s.bar(local); + } + + class Inner { + T bar2(@Signed T p) { + // The casting expression below looks like an upcast (in terms of the qualifier), + // but it could be a downcast in invocation (See method foo2 below for an example). + // We should report downcast warning if there is one because + // Javac doesn't warn when casting a variable from type T to T. + // :: warning: (cast.unsafe) + T x = (T) p; + return x; + } + + void foo2(Inner<@SignednessGlb Integer> s, @Signed Integer local) { + // Here, we passed in an @Signed integer and the method signature after + // substitution is @SignednessGlb Integer bar2(@Signed Object). This makes the typecast + // discussed in method bar2 a downcast. + @SignednessGlb Integer x = s.bar2(local); + } + } +} diff --git a/checker/tests/signedness/LiteralCast.java b/checker/tests/signedness/LiteralCast.java index c1ec2552c684..c069536430af 100644 --- a/checker/tests/signedness/LiteralCast.java +++ b/checker/tests/signedness/LiteralCast.java @@ -24,7 +24,7 @@ void m() { requireSigned((@Unsigned int) 2); requireSigned((int) 2); requireSigned((@m int) 2); - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) requireSigned((@Signed int) u); // :: error: (argument.type.incompatible) requireSigned((@Unsigned int) u); @@ -33,7 +33,7 @@ void m() { // :: error: (argument.type.incompatible) requireSigned((@m int) u); requireSigned((@Signed int) s); - // :: error: (argument.type.incompatible) :: warning: (cast.unsafe) + // :: error: (argument.type.incompatible) :: error: (cast.incomparable) requireSigned((@Unsigned int) s); requireSigned((int) s); requireSigned((@m int) s); @@ -43,14 +43,14 @@ void m() { requireUnsigned((@Unsigned int) 2); requireUnsigned((int) 2); requireUnsigned((@m int) 2); - // :: error: (argument.type.incompatible) :: warning: (cast.unsafe) + // :: error: (argument.type.incompatible) :: error: (cast.incomparable) requireUnsigned((@Signed int) u); requireUnsigned((@Unsigned int) u); requireUnsigned((int) u); requireUnsigned((@m int) u); // :: error: (argument.type.incompatible) requireUnsigned((@Signed int) s); - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) requireUnsigned((@Unsigned int) s); // :: error: (argument.type.incompatible) requireUnsigned((int) s); diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 43c0d06238a0..ff2b41c30359 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -3,11 +3,18 @@ Version 3.42.0-eisop6 (January ??, 2025) **User-visible changes:** +A new error message `cast.incomparable` will be raised if the target type qualifier is neither the subtype +nor the supertype of the expression type qualifier. No longer issue errors for statically verifiable downcast. + **Implementation details:** +Refactored the implementation of `isTypeCastSafe` to categorize the kinds of a typecast, whether +it is an upcast, downcast or incomparable cast. Based on that, further determine if the typecast +is statically verifiable or not. + **Closed issues:** -eisop#1003, eisop#1033. +eisop#155, eisop#1003, eisop#1033. Version 3.42.0-eisop5 (December 20, 2024) @@ -188,7 +195,6 @@ is an enhanced switch statement. eisop#609, eisop#610, eisop#612. - Version 3.40.0 (November 1, 2023) --------------------------------- diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 6cb489b93f68..3995a1f6592f 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -152,6 +152,7 @@ import javax.lang.model.type.DeclaredType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; +import javax.lang.model.type.TypeVariable; import javax.lang.model.util.ElementFilter; /* NO-AFU @@ -2678,40 +2679,78 @@ protected void checkTypecastSafety(TypeCastTree typeCastTree) { } reported = true; // don't issue cast unsafe warning. } - // Don't call TypeHierarchy#isSubtype(exprType, castType) because the underlying Java types // will not be in the correct subtyping relationship if this is a downcast. - if (!reported && !isTypeCastSafe(castType, exprType)) { - checker.reportWarning( - typeCastTree, "cast.unsafe", exprType.toString(true), castType.toString(true)); + if (!reported) { + TypecastKind castResult = isTypeCastSafe(castType, exprType); + + if (castResult == TypecastKind.WARNING) { + checker.reportWarning( + typeCastTree, + "cast.unsafe", + exprType.toString(true), + castType.toString(true)); + } else if (castResult == TypecastKind.ERROR) { + checker.reportError( + typeCastTree, + "cast.incomparable", + exprType.toString(true), + castType.toString(true)); + } } } /** - * Returns true if the cast is safe. - * - *

Only primary qualifiers are checked unless the command line option "checkCastElementType" - * is supplied. + * This method returns TypecastKind.SAFE when the typecast is an upcast or a statically + * verifiable downcast. Returns TypecastKind.WARNING and ERROR for those downcasts which cannot + * be statically verified or some incomparable casts. * * @param castType annotated type of the cast * @param exprType annotated type of the casted expression - * @return true if the type cast is safe, false otherwise + * @return TypecastKind.SAFE if the typecast is safe, error or warning otherwise */ - protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { - TypeKind castTypeKind = castType.getKind(); - if (castTypeKind == TypeKind.DECLARED) { - // Don't issue an error if the annotations are equivalent to the qualifier upper bound - // of the type. - AnnotatedDeclaredType castDeclared = (AnnotatedDeclaredType) castType; - AnnotationMirrorSet bounds = - atypeFactory.getTypeDeclarationBounds(castDeclared.getUnderlyingType()); + protected TypecastKind isTypeCastSafe( + AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + TypecastKind castResult = isUpcast(castType, exprType); - if (AnnotationUtils.areSame(castDeclared.getAnnotations(), bounds)) { - return true; + // not upcast, do downcast and incomparable cast check + if (castResult == TypecastKind.NOT_UPCAST) { + castResult = isSafeDowncast(castType, exprType); + if (castResult == TypecastKind.NOT_DOWNCAST) { // fall into incomparable cast + return isSafeIncomparableCast(castType, exprType); } } + return castResult; + } + + /** Represents all possible kinds of typecasts. */ + protected enum TypecastKind { + /** The cast is safe. */ + SAFE, + /** The cast is illegal. */ + ERROR, + /** Cannot statically verify the cast, report a warning. */ + WARNING, + /** It is not an upcast. */ + NOT_UPCAST, + /** It is not a downcast. */ + NOT_DOWNCAST + } + /** + * Return SAFE if the typecast is an upcast (from the view of the qualifiers). + * + *

Only primary qualifiers are checked unless the command line option "checkCastElementType" + * is supplied. + * + * @param castType annotated type of the cast + * @param exprType annotated type of the casted expression + * @return return NOT_UPCAST, WARNING or SAFE TypecastKind. + */ + protected TypecastKind isUpcast(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + QualifierHierarchy qualifierHierarchy = atypeFactory.getQualifierHierarchy(); AnnotationMirrorSet castAnnos; + TypeKind castTypeKind = castType.getKind(); AnnotatedTypeMirror newCastType; TypeMirror newCastTM; if (!checkCastElementType) { @@ -2735,13 +2774,13 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr TypeMirror newExprTM = newExprType.getUnderlyingType(); if (!typeHierarchy.isSubtype(newExprType, newCastType)) { - return false; + return TypecastKind.WARNING; } if (newCastType.getKind() == TypeKind.ARRAY && newExprType.getKind() != TypeKind.ARRAY) { // Always warn if the cast contains an array, but the expression // doesn't, as in "(Object[]) o" where o is of type Object - return false; + return TypecastKind.WARNING; } else if (newCastType.getKind() == TypeKind.DECLARED && newExprType.getKind() == TypeKind.DECLARED) { int castSize = ((AnnotatedDeclaredType) newCastType).getTypeArguments().size(); @@ -2751,7 +2790,7 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr // Always warn if the cast and expression contain a different number of type // arguments, e.g. to catch a cast from "Object" to "List<@NonNull Object>". // TODO: the same number of arguments actually doesn't guarantee anything. - return false; + return TypecastKind.WARNING; } } else if (castTypeKind == TypeKind.TYPEVAR && exprType.getKind() == TypeKind.TYPEVAR) { // If both the cast type and the casted expression are type variables, then check @@ -2760,12 +2799,14 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr AnnotatedTypes.findEffectiveLowerBoundAnnotations(qualHierarchy, castType); AnnotationMirrorSet lowerBoundAnnotationsExpr = AnnotatedTypes.findEffectiveLowerBoundAnnotations(qualHierarchy, exprType); - return qualHierarchy.isSubtypeShallow( - lowerBoundAnnotationsExpr, - newExprTM, - lowerBoundAnnotationsCast, - newCastTM) - && typeHierarchy.isSubtypeShallowEffective(exprType, castType); + boolean result = + qualHierarchy.isSubtypeShallow( + lowerBoundAnnotationsExpr, + newExprTM, + lowerBoundAnnotationsCast, + newCastTM) + && typeHierarchy.isSubtypeShallowEffective(exprType, castType); + return result ? TypecastKind.SAFE : TypecastKind.WARNING; } if (castTypeKind == TypeKind.TYPEVAR) { // If the cast type is a type var, but the expression is not, then check that the @@ -2777,12 +2818,103 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr } } + // Check when casting the expression having type T (a type variable) to type T. + // As the compiler will not report the unchecked warning in this case, so we should + // do the check for our type system when the subtype relation of the instantiations of the + // two T cannot be statically verified. + // See CastFromTtoT.java for an example. + if (castTypeKind == TypeKind.TYPEVAR && exprType.getKind() == TypeKind.TYPEVAR) { + TypeVariable castTV = (TypeVariable) castType.getUnderlyingType(); + TypeVariable exprTV = (TypeVariable) exprType.getUnderlyingType(); + if (TypesUtils.areSameTypeVariables(castTV, exprTV)) { + AnnotationMirrorSet castLower = + AnnotatedTypes.findEffectiveLowerBoundAnnotations( + qualifierHierarchy, castType); + AnnotationMirrorSet exprLower = + AnnotatedTypes.findEffectiveLowerBoundAnnotations( + qualifierHierarchy, exprType); + AnnotationMirrorSet castUpper = + AnnotatedTypes.findEffectiveAnnotations(qualifierHierarchy, castType); + AnnotationMirrorSet exprUpper = + AnnotatedTypes.findEffectiveAnnotations(qualifierHierarchy, exprType); + + // return SAFE only it satisfies the below condition + if (qualifierHierarchy.isSubtypeShallow(exprLower, exprTV, castLower, castTV) + && qualifierHierarchy.isSubtypeShallow( + exprUpper, exprTV, castUpper, castTV)) { + return TypecastKind.SAFE; + } + return TypecastKind.WARNING; + } + } + AnnotatedTypeMirror exprTypeWidened = atypeFactory.getWidenedType(exprType, castType); - return qualHierarchy.isSubtypeShallow( - exprTypeWidened.getEffectiveAnnotations(), - exprTypeWidened.getUnderlyingType(), + boolean result = + qualHierarchy.isSubtypeShallow( + exprTypeWidened.getEffectiveAnnotations(), + exprTypeWidened.getUnderlyingType(), + castAnnos, + newCastTM); + if (result) { + return TypecastKind.SAFE; + } else if (checkCastElementType) { + // when the flag is enabled, and it is not an upcast, return a warning + return TypecastKind.WARNING; + } else { + return TypecastKind.NOT_UPCAST; + } + } + + /** + * Determine the typecast is downcast or not. If it is, further determine it can be statically + * verified or not. + * + * @param castType annotated type of the cast + * @param exprType annotated type of the casted expression + * @return Can return SAFE, NOT_DOWNCAST or WARNING. + */ + protected TypecastKind isSafeDowncast( + AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + AnnotationMirrorSet castAnnos = castType.getEffectiveAnnotations(); + AnnotationMirrorSet exprAnnos = exprType.getEffectiveAnnotations(); + QualifierHierarchy qualifierHierarchy = atypeFactory.getQualifierHierarchy(); + + if (!qualifierHierarchy.isComparable( castAnnos, - newCastTM); + castType.getUnderlyingType(), + exprAnnos, + exprType.getUnderlyingType())) { // exists an incomparable cast + return TypecastKind.NOT_DOWNCAST; + } + + // check if the downcast can be statically verified + final TypeKind castTypeKind = castType.getKind(); + + if (castTypeKind == TypeKind.DECLARED) { + // Don't issue an error if the annotations are equivalent to the qualifier upper bound + // of the type. + AnnotatedDeclaredType castDeclared = (AnnotatedDeclaredType) castType; + AnnotationMirrorSet bounds = + atypeFactory.getTypeDeclarationBounds(castDeclared.getUnderlyingType()); + + if (AnnotationUtils.areSame(castDeclared.getAnnotations(), bounds)) { + return TypecastKind.SAFE; + } + } + return TypecastKind.WARNING; + } + + /** + * If it is an incomparable cast in terms of qualifiers, return ERROR. Subchecker can override + * this method to allow certain incomparable casts and can return SAFE, WARNING or ERROR. + * + * @param castType annotated type of the cast + * @param exprType annotated type of the casted expression + * @return TypecastKind.ERROR. + */ + protected TypecastKind isSafeIncomparableCast( + AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + return TypecastKind.ERROR; } /** @@ -2801,7 +2933,7 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr */ private boolean isTypeCastSafeInvariant( AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType, AnnotationMirror top) { - if (!isTypeCastSafe(castType, exprType)) { + if (isTypeCastSafe(castType, exprType) != TypecastKind.SAFE) { return false; } @@ -2854,7 +2986,7 @@ public Void visitInstanceOf(InstanceOfTree tree, Void p) { AnnotatedTypeMirror variableType = atypeFactory.getAnnotatedType(variableTree); AnnotatedTypeMirror expType = atypeFactory.getAnnotatedType(tree.getExpression()); - if (!isTypeCastSafe(variableType, expType)) { + if (isTypeCastSafe(variableType, expType) != TypecastKind.SAFE) { checker.reportWarning( tree, "instanceof.pattern.unsafe", expType, variableTree); } diff --git a/framework/src/main/java/org/checkerframework/common/basetype/messages.properties b/framework/src/main/java/org/checkerframework/common/basetype/messages.properties index 87fcc85c9acd..25a1317517a8 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/messages.properties +++ b/framework/src/main/java/org/checkerframework/common/basetype/messages.properties @@ -29,6 +29,7 @@ type.invalid.annotations.on.use=invalid type: annotations %s conflict with decla type.invalid.annotations.on.location=annotation %s used on prohibited locations %s type.invalid.super.wildcard=bounds must have the same annotations.%nextends bound: %s%nsuper bound : %s cast.unsafe=cast from "%s" to "%s" cannot be statically verified +cast.incomparable=illegal typecast from "%s" to "%s" invariant.cast.unsafe=cannot cast from "%s" to "%s" cast.unsafe.constructor.invocation=constructor invocation cast from "%s" to "%s" cannot be statically verified exception.parameter.invalid=invalid type in exception parameter.%nfound : %s%nrequired: %s diff --git a/framework/src/main/java/org/checkerframework/common/value/ValueVisitor.java b/framework/src/main/java/org/checkerframework/common/value/ValueVisitor.java index 20833b218f72..76e26b352337 100644 --- a/framework/src/main/java/org/checkerframework/common/value/ValueVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/value/ValueVisitor.java @@ -342,7 +342,8 @@ public Void visitTypeCast(TypeCastTree tree, Void p) { // This method returns true for (@IntVal(-1), @IntVal(255)) if the underlying type is `byte`, // but not for any other underlying type. @Override - protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { + protected TypecastKind isTypeCastSafe( + AnnotatedTypeMirror castType, AnnotatedTypeMirror exprType) { TypeKind castTypeKind = TypeKindUtils.primitiveOrBoxedToTypeKind(castType.getUnderlyingType()); TypeKind exprTypeKind = @@ -354,7 +355,7 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr AnnotationMirrorSet castAnnos = castType.getAnnotations(); AnnotationMirrorSet exprAnnos = exprType.getAnnotations(); if (castAnnos.equals(exprAnnos)) { - return true; + return TypecastKind.SAFE; } assert castAnnos.size() == 1; assert exprAnnos.size() == 1; @@ -369,13 +370,21 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr // Special-case singleton sets for speed. switch (castTypeKind) { case BYTE: - return castValues.get(0).byteValue() == exprValues.get(0).byteValue(); + return castValues.get(0).byteValue() == exprValues.get(0).byteValue() + ? TypecastKind.SAFE + : TypecastKind.ERROR; case INT: - return castValues.get(0).intValue() == exprValues.get(0).intValue(); + return castValues.get(0).intValue() == exprValues.get(0).intValue() + ? TypecastKind.SAFE + : TypecastKind.ERROR; case SHORT: - return castValues.get(0).shortValue() == exprValues.get(0).shortValue(); + return castValues.get(0).shortValue() == exprValues.get(0).shortValue() + ? TypecastKind.SAFE + : TypecastKind.ERROR; default: - return castValues.get(0).longValue() == exprValues.get(0).longValue(); + return castValues.get(0).longValue() == exprValues.get(0).longValue() + ? TypecastKind.SAFE + : TypecastKind.ERROR; } } else { switch (castTypeKind) { @@ -390,7 +399,9 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr CollectionsPlume.mapList( Number::byteValue, exprValues)); return CollectionsPlume.sortedSetContainsAll( - castValuesTree, exprValuesTree); + castValuesTree, exprValuesTree) + ? TypecastKind.SAFE + : TypecastKind.ERROR; } case INT: { @@ -403,7 +414,9 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr CollectionsPlume.mapList( Number::intValue, exprValues)); return CollectionsPlume.sortedSetContainsAll( - castValuesTree, exprValuesTree); + castValuesTree, exprValuesTree) + ? TypecastKind.SAFE + : TypecastKind.ERROR; } case SHORT: { @@ -416,14 +429,18 @@ protected boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirr CollectionsPlume.mapList( Number::shortValue, exprValues)); return CollectionsPlume.sortedSetContainsAll( - castValuesTree, exprValuesTree); + castValuesTree, exprValuesTree) + ? TypecastKind.SAFE + : TypecastKind.ERROR; } default: { TreeSet castValuesTree = new TreeSet<>(castValues); TreeSet exprValuesTree = new TreeSet<>(exprValues); return CollectionsPlume.sortedSetContainsAll( - castValuesTree, exprValuesTree); + castValuesTree, exprValuesTree) + ? TypecastKind.SAFE + : TypecastKind.ERROR; } } } diff --git a/framework/src/main/java/org/checkerframework/framework/type/QualifierHierarchy.java b/framework/src/main/java/org/checkerframework/framework/type/QualifierHierarchy.java index b53fb03b5870..dcfc88ba6fa8 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/QualifierHierarchy.java +++ b/framework/src/main/java/org/checkerframework/framework/type/QualifierHierarchy.java @@ -303,6 +303,26 @@ public final boolean isSubtypeShallow( return isSubtypeShallow(subQualifiers, typeMirror, superQualifiers, typeMirror); } + /** + * Tests whether all qualifiers in {@code qualifiers1} are comparable with the qualifier in the + * same hierarchy in {@code qualifiers2}. + * + * @param qualifiers1 set of qualifiers; exactly one per hierarchy + * @param type1 the type associated with {@code qualifiers1} + * @param qualifiers2 set of qualifiers; exactly one per hierarchy + * @param type2 the type associated with {@code qualifiers2} + * @return true iff all qualifiers in {@code qualifiers1} are comparable with qualifiers in + * {@code qualifiers2} + */ + public boolean isComparable( + Collection qualifiers1, + TypeMirror type1, + Collection qualifiers2, + TypeMirror type2) { + return isSubtypeShallow(qualifiers1, type1, qualifiers2, type2) + || isSubtypeShallow(qualifiers2, type2, qualifiers1, type1); + } + /** * Returns the least upper bound (LUB) of the qualifiers {@code qualifier1} and {@code * qualifier2}. Returns {@code null} if the qualifiers are not from the same qualifier diff --git a/framework/tests/value/Basics.java b/framework/tests/value/Basics.java index 05787674e976..8b674e9bd9ae 100644 --- a/framework/tests/value/Basics.java +++ b/framework/tests/value/Basics.java @@ -168,9 +168,9 @@ public void intCastTest(@IntVal({0, 1}) int input) { @IntVal({0, 1}) int c = (int) input; @IntVal({0, 1}) int ac = (@IntVal({0, 1}) int) input; @IntVal({0, 1, 2}) int sc = (@IntVal({0, 1, 2}) int) input; - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) @IntVal({1}) int uc = (@IntVal({1}) int) input; - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) @IntVal({2}) int bc = (@IntVal({2}) int) input; } diff --git a/framework/tests/value/Issue2367.java b/framework/tests/value/Issue2367.java index 8ab6c16d8d03..18cd65e2d3ab 100644 --- a/framework/tests/value/Issue2367.java +++ b/framework/tests/value/Issue2367.java @@ -12,7 +12,7 @@ public class Issue2367 { // Without the `(byte)` cast, all of these produce the following javac error: // error: incompatible types: possible lossy conversion from int to byte - // The Value Checker's `cast.unsafe` error is analogous and is desirable. + // The Value Checker's `cast.incomparable` error is analogous and is desirable. byte b4 = (byte) 139; // b4 == -117 byte b5 = (byte) -240; diff --git a/framework/tests/value/Overflows.java b/framework/tests/value/Overflows.java index ae18c26bb92c..61fd0f2e8950 100644 --- a/framework/tests/value/Overflows.java +++ b/framework/tests/value/Overflows.java @@ -11,7 +11,7 @@ static void bytes() { static void chars() { char max = Character.MAX_VALUE; - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) @IntVal(0) char maxPlus1 = (char) (max + 1); } diff --git a/framework/tests/value/TypeCast.java b/framework/tests/value/TypeCast.java index 959bb4d7c787..67a0803852d3 100644 --- a/framework/tests/value/TypeCast.java +++ b/framework/tests/value/TypeCast.java @@ -45,12 +45,12 @@ void otherCast() { void rangeCast(@IntRange(from = 127, to = 128) int a, @IntRange(from = 128, to = 129) int b) { @IntRange(from = 0, to = 128) - // :: error: (assignment.type.incompatible) :: warning: (cast.unsafe) + // :: error: (assignment.type.incompatible) :: error: (cast.incomparable) byte c = (byte) a; // (byte) a is @IntRange(from = -128, to = 127) because of casting @IntRange(from = -128, to = -127) - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) byte d = (byte) b; } } diff --git a/framework/tests/value/Underflows.java b/framework/tests/value/Underflows.java index 70eddf4cdf40..d67793149d64 100644 --- a/framework/tests/value/Underflows.java +++ b/framework/tests/value/Underflows.java @@ -10,7 +10,7 @@ static void bytes() { static void chars() { char min = Character.MIN_VALUE; - // :: warning: (cast.unsafe) + // :: error: (cast.incomparable) @IntVal(65535) char maxPlus1 = (char) (min - 1); } diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java index 05297920d5a0..d8d4ff4b0961 100644 --- a/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java +++ b/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java @@ -300,6 +300,20 @@ public static boolean areSamePrimitiveTypes(TypeMirror left, TypeMirror right) { return (left.getKind() == right.getKind()); } + /** + * Returns true iff the arguments are type variables with the same name. This method doesn't + * distinguish the difference of type variables having the same name but declared in different + * contexts, so we should be careful using it. Javac is able to identify the difference and + * issue an unchecked warning for the above scenario. + * + * @param v1 a type variable + * @param v2 a type variable + * @return whether the arguments are the same type variables + */ + public static boolean areSameTypeVariables(TypeVariable v1, TypeVariable v2) { + return v1.asElement().getSimpleName() == v2.asElement().getSimpleName(); + } + /// Predicates /**