From 609ffc10a937c06ff55005bdb3ed1d7036576ea8 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Wed, 13 Jan 2016 19:31:23 +0300 Subject: [PATCH] KT-10322, KT-10646, KT-10647: - update diagnostic to (supposedly) more useful - also report IMPLICIT_CAST_TO_ANY if expected type is DONT_CARE (effectively "no expected type" for lambda expression). --- .../cfg/ControlFlowInformationProvider.java | 41 ++++++++++++------- .../jetbrains/kotlin/diagnostics/Errors.java | 2 +- .../rendering/DefaultErrorMessages.java | 4 +- .../controlStructures/ifInResultOfLambda.kt | 4 +- .../controlStructures/ifWhenWithoutElse.kt | 2 +- .../improperElseInExpression.kt | 8 ++-- .../tests/controlStructures/kt10322.kt | 12 ++++++ .../tests/controlStructures/kt10322.txt | 4 ++ .../kt770.kt351.kt735_StatementType.kt | 26 ++++++------ .../controlStructures/whenInResultOfLambda.kt | 13 +++++- .../whenInResultOfLambda.txt | 2 + .../testsWithStdLib/implicitCastToAny.kt | 22 ++++++++++ .../testsWithStdLib/implicitCastToAny.txt | 6 +++ .../checkers/DiagnosticsTestGenerated.java | 6 +++ .../DiagnosticsTestWithStdLibGenerated.java | 6 +++ 15 files changed, 118 insertions(+), 40 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/controlStructures/kt10322.kt create mode 100644 compiler/testData/diagnostics/tests/controlStructures/kt10322.txt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java index 98416b61fea..19a2fc2bfbd 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java @@ -53,6 +53,7 @@ import org.jetbrains.kotlin.diagnostics.Errors; import org.jetbrains.kotlin.idea.MainFunctionDetector; import org.jetbrains.kotlin.lexer.KtTokens; import org.jetbrains.kotlin.psi.*; +import org.jetbrains.kotlin.psi.psiUtil.PsiUtilsKt; import org.jetbrains.kotlin.resolve.*; import org.jetbrains.kotlin.resolve.bindingContextUtil.BindingContextUtilsKt; import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; @@ -69,6 +70,7 @@ import static org.jetbrains.kotlin.cfg.TailRecursionKind.*; import static org.jetbrains.kotlin.diagnostics.Errors.*; import static org.jetbrains.kotlin.diagnostics.Errors.UNREACHABLE_CODE; import static org.jetbrains.kotlin.resolve.BindingContext.*; +import static org.jetbrains.kotlin.types.TypeUtils.DONT_CARE; import static org.jetbrains.kotlin.types.TypeUtils.NO_EXPECTED_TYPE; import static org.jetbrains.kotlin.types.TypeUtils.noExpectedType; @@ -124,9 +126,9 @@ public class ControlFlowInformationProvider { markUnusedExpressions(); - markIfWithoutElse(); + checkIfExpressions(); - markWhenWithoutElse(); + checkWhenExpressions(); } public void checkFunction(@Nullable KotlinType expectedReturnType) { @@ -691,7 +693,7 @@ public class ControlFlowInformationProvider { ); } - public void markIfWithoutElse() { + public void checkIfExpressions() { PseudocodeTraverserKt.traverse( pseudocode, TraversalOrder.FORWARD, new ControlFlowInformationProvider.FunctionVoid1() { @Override @@ -725,7 +727,7 @@ public class ControlFlowInformationProvider { @NotNull Collection branchExpressions ) { KotlinType expectedExpressionType = trace.get(EXPECTED_EXPRESSION_TYPE, expression); - if (expectedExpressionType != null) return; + if (expectedExpressionType != null && expectedExpressionType != DONT_CARE) return; KotlinType expressionType = trace.getType(expression); if (expressionType == null) { @@ -733,19 +735,33 @@ public class ControlFlowInformationProvider { } if (KotlinBuiltIns.isAnyOrNullableAny(expressionType)) { for (KtExpression branchExpression : branchExpressions) { + if (branchExpression == null) continue; KotlinType branchType = trace.getType(branchExpression); if (branchType == null || KotlinBuiltIns.isAnyOrNullableAny(branchType)) { return; } } - trace.report(IMPLICIT_CAST_TO_UNIT_OR_ANY.on(expression, expressionType)); - } - else if (KotlinBuiltIns.isUnit(expressionType)) { - trace.report(IMPLICIT_CAST_TO_UNIT_OR_ANY.on(expression, expressionType)); + for (KtExpression branchExpression : branchExpressions) { + if (branchExpression == null) continue; + KotlinType branchType = trace.getType(branchExpression); + assert branchType != null : "Branch expression type should be non-null"; + trace.report(IMPLICIT_CAST_TO_ANY.on(getResultingExpression(branchExpression), branchType, expressionType)); + } } } - public void markWhenWithoutElse() { + private static @NotNull KtExpression getResultingExpression(@NotNull KtExpression expression) { + KtExpression finger = expression; + while (true) { + KtExpression deparenthesized = KtPsiUtil.deparenthesize(finger); + deparenthesized = KtPsiUtil.getExpressionOrLastStatementInBlock(deparenthesized); + if (deparenthesized == null || deparenthesized == finger) break; + finger = deparenthesized; + } + return finger; + } + + public void checkWhenExpressions() { final Map> initializers = pseudocodeVariablesData.getVariableInitializers(); PseudocodeTraverserKt.traverse( pseudocode, TraversalOrder.FORWARD, new ControlFlowInformationProvider.FunctionVoid1() { @@ -776,13 +792,10 @@ public class ControlFlowInformationProvider { KtWhenExpression whenExpression = (KtWhenExpression) element; if (BindingContextUtilsKt.isUsedAsExpression(whenExpression, trace.getBindingContext())) { - List branchExpressions = new ArrayList(whenExpression.getEntries().size() + 1); + List branchExpressions = new ArrayList(whenExpression.getEntries().size()); for (KtWhenEntry whenEntry : whenExpression.getEntries()) { branchExpressions.add(whenEntry.getExpression()); } - if (whenExpression.getElseExpression() != null) { - branchExpressions.add(whenExpression.getElseExpression()); - } checkImplicitCastOnConditionalExpression(whenExpression, branchExpressions); } @@ -806,8 +819,6 @@ public class ControlFlowInformationProvider { } } } - - } } ); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 98de95a19b6..d764d568c27 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -681,7 +681,7 @@ public interface Errors { DiagnosticFactory0 IS_ENUM_ENTRY = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 ENUM_ENTRY_AS_TYPE = DiagnosticFactory0.create(ERROR); - DiagnosticFactory1 IMPLICIT_CAST_TO_UNIT_OR_ANY = DiagnosticFactory1.create(WARNING); + DiagnosticFactory2 IMPLICIT_CAST_TO_ANY = DiagnosticFactory2.create(WARNING); DiagnosticFactory3 SMARTCAST_IMPOSSIBLE = DiagnosticFactory3.create(ERROR); DiagnosticFactory0 ALWAYS_NULL = DiagnosticFactory0.create(WARNING); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 909698ee907..ece479c8db2 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -379,8 +379,8 @@ public class DefaultErrorMessages { MAP.put(EXPECTED_PARAMETER_TYPE_MISMATCH, "Expected parameter of type {0}", RENDER_TYPE); MAP.put(EXPECTED_PARAMETERS_NUMBER_MISMATCH, "Expected {0,choice,0#no parameters|1#one parameter of type|1<{0,number,integer} parameters of types} {1}", null, RENDER_COLLECTION_OF_TYPES); - MAP.put(IMPLICIT_CAST_TO_UNIT_OR_ANY, "Type is cast to ''{0}''. Please specify ''{0}'' as expected type, if you mean such cast", - RENDER_TYPE); + MAP.put(IMPLICIT_CAST_TO_ANY, "Conditional branch result of type {0} is implicitly cast to {1}", + RENDER_TYPE, RENDER_TYPE); MAP.put(EXPRESSION_EXPECTED, "{0} is not an expression, and only expressions are allowed here", new Renderer() { @NotNull @Override diff --git a/compiler/testData/diagnostics/tests/controlStructures/ifInResultOfLambda.kt b/compiler/testData/diagnostics/tests/controlStructures/ifInResultOfLambda.kt index 021014c1597..55752e51bbe 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/ifInResultOfLambda.kt +++ b/compiler/testData/diagnostics/tests/controlStructures/ifInResultOfLambda.kt @@ -1,6 +1,6 @@ -val test1 = { if (true) 1 else "" } +val test1 = { if (true) 1 else "" } -val test2 = { { if (true) 1 else "" } } +val test2 = { { if (true) 1 else "" } } val test3: (Boolean) -> Any = { if (it) 1 else "" } diff --git a/compiler/testData/diagnostics/tests/controlStructures/ifWhenWithoutElse.kt b/compiler/testData/diagnostics/tests/controlStructures/ifWhenWithoutElse.kt index 97bf88b8ecc..f3c625fb63b 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/ifWhenWithoutElse.kt +++ b/compiler/testData/diagnostics/tests/controlStructures/ifWhenWithoutElse.kt @@ -46,7 +46,7 @@ fun f3() = if (true) 42 fun f4(): Unit = if (true) work() fun f5(): Unit = if (true) mlist.add() fun f6(): Unit = if (true) 42 -fun g1() = when { true -> work() } +fun g1() = when { true -> work() } fun g2() = when { true -> mlist.add() } fun g3() = when { true -> 42 } fun g4(): Unit = when { true -> work() } diff --git a/compiler/testData/diagnostics/tests/controlStructures/improperElseInExpression.kt b/compiler/testData/diagnostics/tests/controlStructures/improperElseInExpression.kt index 90fc5ecfccf..6fd3a30c4d8 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/improperElseInExpression.kt +++ b/compiler/testData/diagnostics/tests/controlStructures/improperElseInExpression.kt @@ -5,8 +5,8 @@ fun example() { val b = if (true) else false val c = if (true) true val d = if (true) true else; - val e = if (true) {} else false - val f = if (true) true else {} + val e = if (true) {} else false + val f = if (true) true else {} { if (true) true @@ -17,12 +17,12 @@ fun example() { }(); { - if (true) {} else false + if (true) {} else false }(); { - if (true) true else {} + if (true) true else {} }() fun t(): Boolean { diff --git a/compiler/testData/diagnostics/tests/controlStructures/kt10322.kt b/compiler/testData/diagnostics/tests/controlStructures/kt10322.kt new file mode 100644 index 00000000000..36f0ddca628 --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlStructures/kt10322.kt @@ -0,0 +1,12 @@ +fun run(block: () -> T) : T = block() + +fun test1() { + run { + if (true) { + if (true) {} + } + else { + 1 + } + } +} diff --git a/compiler/testData/diagnostics/tests/controlStructures/kt10322.txt b/compiler/testData/diagnostics/tests/controlStructures/kt10322.txt new file mode 100644 index 00000000000..5ef0eb35ede --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlStructures/kt10322.txt @@ -0,0 +1,4 @@ +package + +public fun run(/*0*/ block: () -> T): T +public fun test1(): kotlin.Unit diff --git a/compiler/testData/diagnostics/tests/controlStructures/kt770.kt351.kt735_StatementType.kt b/compiler/testData/diagnostics/tests/controlStructures/kt770.kt351.kt735_StatementType.kt index a5d25e3c119..4cb36becfa1 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/kt770.kt351.kt735_StatementType.kt +++ b/compiler/testData/diagnostics/tests/controlStructures/kt770.kt351.kt735_StatementType.kt @@ -19,12 +19,12 @@ val w = while (true) {} fun foo() { var z = 2 val r = { // type fun(): Any is inferred - if (true) { - 2 + if (true) { + 2 } else { - z = 34 - } + z = 34 + } } val f: ()-> Int = r val g: ()-> Any = r @@ -73,11 +73,11 @@ fun testCoercionToUnit() { var x = 43 val checkType = { - if (true) { - x = 4 + if (true) { + x = 4 } else { - 45 - } + 45 + } } val f : () -> String = checkType } @@ -93,18 +93,18 @@ fun testImplicitCoercion() { else -> z = 20 } - var u = when(d) { + var u = when(d) { 3 -> { - z = 34 + z = 34 } - else -> z-- - } + else -> z-- + } var iff = if (true) { z = 34 } val g = if (true) 4 - val h = if (false) 4 else {} + val h = if (false) 4 else {} bar(if (true) { 4 diff --git a/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.kt b/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.kt index 92fbc2e2069..c1510a67e1d 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.kt +++ b/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.kt @@ -1,7 +1,16 @@ -val test1 = { when (true) { true -> 1; else -> "" } } +val test1 = { when (true) { true -> 1; else -> "" } } -val test2 = { { when (true) { true -> 1; else -> "" } } } +val test2 = { { when (true) { true -> 1; else -> "" } } } val test3: (Boolean) -> Any = { when (true) { true -> 1; else -> "" } } val test4: (Boolean) -> Any? = { when (true) { true -> 1; else -> "" } } + +fun println() {} + +val test5 = { + when (true) { + true -> println() + else -> println() + } +} diff --git a/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.txt b/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.txt index 8c178e5b829..c5cd166a483 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.txt +++ b/compiler/testData/diagnostics/tests/controlStructures/whenInResultOfLambda.txt @@ -4,3 +4,5 @@ public val test1: () -> kotlin.Any public val test2: () -> () -> kotlin.Any public val test3: (kotlin.Boolean) -> kotlin.Any public val test4: (kotlin.Boolean) -> kotlin.Any? +public val test5: () -> kotlin.Unit +public fun println(): kotlin.Unit diff --git a/compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.kt b/compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.kt new file mode 100644 index 00000000000..b5614aff8ba --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.kt @@ -0,0 +1,22 @@ +var longWords = 0 +val smallWords = hashSetOf() + +fun test1(word: String) = + run { + if (word.length > 4) { + longWords++ + } + else { + smallWords.add(word) + } + } + +fun test2(word: String) = + run { + if (word.length > 4) { + if (word.startsWith("a")) longWords++ + } + else { + smallWords.add(word) + } + } \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.txt b/compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.txt new file mode 100644 index 00000000000..7073d05935c --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.txt @@ -0,0 +1,6 @@ +package + +public var longWords: kotlin.Int +public val smallWords: java.util.HashSet +public fun test1(/*0*/ word: kotlin.String): kotlin.Any +public fun test2(/*0*/ word: kotlin.String): kotlin.Any diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 2bc30b4490c..5a617cf3625 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -3342,6 +3342,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("kt10322.kt") + public void testKt10322() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlStructures/kt10322.kt"); + doTest(fileName); + } + @TestMetadata("kt1075.kt") public void testKt1075() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlStructures/kt1075.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java index 8007bb0467c..1607be5252a 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java @@ -71,6 +71,12 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW doTest(fileName); } + @TestMetadata("implicitCastToAny.kt") + public void testImplicitCastToAny() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/implicitCastToAny.kt"); + doTest(fileName); + } + @TestMetadata("instar.kt") public void testInstar() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/instar.kt");