From a28d211442a3b8b4f083962c237e93aa909814d6 Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Wed, 4 Sep 2013 13:12:07 +0400 Subject: [PATCH] 'Cast can never succeed' diagnostics improved --- .../BasicExpressionTypingVisitor.java | 88 +++++++++++++++++-- .../tests/cast/neverSucceeds/MappedDirect.kt | 33 +++++++ .../cast/neverSucceeds/MappedSubtypes.kt | 21 +++++ .../cast/neverSucceeds/NoGenericsRelated.kt | 46 ++++++++++ .../cast/neverSucceeds/NoGenericsUnrelated.kt | 34 +++++++ .../checkers/JetDiagnosticsTestGenerated.java | 37 +++++++- 6 files changed, 250 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedDirect.kt create mode 100644 compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedSubtypes.kt create mode 100644 compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsRelated.kt create mode 100644 compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsUnrelated.kt 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 c589d7689ad..bb1dc5d38fb 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 @@ -35,7 +35,9 @@ import org.jetbrains.jet.lang.resolve.calls.autocasts.DataFlowInfo; import org.jetbrains.jet.lang.resolve.calls.autocasts.DataFlowValue; import org.jetbrains.jet.lang.resolve.calls.autocasts.DataFlowValueFactory; import org.jetbrains.jet.lang.resolve.calls.autocasts.Nullability; -import org.jetbrains.jet.lang.resolve.calls.context.*; +import org.jetbrains.jet.lang.resolve.calls.context.CheckValueArgumentsMode; +import org.jetbrains.jet.lang.resolve.calls.context.ExpressionPosition; +import org.jetbrains.jet.lang.resolve.calls.context.TemporaryTraceAndCache; import org.jetbrains.jet.lang.resolve.calls.model.ResolvedCall; import org.jetbrains.jet.lang.resolve.calls.model.ResolvedCallWithTrace; import org.jetbrains.jet.lang.resolve.calls.model.VariableAsFunctionResolvedCall; @@ -71,7 +73,8 @@ import static org.jetbrains.jet.lang.resolve.calls.context.ContextDependency.IND import static org.jetbrains.jet.lang.resolve.scopes.receivers.ReceiverValue.NO_RECEIVER; import static org.jetbrains.jet.lang.types.TypeUtils.NO_EXPECTED_TYPE; import static org.jetbrains.jet.lang.types.TypeUtils.noExpectedType; -import static org.jetbrains.jet.lang.types.expressions.ControlStructureTypingUtils.*; +import static org.jetbrains.jet.lang.types.expressions.ControlStructureTypingUtils.createCallForSpecialConstruction; +import static org.jetbrains.jet.lang.types.expressions.ControlStructureTypingUtils.resolveSpecialConstructionAsCall; import static org.jetbrains.jet.lang.types.expressions.ExpressionTypingUtils.*; @SuppressWarnings("SuspiciousMethodCalls") @@ -190,7 +193,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { return DataFlowUtils.checkType(result, expression, context, dataFlowInfo); } - private static void checkBinaryWithTypeRHS( + private void checkBinaryWithTypeRHS( @NotNull JetBinaryExpressionWithTypeRHS expression, @NotNull ExpressionTypingContext context, @NotNull JetType targetType, @@ -209,7 +212,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { checkForCastImpossibility(expression, actualType, targetType, context); } - private static void checkForCastImpossibility( + private void checkForCastImpossibility( JetBinaryExpressionWithTypeRHS expression, JetType actualType, JetType targetType, @@ -217,15 +220,15 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { ) { if (actualType == null || noExpectedType(targetType)) return; + if (!isCastPossible(actualType, targetType)) { + context.trace.report(CAST_NEVER_SUCCEEDS.on(expression.getOperationReference())); + } + JetTypeChecker typeChecker = JetTypeChecker.INSTANCE; if (!typeChecker.isSubtypeOf(targetType, actualType)) { if (typeChecker.isSubtypeOf(actualType, targetType)) { context.trace.report(USELESS_CAST_STATIC_ASSERT_IS_FINE.on(expression.getOperationReference())); } - else { - // See JET-58 Make 'as never succeeds' a warning, or even never check for Java (external) types - context.trace.report(CAST_NEVER_SUCCEEDS.on(expression.getOperationReference())); - } } else { if (typeChecker.isSubtypeOf(actualType, targetType)) { @@ -239,6 +242,75 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { } } + // As this method produces a warning, it must be _complete_ (not sound), i.e. every time it says "cast impossible", + // it must be really impossible + public boolean isCastPossible(@NotNull JetType lhsType, @NotNull JetType rhsType) { + if (isRelated(lhsType, rhsType)) 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 + if (isTypeParameter(lhsType) || isTypeParameter(rhsType)) return true; + if (isFinal(lhsType) || isFinal(rhsType)) return false; + if (isTrait(lhsType) || isTrait(rhsType)) return true; + return false; + } + + /** + * Two types are related, roughly, when one is a subtype or supertype of the other. + * + * Note that some types have platform-specific counterparts, i.e. jet.String is mapped to java.lang.String, + * such types (and all their sub- and supertypes) are related too. + * + * Due to limitations in PlatformToKotlinClassMap, we only consider mapping of platform classes to Kotlin classed + * (i.e. java.lang.String -> jet.String) and ignore mappings that go the other way. + */ + private boolean isRelated(@NotNull JetType a, @NotNull JetType b) { + List aTypes = mapToPlatformIndependentTypes(a); + List bTypes = mapToPlatformIndependentTypes(b); + + for (int i = 0; i < aTypes.size(); i++) { + JetType aType = aTypes.get(i); + for (int j = 0; j < bTypes.size(); j++) { + JetType bType = bTypes.get(j); + + if (JetTypeChecker.INSTANCE.isSubtypeOf(aType, bType)) return true; + if (JetTypeChecker.INSTANCE.isSubtypeOf(bType, aType)) return true; + } + } + + return false; + } + + private List mapToPlatformIndependentTypes(@NotNull JetType type) { + ClassifierDescriptor descriptor = type.getConstructor().getDeclarationDescriptor(); + if (!(descriptor instanceof ClassDescriptor)) return Collections.singletonList(type); + + ClassDescriptor originalClass = (ClassDescriptor) descriptor; + Collection kotlinClasses = platformToKotlinClassMap.mapPlatformClass(originalClass); + if (kotlinClasses.isEmpty()) return Collections.singletonList(type); + + List result = Lists.newArrayListWithCapacity(2); + result.add(type); + for (ClassDescriptor classDescriptor : kotlinClasses) { + JetType kotlinType = TypeUtils.substituteProjectionsForParameters(classDescriptor, type.getArguments()); + result.add(kotlinType); + } + + return result; + } + + private static boolean isTypeParameter(@NotNull JetType type) { + return type.getConstructor().getDeclarationDescriptor() instanceof TypeParameterDescriptor; + } + + private static boolean isFinal(@NotNull JetType type) { + return !TypeUtils.canHaveSubtypes(JetTypeChecker.INSTANCE, type); + } + + private static boolean isTrait(@NotNull JetType type) { + ClassifierDescriptor descriptor = type.getConstructor().getDeclarationDescriptor(); + return descriptor instanceof ClassDescriptor && ((ClassDescriptor) descriptor).getKind() == ClassKind.TRAIT; + } + /** * Check if assignment from supertype to subtype is erased. * It is an error in "is" statement and warning in "as". diff --git a/compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedDirect.kt b/compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedDirect.kt new file mode 100644 index 00000000000..c3036d892ac --- /dev/null +++ b/compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedDirect.kt @@ -0,0 +1,33 @@ +// !DIAGNOSTICS: -WARNING +CAST_NEVER_SUCCEEDS +import java.lang.String as JString +import java.lang.CharSequence as JCS + +fun test( + s: String, + js: JString, + cs: CharSequence, + jcs: JCS +) { + s as JString + s as JCS + s as CharSequence + s as String + + js as JString + js as JCS + js as CharSequence + js as String + + cs as JString + cs as JCS + cs as CharSequence + cs as String + + jcs as JString + jcs as JCS + jcs as CharSequence + jcs as String + + jcs as Int + s as java.lang.Integer +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedSubtypes.kt b/compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedSubtypes.kt new file mode 100644 index 00000000000..821ae2270aa --- /dev/null +++ b/compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedSubtypes.kt @@ -0,0 +1,21 @@ +// !DIAGNOSTICS: -WARNING +CAST_NEVER_SUCCEEDS -ABSTRACT_MEMBER_NOT_IMPLEMENTED +import java.lang.CharSequence as JCS + +class JSub: JCS +class Sub: CharSequence + +fun test( + s: Sub, + js: JSub, + cs: CharSequence, + jcs: JCS +) { + // js as CharSequence // - this case is not supported due to limitation in PlatformToKotlinClassMap + js as JCS + + s as CharSequence + s as JCS + + js as Sub + s as JSub +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsRelated.kt b/compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsRelated.kt new file mode 100644 index 00000000000..f1f4577abcf --- /dev/null +++ b/compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsRelated.kt @@ -0,0 +1,46 @@ +// !DIAGNOSTICS: -WARNING +CAST_NEVER_SUCCEEDS +trait T1 +trait T2 +trait T3 +open class OC1: T1 +open class OC2: OC1(), T2 +class FC1: OC2(), T3 +trait T4: OC1 +trait T5: T2 + +fun test( + t2: T2, + t4: T4, + fc1: FC1, + oc1: OC1, + oc2: OC2, + tp1: TP1, + tp2: TP2 +) { + fc1 as FC1 + fc1 as OC1 + fc1 as T1 + fc1 as TP1 + + oc1 as FC1 + oc1 as OC2 + oc2 as OC1 + oc1 as T2 + oc1 as T1 + oc1 as TP1 + oc1 as TP2 + + t2 as FC1 + t2 as OC2 + t4 as OC1 + t2 as T2 + t2 as T5 + t2 as TP2 + + tp1 as FC1 + tp1 as OC1 + tp1 as OC2 + tp2 as T2 + tp2 as T5 + tp1 as TP3 +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsUnrelated.kt b/compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsUnrelated.kt new file mode 100644 index 00000000000..897080f40ea --- /dev/null +++ b/compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsUnrelated.kt @@ -0,0 +1,34 @@ +// !DIAGNOSTICS: -WARNING +CAST_NEVER_SUCCEEDS +trait Trait1 +trait Trait2 +open class OClass1 +open class OClass2 +class FClass1 +class FClass2 + +fun test( + t1: Trait1, + oc1: OClass1, + fc1: FClass1, + tp1: TP1 +) { + t1 as Trait2 + t1 as OClass2 + t1 as FClass2 + t1 as TP2 + + oc1 as Trait2 + oc1 as OClass2 + oc1 as FClass2 + oc1 as TP2 + + fc1 as Trait2 + fc1 as OClass2 + fc1 as FClass2 + fc1 as TP2 + + tp1 as Trait2 + tp1 as OClass2 + tp1 as FClass2 + tp1 as TP2 +} \ 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 f8a0e241433..634079ffe53 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -954,6 +954,7 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage } @TestMetadata("compiler/testData/diagnostics/tests/cast") + @InnerTestClasses({Cast.NeverSucceeds.class}) public static class Cast extends AbstractDiagnosticsTestWithEagerResolve { public void testAllFilesPresentInCast() throws Exception { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("compiler/testData/diagnostics/tests/cast"), Pattern.compile("^(.+)\\.kt$"), true); @@ -1064,6 +1065,40 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage doTest("compiler/testData/diagnostics/tests/cast/WhenWithExpression.kt"); } + @TestMetadata("compiler/testData/diagnostics/tests/cast/neverSucceeds") + public static class NeverSucceeds extends AbstractDiagnosticsTestWithEagerResolve { + public void testAllFilesPresentInNeverSucceeds() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("compiler/testData/diagnostics/tests/cast/neverSucceeds"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("MappedDirect.kt") + public void testMappedDirect() throws Exception { + doTest("compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedDirect.kt"); + } + + @TestMetadata("MappedSubtypes.kt") + public void testMappedSubtypes() throws Exception { + doTest("compiler/testData/diagnostics/tests/cast/neverSucceeds/MappedSubtypes.kt"); + } + + @TestMetadata("NoGenericsRelated.kt") + public void testNoGenericsRelated() throws Exception { + doTest("compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsRelated.kt"); + } + + @TestMetadata("NoGenericsUnrelated.kt") + public void testNoGenericsUnrelated() throws Exception { + doTest("compiler/testData/diagnostics/tests/cast/neverSucceeds/NoGenericsUnrelated.kt"); + } + + } + + public static Test innerSuite() { + TestSuite suite = new TestSuite("Cast"); + suite.addTestSuite(Cast.class); + suite.addTestSuite(NeverSucceeds.class); + return suite; + } } @TestMetadata("compiler/testData/diagnostics/tests/checkArguments") @@ -5423,7 +5458,7 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage suite.addTestSuite(Annotations.class); suite.addTestSuite(BackingField.class); suite.addTestSuite(CallableReference.class); - suite.addTestSuite(Cast.class); + suite.addTest(Cast.innerSuite()); suite.addTestSuite(CheckArguments.class); suite.addTestSuite(ControlFlowAnalysis.class); suite.addTestSuite(ControlStructures.class);