Fixed error reporting for special constructions (if, elvis)

Track whether an error was reported for sub expressions (like 'if' branches) or it should be reported for the whole expression #KT-6189 Fixed
2014-11-13 13:39:31 +03:00
parent 8109b1f997
commit 7f62675665
9 changed files with 201 additions and 31 deletions
@@ -20,6 +20,7 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.intellij.lang.ASTNode;
 import com.intellij.openapi.diagnostic.Logger;
+import com.intellij.openapi.util.Ref;
 import com.intellij.psi.util.PsiTreeUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -260,64 +261,73 @@ public class ControlStructureTypingUtils {
            }
        }

-        final JetVisitor<Void, CheckTypeContext> checkTypeVisitor = new JetVisitor<Void, CheckTypeContext>() {
-            private void checkExpressionType(@Nullable JetExpression expression, CheckTypeContext c) {
-                if (expression == null) return;
-                expression.accept(this, c);
+        final JetVisitor<Boolean, CheckTypeContext> checkTypeVisitor = new JetVisitor<Boolean, CheckTypeContext>() {
+
+            private boolean checkExpressionType(@NotNull JetExpression expression, CheckTypeContext c) {
+                JetTypeInfo typeInfo = BindingContextUtils.getRecordedTypeInfo(expression, c.trace.getBindingContext());
+                if (typeInfo == null) return false;
+
+                Ref<Boolean> hasError = Ref.create();
+                DataFlowUtils.checkType(typeInfo.getType(), expression, c.expectedType, typeInfo.getDataFlowInfo(), c.trace, hasError);
+                return hasError.get();
+            }
+
+            private boolean checkExpressionTypeRecursively(@Nullable JetExpression expression, CheckTypeContext c) {
+                if (expression == null) return false;
+                return expression.accept(this, c);
+            }
+
+            private boolean checkSubExpressions(
+                    JetExpression firstSub, JetExpression secondSub, JetExpression expression,
+                    CheckTypeContext firstContext, CheckTypeContext secondContext, CheckTypeContext context
+            ) {
+                boolean errorWasReported = checkExpressionTypeRecursively(firstSub, firstContext);
+                errorWasReported |= checkExpressionTypeRecursively(secondSub, secondContext);
+                return errorWasReported || checkExpressionType(expression, context);
            }

            @Override
-            public Void visitIfExpression(@NotNull JetIfExpression ifExpression, CheckTypeContext c) {
+            public Boolean visitIfExpression(@NotNull JetIfExpression ifExpression, CheckTypeContext c) {
                JetExpression thenBranch = ifExpression.getThen();
                JetExpression elseBranch = ifExpression.getElse();
                if (thenBranch == null || elseBranch == null) {
-                    visitExpression(ifExpression, c);
-                    return null;
+                    return checkExpressionType(ifExpression, c);
                }
-                checkExpressionType(thenBranch, c);
-                checkExpressionType(elseBranch, c);
-                return null;
+                return checkSubExpressions(thenBranch, elseBranch, ifExpression, c, c, c);
            }

            @Override
-            public Void visitBlockExpression(@NotNull JetBlockExpression expression, CheckTypeContext c) {
+            public Boolean visitBlockExpression(@NotNull JetBlockExpression expression, CheckTypeContext c) {
                if (expression.getStatements().isEmpty()) {
-                    visitExpression(expression, c);
-                    return null;
+                    return checkExpressionType(expression, c);
                }
                JetElement lastStatement = JetPsiUtil.getLastStatementInABlock(expression);
                if (lastStatement instanceof JetExpression) {
-                    checkExpressionType((JetExpression) lastStatement, c);
+                    return checkExpressionTypeRecursively((JetExpression) lastStatement, c);
                }
-                return null;
+                return false;
            }

            @Override
-            public Void visitPostfixExpression(@NotNull JetPostfixExpression expression, CheckTypeContext c) {
+            public Boolean visitPostfixExpression(@NotNull JetPostfixExpression expression, CheckTypeContext c) {
                if (expression.getOperationReference().getReferencedNameElementType() == JetTokens.EXCLEXCL) {
-                    checkExpressionType(expression.getBaseExpression(), c.makeTypeNullable());
-                    return null;
+                    return checkExpressionTypeRecursively(expression.getBaseExpression(), c.makeTypeNullable());
                }
                return super.visitPostfixExpression(expression, c);
            }

            @Override
-            public Void visitBinaryExpression(@NotNull JetBinaryExpression expression, CheckTypeContext c) {
+            public Boolean visitBinaryExpression(@NotNull JetBinaryExpression expression, CheckTypeContext c) {
                if (expression.getOperationReference().getReferencedNameElementType() == JetTokens.ELVIS) {
-                    checkExpressionType(expression.getLeft(), c.makeTypeNullable());
-                    checkExpressionType(expression.getRight(), c);
-                    return null;
+
+                    return checkSubExpressions(expression.getLeft(), expression.getRight(), expression, c.makeTypeNullable(), c, c);
                }
                return super.visitBinaryExpression(expression, c);
            }

            @Override
-            public Void visitExpression(@NotNull JetExpression expression, CheckTypeContext c) {
-                JetTypeInfo typeInfo = BindingContextUtils.getRecordedTypeInfo(expression, c.trace.getBindingContext());
-                if (typeInfo != null) {
-                    DataFlowUtils.checkType(typeInfo.getType(), expression, c.expectedType, typeInfo.getDataFlowInfo(), c.trace);
-                }
-                return null;
+            public Boolean visitExpression(@NotNull JetExpression expression, CheckTypeContext c) {
+                return checkExpressionType(expression, c);
            }
        };

@@ -160,9 +160,21 @@ public class DataFlowUtils {
    }

    @Nullable
-    public static JetType checkType(@Nullable final JetType expressionType, @NotNull JetExpression expressionToCheck,
-            @NotNull JetType expectedType, @NotNull final DataFlowInfo dataFlowInfo, @NotNull final BindingTrace trace
+    public static JetType checkType(
+            @Nullable JetType expressionType, @NotNull JetExpression expressionToCheck,
+            @NotNull JetType expectedType, @NotNull DataFlowInfo dataFlowInfo, @NotNull BindingTrace trace
    ) {
+        return checkType(expressionType, expressionToCheck, expectedType, dataFlowInfo, trace, null);
+    }
+
+    @Nullable
+    public static JetType checkType(
+            @Nullable final JetType expressionType, @NotNull JetExpression expressionToCheck,
+            @NotNull JetType expectedType, @NotNull final DataFlowInfo dataFlowInfo, @NotNull final BindingTrace trace,
+            @Nullable Ref<Boolean> hasError
+    ) {
+        if (hasError != null) hasError.set(false);
+
        final JetExpression expression = JetPsiUtil.safeDeparenthesize(expressionToCheck, false);
        recordExpectedType(trace, expression, expectedType);

@@ -209,7 +221,9 @@ public class DataFlowUtils {
            if (value instanceof IntegerValueTypeConstant) {
                value = EvaluatePackage.createCompileTimeConstantWithType((IntegerValueTypeConstant) value, expectedType);
            }
-            new CompileTimeConstantChecker(trace, true).checkConstantExpressionType(value, (JetConstantExpression) expression, expectedType);
+            boolean error = new CompileTimeConstantChecker(trace, true)
+                    .checkConstantExpressionType(value, (JetConstantExpression) expression, expectedType);
+            if (hasError != null) hasError.set(error);
            return expressionType;
        }

@@ -222,6 +236,7 @@ public class DataFlowUtils {
            }
        }
        trace.report(TYPE_MISMATCH.on(expression, expectedType, expressionType));
+        if (hasError != null) hasError.set(true);
        return expressionType;
    }

@@ -0,0 +1,13 @@
+// !DIAGNOSTICS: -USELESS_ELVIS
+
+fun test() {
+    bar(if (true) {
+        <!CONSTANT_EXPECTED_TYPE_MISMATCH!>1<!>
+    } else {
+        <!CONSTANT_EXPECTED_TYPE_MISMATCH!>2<!>
+    })
+
+    bar(<!CONSTANT_EXPECTED_TYPE_MISMATCH!>1<!> ?: <!CONSTANT_EXPECTED_TYPE_MISMATCH!>2<!>)
+}
+
+fun bar(s: String) = s
@@ -0,0 +1,37 @@
+DescriptorResolver@0 {
+    <name not found> = JetTypeImpl@1['String']
+}
+
+LazyJavaPackageFragmentProvider@2 {
+    packageFragments('<root>': FqName@3) = LazyJavaPackageFragment@4['<root>']
+    packageFragments('String': FqName@5) = null
+    packageFragments('java': FqName@6) = LazyJavaPackageFragment@7['java']
+    packageFragments('java.lang': FqName@8) = LazyJavaPackageFragment@9['lang']
+    packageFragments('java.lang.String': FqName@10) = null
+    packageFragments('kotlin': FqName@11) = null
+    packageFragments('kotlin.String': FqName@12) = null
+    packageFragments('kotlin.io': FqName@13) = null
+    packageFragments('kotlin.jvm': FqName@14) = null
+}
+
+LazyJavaPackageFragment@4['<root>'] {
+    classes('String': Name@15) = null // through LazyPackageFragmentScopeForJavaPackage@16
+    classes('bar': Name@17) = null // through LazyPackageFragmentScopeForJavaPackage@16
+    deserializedPackageScope = Empty@18 // through LazyPackageFragmentScopeForJavaPackage@16
+    functions('bar': Name@17) = EmptyList@19[empty] // through LazyPackageFragmentScopeForJavaPackage@16
+    memberIndex = computeMemberIndex$1@20 // through LazyPackageFragmentScopeForJavaPackage@16
+}
+
+LazyJavaPackageFragment@7['java'] {
+    classes('lang': Name@21) = null // through LazyPackageFragmentScopeForJavaPackage@22
+    deserializedPackageScope = Empty@18 // through LazyPackageFragmentScopeForJavaPackage@22
+    functions('lang': Name@23) = EmptyList@19[empty] // through LazyPackageFragmentScopeForJavaPackage@22
+    memberIndex = computeMemberIndex$1@24 // through LazyPackageFragmentScopeForJavaPackage@22
+}
+
+LazyJavaPackageFragment@9['lang'] {
+    classes('bar': Name@17) = null // through LazyPackageFragmentScopeForJavaPackage@25
+    deserializedPackageScope = Empty@18 // through LazyPackageFragmentScopeForJavaPackage@25
+    functions('bar': Name@17) = EmptyList@19[empty] // through LazyPackageFragmentScopeForJavaPackage@25
+    memberIndex = computeMemberIndex$1@26 // through LazyPackageFragmentScopeForJavaPackage@25
+}
@@ -0,0 +1,4 @@
+package
+
+internal fun bar(/*0*/ s: kotlin.String): kotlin.String
+internal fun test(): kotlin.Unit
@@ -0,0 +1,16 @@
+// !DIAGNOSTICS: -UNUSED_VARIABLE
+
+trait A
+trait B
+
+trait C: A, B
+trait D: A, B
+trait E: A, B
+
+fun foo(c: C?, d: D?, e: E?) {
+    val a: A? = <!TYPE_MISMATCH!>c ?: d<!> ?: e
+
+    val b: B? = if (false) <!TYPE_MISMATCH!>if (true) c else d<!> else e
+
+    //outer elvis operator and if-expression have error types
+}
@@ -0,0 +1,30 @@
+LazyJavaPackageFragmentProvider@0 {
+    packageFragments('<root>': FqName@1) = LazyJavaPackageFragment@2['<root>']
+    packageFragments('A': FqName@3) = null
+    packageFragments('B': FqName@4) = null
+    packageFragments('C': FqName@5) = null
+    packageFragments('D': FqName@6) = null
+    packageFragments('E': FqName@7) = null
+    packageFragments('java': FqName@8) = LazyJavaPackageFragment@9['java']
+    packageFragments('java.lang': FqName@10) = LazyJavaPackageFragment@11['lang']
+    packageFragments('java.lang.A': FqName@12) = null
+    packageFragments('java.lang.B': FqName@13) = null
+    packageFragments('java.lang.C': FqName@14) = null
+    packageFragments('java.lang.D': FqName@15) = null
+    packageFragments('java.lang.E': FqName@16) = null
+    packageFragments('kotlin': FqName@17) = null
+    packageFragments('kotlin.A': FqName@18) = null
+    packageFragments('kotlin.B': FqName@19) = null
+    packageFragments('kotlin.C': FqName@20) = null
+    packageFragments('kotlin.D': FqName@21) = null
+    packageFragments('kotlin.E': FqName@22) = null
+    packageFragments('kotlin.io': FqName@23) = null
+    packageFragments('kotlin.jvm': FqName@24) = null
+}
+
+LazyJavaPackageFragment@9['java'] {
+    classes('lang': Name@25) = null // through LazyPackageFragmentScopeForJavaPackage@26
+    deserializedPackageScope = Empty@27 // through LazyPackageFragmentScopeForJavaPackage@26
+    functions('lang': Name@28) = EmptyList@29[empty] // through LazyPackageFragmentScopeForJavaPackage@26
+    memberIndex = computeMemberIndex$1@30 // through LazyPackageFragmentScopeForJavaPackage@26
+}
@@ -0,0 +1,33 @@
+package
+
+internal fun foo(/*0*/ c: C?, /*1*/ d: D?, /*2*/ e: E?): kotlin.Unit
+
+internal trait A {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait B {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait C : A, B {
+    public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait D : A, B {
+    public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait E : A, B {
+    public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String
+}
@@ -9002,6 +9002,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                    JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/resolve/specialConstructions"), Pattern.compile("^(.+)\\.kt$"), true);
                }

+                @TestMetadata("constantsInIf.kt")
+                public void testConstantsInIf() throws Exception {
+                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.kt");
+                    doTest(fileName);
+                }
+
                @TestMetadata("elvisAsCall.kt")
                public void testElvisAsCall() throws Exception {
                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt");
@@ -9014,6 +9020,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                    doTest(fileName);
                }

+                @TestMetadata("multipleSuperClasses.kt")
+                public void testMultipleSuperClasses() throws Exception {
+                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.kt");
+                    doTest(fileName);
+                }
+
                @TestMetadata("reportTypeMismatchDeeplyOnBranches.kt")
                public void testReportTypeMismatchDeeplyOnBranches() throws Exception {
                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/resolve/specialConstructions/reportTypeMismatchDeeplyOnBranches.kt");