From a47729c6268d5e545db1682b20dd7657062f311b Mon Sep 17 00:00:00 2001 From: Svetlana Isakova Date: Fri, 3 Oct 2014 19:33:42 +0400 Subject: [PATCH] KT-5455 Need warning about redundant type cast #KT-5455 Fixed --- .../rendering/DefaultErrorMessages.java | 2 +- .../calls/smartcasts/SmartCastUtils.java | 4 +- .../jet/lang/types/CastDiagnosticsUtil.java | 6 ++- .../BasicExpressionTypingVisitor.java | 48 +++++++++---------- .../lang/types/expressions/DataFlowUtils.java | 17 +++++++ .../tests/cast/bare/FromErrorType.kt | 2 +- .../tests/cast/bare/ToErrorType.kt | 2 +- .../cast/neverSucceeds/CastToNotNullSuper.kt | 8 ++++ .../cast/neverSucceeds/CastToNotNullSuper.txt | 19 ++++++++ .../diagnostics/tests/dataFlow/EmptyIf.kt | 2 +- .../diagnostics/tests/smartCasts/kt5455.kt | 28 +++++++++++ .../diagnostics/tests/smartCasts/kt5455.txt | 22 +++++++++ .../checkers/JetDiagnosticsTestGenerated.java | 12 +++++ 13 files changed, 139 insertions(+), 33 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.kt create mode 100644 compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.txt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/kt5455.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/kt5455.txt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java index b88a2df5e82..86b9f9e7557 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java @@ -252,7 +252,7 @@ public class DefaultErrorMessages { MAP.put(ABSTRACT_SUPER_CALL, "Abstract member cannot be accessed directly"); MAP.put(NOT_A_SUPERTYPE, "Not a supertype"); MAP.put(TYPE_ARGUMENTS_REDUNDANT_IN_SUPER_QUALIFIER, "Type arguments do not need to be specified in a 'super' qualifier"); - MAP.put(USELESS_CAST_STATIC_ASSERT_IS_FINE, "No cast needed, use ':' instead"); + MAP.put(USELESS_CAST_STATIC_ASSERT_IS_FINE, "No cast needed. You can use ':' if you need a cast to a super type"); MAP.put(USELESS_CAST, "No cast needed"); MAP.put(CAST_NEVER_SUCCEEDS, "This cast can never succeed"); MAP.put(USELESS_NULLABLE_CHECK, "Non-null type is checked for instance of nullable type"); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/smartcasts/SmartCastUtils.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/smartcasts/SmartCastUtils.java index 88b24f2e45c..c98a4f89e67 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/smartcasts/SmartCastUtils.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/smartcasts/SmartCastUtils.java @@ -80,9 +80,7 @@ public class SmartCastUtils { return collectSmartCastReceiverValues(dataFlowInfo, dataFlowValue); } else if (receiverToCast instanceof ExpressionReceiver) { - ExpressionReceiver receiver = (ExpressionReceiver) receiverToCast; - DataFlowValue dataFlowValue = - DataFlowValueFactory.createDataFlowValue(receiver.getExpression(), receiver.getType(), bindingContext); + DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(receiverToCast, bindingContext); return collectSmartCastReceiverValues(dataFlowInfo, dataFlowValue); } return Collections.emptyList(); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/CastDiagnosticsUtil.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/CastDiagnosticsUtil.java index e9032ba4682..ce92a892561 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/CastDiagnosticsUtil.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/CastDiagnosticsUtil.java @@ -27,6 +27,7 @@ import org.jetbrains.jet.lang.descriptors.ClassifierDescriptor; import org.jetbrains.jet.lang.descriptors.TypeParameterDescriptor; import org.jetbrains.jet.lang.types.checker.JetTypeChecker; import org.jetbrains.jet.lang.types.checker.TypeCheckingProcedure; +import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import java.util.Collection; import java.util.Collections; @@ -42,6 +43,7 @@ public class CastDiagnosticsUtil { @NotNull JetType rhsType, @NotNull PlatformToKotlinClassMap platformToKotlinClassMap ) { + if (KotlinBuiltIns.getInstance().isNullableNothing(lhsType) && !TypeUtils.isNullableType(rhsType)) return false; if (isRelated(lhsType, rhsType, platformToKotlinClassMap)) return true; // This is an oversimplification (which does not render the method incomplete): // we consider any type parameter capable of taking any value, which may be made more precise if we considered bounds @@ -61,8 +63,8 @@ public class CastDiagnosticsUtil { * (i.e. java.lang.String -> kotlin.String) and ignore mappings that go the other way. */ private static boolean isRelated(@NotNull JetType a, @NotNull JetType b, @NotNull PlatformToKotlinClassMap platformToKotlinClassMap) { - List aTypes = mapToPlatformIndependentTypes(a, platformToKotlinClassMap); - List bTypes = mapToPlatformIndependentTypes(b, platformToKotlinClassMap); + List aTypes = mapToPlatformIndependentTypes(TypeUtils.makeNotNullable(a), platformToKotlinClassMap); + List bTypes = mapToPlatformIndependentTypes(TypeUtils.makeNotNullable(b), platformToKotlinClassMap); for (JetType aType : aTypes) { for (JetType bType : bTypes) { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java index 491c5f7cfb1..5665e91b13f 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java @@ -27,7 +27,6 @@ import org.jetbrains.jet.lang.descriptors.annotations.Annotations; import org.jetbrains.jet.lang.descriptors.impl.AnonymousFunctionDescriptor; import org.jetbrains.jet.lang.descriptors.impl.LocalVariableDescriptor; import org.jetbrains.jet.lang.diagnostics.Diagnostic; -import org.jetbrains.jet.lang.diagnostics.Errors; import org.jetbrains.jet.lang.evaluate.ConstantExpressionEvaluator; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.*; @@ -45,7 +44,6 @@ import org.jetbrains.jet.lang.resolve.calls.results.OverloadResolutionResultsImp import org.jetbrains.jet.lang.resolve.calls.results.OverloadResolutionResultsUtil; import org.jetbrains.jet.lang.resolve.calls.smartcasts.DataFlowInfo; import org.jetbrains.jet.lang.resolve.calls.smartcasts.DataFlowValue; -import org.jetbrains.jet.lang.resolve.calls.smartcasts.DataFlowValueFactory; import org.jetbrains.jet.lang.resolve.calls.smartcasts.Nullability; import org.jetbrains.jet.lang.resolve.calls.tasks.ExplicitReceiverKind; import org.jetbrains.jet.lang.resolve.calls.tasks.ResolutionCandidate; @@ -76,6 +74,7 @@ import static org.jetbrains.jet.lang.diagnostics.Errors.*; import static org.jetbrains.jet.lang.resolve.BindingContext.*; import static org.jetbrains.jet.lang.resolve.DescriptorUtils.getStaticNestedClassesScope; import static org.jetbrains.jet.lang.resolve.calls.context.ContextDependency.INDEPENDENT; +import static org.jetbrains.jet.lang.resolve.calls.smartcasts.DataFlowValueFactory.createDataFlowValue; import static org.jetbrains.jet.lang.resolve.scopes.receivers.ReceiverValue.NO_RECEIVER; import static org.jetbrains.jet.lang.resolve.source.SourcePackage.toSourceElement; import static org.jetbrains.jet.lang.types.TypeUtils.NO_EXPECTED_TYPE; @@ -174,7 +173,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { checkBinaryWithTypeRHS(expression, contextWithNoExpectedType, targetType, subjectType); dataFlowInfo = typeInfo.getDataFlowInfo(); if (operationType == AS_KEYWORD) { - DataFlowValue value = DataFlowValueFactory.createDataFlowValue(left, subjectType, context.trace.getBindingContext()); + DataFlowValue value = createDataFlowValue(left, subjectType, context.trace.getBindingContext()); dataFlowInfo = dataFlowInfo.establishSubtyping(value, targetType); } } @@ -199,10 +198,10 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { context.trace.report(UNSUPPORTED.on(operationSign, "binary operation with type RHS")); return; } - checkForCastImpossibility(expression, actualType, targetType, context); + checkForCastImpossibilityOrRedundancy(expression, actualType, targetType, context); } - private void checkForCastImpossibility( + private void checkForCastImpossibilityOrRedundancy( JetBinaryExpressionWithTypeRHS expression, JetType actualType, JetType targetType, @@ -212,24 +211,25 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { if (!CastDiagnosticsUtil.isCastPossible(actualType, targetType, components.platformToKotlinClassMap)) { context.trace.report(CAST_NEVER_SUCCEEDS.on(expression.getOperationReference())); + return; } - else { - JetTypeChecker typeChecker = JetTypeChecker.DEFAULT; - // Upcast? - if (typeChecker.isSubtypeOf(actualType, targetType)) { - if (!typeChecker.isSubtypeOf(targetType, actualType)) { - // proper upcast: String as Any - context.trace.report(USELESS_CAST_STATIC_ASSERT_IS_FINE.on(expression.getOperationReference())); - } - else { - // cast to itself: String as String - context.trace.report(USELESS_CAST.on(expression.getOperationReference())); - } - } - else if (CastDiagnosticsUtil.isCastErased(actualType, targetType, typeChecker)) { - context.trace.report(Errors.UNCHECKED_CAST.on(expression, actualType, targetType)); + JetTypeChecker typeChecker = JetTypeChecker.DEFAULT; + if (actualType.equals(targetType)) { + // cast to itself: String as String + context.trace.report(USELESS_CAST.on(expression.getOperationReference())); + return; + } + Collection possibleTypes = DataFlowUtils.getAllPossibleTypes( + expression.getLeft(), context.dataFlowInfo, actualType, context.trace.getBindingContext()); + for (JetType possibleType : possibleTypes) { + if (typeChecker.isSubtypeOf(possibleType, targetType)) { + context.trace.report(USELESS_CAST_STATIC_ASSERT_IS_FINE.on(expression.getOperationReference())); + return; } } + if (CastDiagnosticsUtil.isCastErased(actualType, targetType, typeChecker)) { + context.trace.report(UNCHECKED_CAST.on(expression, actualType, targetType)); + } } @Override @@ -803,7 +803,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { context.trace.report(UNNECESSARY_NOT_NULL_ASSERTION.on(operationSign, baseType)); } else { - DataFlowValue value = DataFlowValueFactory.createDataFlowValue(baseExpression, baseType, context.trace.getBindingContext()); + DataFlowValue value = createDataFlowValue(baseExpression, baseType, context.trace.getBindingContext()); dataFlowInfo = dataFlowInfo.disequate(value, DataFlowValue.NULL); } return JetTypeInfo.create(TypeUtils.makeNotNullable(baseType), dataFlowInfo); @@ -835,7 +835,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { } private static boolean isKnownToBeNotNull(JetExpression expression, JetType jetType, ExpressionTypingContext context) { - DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(expression, jetType, context.trace.getBindingContext()); + DataFlowValue dataFlowValue = createDataFlowValue(expression, jetType, context.trace.getBindingContext()); return !context.dataFlowInfo.getNullability(dataFlowValue).canBeNull(); } @@ -1075,7 +1075,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { DataFlowInfo dataFlowInfo = resolvedCall.getDataFlowInfoForArguments().getResultInfo(); if (leftType != null && rightType != null && KotlinBuiltIns.getInstance().isNothingOrNullableNothing(rightType) && !rightType.isNullable()) { - DataFlowValue value = DataFlowValueFactory.createDataFlowValue(left, leftType, context.trace.getBindingContext()); + DataFlowValue value = createDataFlowValue(left, leftType, context.trace.getBindingContext()); dataFlowInfo = dataFlowInfo.disequate(value, DataFlowValue.NULL); } JetType type = resolvedCall.getResultingDescriptor().getReturnType(); @@ -1158,7 +1158,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { JetType type = facade.getTypeInfo(expr, context).getType(); if (type == null || type.isError()) return; - DataFlowValue value = DataFlowValueFactory.createDataFlowValue(expr, type, context.trace.getBindingContext()); + DataFlowValue value = createDataFlowValue(expr, type, context.trace.getBindingContext()); Nullability nullability = context.dataFlowInfo.getNullability(value); boolean expressionIsAlways; diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java index e25b1f67e21..daeb341ad5a 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java @@ -16,6 +16,7 @@ package org.jetbrains.jet.lang.types.expressions; +import com.google.common.collect.Sets; import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.tree.IElementType; @@ -39,6 +40,7 @@ import org.jetbrains.jet.lang.types.checker.JetTypeChecker; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.lexer.JetTokens; +import java.util.Collection; import java.util.Set; import static org.jetbrains.jet.lang.diagnostics.Errors.*; @@ -265,4 +267,19 @@ public class DataFlowUtils { context.trace.report(EXPRESSION_EXPECTED.on(expression, expression)); return JetTypeInfo.create(null, context.dataFlowInfo); } + + @NotNull + public static Collection getAllPossibleTypes( + @NotNull JetExpression expression, + @NotNull DataFlowInfo dataFlowInfo, + @NotNull JetType type, + @NotNull BindingContext bindingContext + ) { + DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(expression, type, bindingContext); + Collection possibleTypes = Sets.newHashSet(type); + if (dataFlowValue.isStableIdentifier()) { + possibleTypes.addAll(dataFlowInfo.getPossibleTypes(dataFlowValue)); + } + return possibleTypes; + } } diff --git a/compiler/testData/diagnostics/tests/cast/bare/FromErrorType.kt b/compiler/testData/diagnostics/tests/cast/bare/FromErrorType.kt index 31fb179653e..7842cd15494 100644 --- a/compiler/testData/diagnostics/tests/cast/bare/FromErrorType.kt +++ b/compiler/testData/diagnostics/tests/cast/bare/FromErrorType.kt @@ -1,6 +1,6 @@ class G fun foo(p: P) { - val v = p as G? + val v = p as G? v!!: G<*> } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/cast/bare/ToErrorType.kt b/compiler/testData/diagnostics/tests/cast/bare/ToErrorType.kt index ee8ce3bdb93..0d7d75f6fb3 100644 --- a/compiler/testData/diagnostics/tests/cast/bare/ToErrorType.kt +++ b/compiler/testData/diagnostics/tests/cast/bare/ToErrorType.kt @@ -1,6 +1,6 @@ class P fun foo(p: P): Any { - val v = p as G + val v = p as G return v } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.kt b/compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.kt new file mode 100644 index 00000000000..15e4ca66f5c --- /dev/null +++ b/compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.kt @@ -0,0 +1,8 @@ +open class A { + fun foo() {} +} +class B : A() + +fun test(b: B?) { + (b as A).foo() +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.txt b/compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.txt new file mode 100644 index 00000000000..5434ed69a67 --- /dev/null +++ b/compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.txt @@ -0,0 +1,19 @@ +package + +internal fun test(/*0*/ b: B?): kotlin.Unit + +internal open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal final fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal final class B : A { + public constructor B() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal final override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/dataFlow/EmptyIf.kt b/compiler/testData/diagnostics/tests/dataFlow/EmptyIf.kt index ea752747354..04ef93280a0 100644 --- a/compiler/testData/diagnostics/tests/dataFlow/EmptyIf.kt +++ b/compiler/testData/diagnostics/tests/dataFlow/EmptyIf.kt @@ -11,7 +11,7 @@ fun f2(s: Number?) { } fun f3(s: Number?) { - if (s is Int && s as Int == 42); + if (s is Int && s as Int == 42); s : Int } diff --git a/compiler/testData/diagnostics/tests/smartCasts/kt5455.kt b/compiler/testData/diagnostics/tests/smartCasts/kt5455.kt new file mode 100644 index 00000000000..173016bff7a --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/kt5455.kt @@ -0,0 +1,28 @@ +//KT-5455 Need warning about redundant type cast +fun foo(o: Any): Int { + if (o is String) { + return (o as String).length + } + return -1 +} + +open class A { + fun foo() {} +} +class B: A() + +fun test(a: Any?) { + if (a is B) { + (a as A).foo() + } +} + +fun test1(a: B) { + (a as A?)?.foo() +} + +fun test2(b: B?) { + if (b != null) { + (b as A).foo() + } +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/kt5455.txt b/compiler/testData/diagnostics/tests/smartCasts/kt5455.txt new file mode 100644 index 00000000000..53bfd88ec9a --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/kt5455.txt @@ -0,0 +1,22 @@ +package + +internal fun foo(/*0*/ o: kotlin.Any): kotlin.Int +internal fun test(/*0*/ a: kotlin.Any?): kotlin.Unit +internal fun test1(/*0*/ a: B): kotlin.Unit +internal fun test2(/*0*/ b: B?): kotlin.Unit + +internal open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal final fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal final class B : A { + public constructor B() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal final override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index ebd8a759a66..b86fdfc57ab 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -1276,6 +1276,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/cast/neverSucceeds"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("CastToNotNullSuper.kt") + public void testCastToNotNullSuper() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/cast/neverSucceeds/CastToNotNullSuper.kt"); + doTest(fileName); + } + @TestMetadata("MappedDirect.kt") public void testMappedDirect() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedDirect.kt"); @@ -9410,6 +9416,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("kt5455.kt") + public void testKt5455() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/kt5455.kt"); + doTest(fileName); + } + @TestMetadata("noErrorCheckForPackageLevelVal.kt") public void testNoErrorCheckForPackageLevelVal() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/noErrorCheckForPackageLevelVal.kt");