From 7f6267566513575713af6c4e23ee6e29cf43e158 Mon Sep 17 00:00:00 2001 From: Svetlana Isakova Date: Thu, 13 Nov 2014 13:39:31 +0300 Subject: [PATCH] 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 --- .../ControlStructureTypingUtils.java | 66 +++++++++++-------- .../lang/types/expressions/DataFlowUtils.java | 21 +++++- .../specialConstructions/constantsInIf.kt | 13 ++++ .../constantsInIf.lazy.log | 37 +++++++++++ .../specialConstructions/constantsInIf.txt | 4 ++ .../multipleSuperClasses.kt | 16 +++++ .../multipleSuperClasses.lazy.log | 30 +++++++++ .../multipleSuperClasses.txt | 33 ++++++++++ .../checkers/JetDiagnosticsTestGenerated.java | 12 ++++ 9 files changed, 201 insertions(+), 31 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.kt create mode 100644 compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.lazy.log create mode 100644 compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.txt create mode 100644 compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.kt create mode 100644 compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.lazy.log create mode 100644 compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.txt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingUtils.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingUtils.java index 94f8d274a59..136c852cefd 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingUtils.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingUtils.java @@ -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 checkTypeVisitor = new JetVisitor() { - private void checkExpressionType(@Nullable JetExpression expression, CheckTypeContext c) { - if (expression == null) return; - expression.accept(this, c); + final JetVisitor checkTypeVisitor = new JetVisitor() { + + private boolean checkExpressionType(@NotNull JetExpression expression, CheckTypeContext c) { + JetTypeInfo typeInfo = BindingContextUtils.getRecordedTypeInfo(expression, c.trace.getBindingContext()); + if (typeInfo == null) return false; + + Ref 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); } }; 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 daeb341ad5a..8921224d88e 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 @@ -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 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; } diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.kt b/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.kt new file mode 100644 index 00000000000..c6655a46a5b --- /dev/null +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.kt @@ -0,0 +1,13 @@ +// !DIAGNOSTICS: -USELESS_ELVIS + +fun test() { + bar(if (true) { + 1 + } else { + 2 + }) + + bar(1 ?: 2) +} + +fun bar(s: String) = s \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.lazy.log b/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.lazy.log new file mode 100644 index 00000000000..581dc2a859a --- /dev/null +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.lazy.log @@ -0,0 +1,37 @@ +DescriptorResolver@0 { + = JetTypeImpl@1['String'] +} + +LazyJavaPackageFragmentProvider@2 { + packageFragments('': FqName@3) = LazyJavaPackageFragment@4[''] + 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[''] { + 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 +} diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.txt b/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.txt new file mode 100644 index 00000000000..474440566a9 --- /dev/null +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/constantsInIf.txt @@ -0,0 +1,4 @@ +package + +internal fun bar(/*0*/ s: kotlin.String): kotlin.String +internal fun test(): kotlin.Unit diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.kt b/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.kt new file mode 100644 index 00000000000..10772f81a8c --- /dev/null +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.kt @@ -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? = c ?: d ?: e + + val b: B? = if (false) if (true) c else d else e + + //outer elvis operator and if-expression have error types +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.lazy.log b/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.lazy.log new file mode 100644 index 00000000000..33e2661e54f --- /dev/null +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.lazy.log @@ -0,0 +1,30 @@ +LazyJavaPackageFragmentProvider@0 { + packageFragments('': FqName@1) = LazyJavaPackageFragment@2[''] + 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 +} diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.txt b/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.txt new file mode 100644 index 00000000000..261a0fa388f --- /dev/null +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/multipleSuperClasses.txt @@ -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 +} diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index 25eb6f5d447..a5b62f5546f 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -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");