From b53a3b324fe6faeec6fbfb50a62a14377554aa78 Mon Sep 17 00:00:00 2001 From: Mikhail Zarechenskiy Date: Mon, 19 Jun 2017 03:44:10 +0300 Subject: [PATCH] Fix 'infix call' diagnostic for `in` operation #KT-8845 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 3 +- .../rendering/DefaultErrorMessages.java | 5 ++- .../calls/tasks/AbstractTracingStrategy.java | 39 ++++++++++++------ .../tests/BinaryCallsOnNullableValues.kt | 40 +++++++++---------- .../InfixCallNullability.kt | 14 +++---- .../tests/nullabilityAndSmartCasts/kt2216.kt | 6 +-- .../nullabilityWarnings/arithmetic.kt | 4 +- .../diagnostics/tests/regressions/kt282.kt | 4 +- .../specialConstructions/elvisAsCall.kt | 4 +- .../smartCasts/objectLiterals/exclexcl.kt | 2 +- .../kotlin/generators/tests/GenerateTests.kt | 1 + .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 4 ++ .../operatorCallDiagnosticsOnInOperator.kt | 8 ++++ .../checkers/PsiCheckerTestGenerated.java | 15 +++++++ 14 files changed, 97 insertions(+), 52 deletions(-) create mode 100644 idea/testData/checker/diagnosticsMessage/operatorCallDiagnosticsOnInOperator.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 4639a35e169..19f4cc86006 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -784,7 +784,8 @@ public interface Errors { DiagnosticFactory1 UNSAFE_CALL = DiagnosticFactory1.create(ERROR); DiagnosticFactory1 UNSAFE_IMPLICIT_INVOKE_CALL = DiagnosticFactory1.create(ERROR); - DiagnosticFactory3 UNSAFE_INFIX_CALL = DiagnosticFactory3.create(ERROR); + DiagnosticFactory3 UNSAFE_INFIX_CALL = DiagnosticFactory3.create(ERROR); + DiagnosticFactory3 UNSAFE_OPERATOR_CALL = DiagnosticFactory3.create(ERROR); DiagnosticFactory1 UNNECESSARY_SAFE_CALL = DiagnosticFactory1.create(WARNING); DiagnosticFactory0 UNEXPECTED_SAFE_CALL = DiagnosticFactory0.create(ERROR); DiagnosticFactory1 UNNECESSARY_NOT_NULL_ASSERTION = DiagnosticFactory1.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 9c51f658de0..1172cef0c06 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -717,7 +717,10 @@ public class DefaultErrorMessages { MAP.put(UNSAFE_INFIX_CALL, "Infix call corresponds to a dot-qualified call ''{0}.{1}({2})'' which is not allowed on a nullable receiver ''{0}''. " + "Use ''?.''-qualified call instead", - STRING, STRING, STRING); + ELEMENT_TEXT, STRING, ELEMENT_TEXT); + MAP.put(UNSAFE_OPERATOR_CALL, + "Operator call corresponds to a dot-qualified call ''{0}.{1}({2})'' which is not allowed on a nullable receiver ''{0}''.", + ELEMENT_TEXT, STRING, ELEMENT_TEXT); MAP.put(OVERLOAD_RESOLUTION_AMBIGUITY, "Overload resolution ambiguity: {0}", AMBIGUOUS_CALLS); MAP.put(NONE_APPLICABLE, "None of the following functions can be called with the arguments supplied: {0}", AMBIGUOUS_CALLS); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/AbstractTracingStrategy.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/AbstractTracingStrategy.java index 960fd477bb0..2ba97de6b22 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/AbstractTracingStrategy.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/AbstractTracingStrategy.java @@ -1,5 +1,5 @@ /* - * Copyright 2010-2016 JetBrains s.r.o. + * Copyright 2010-2017 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -173,18 +173,7 @@ public abstract class AbstractTracingStrategy implements TracingStrategy { else { PsiElement callElement = call.getCallElement(); if (callElement instanceof KtBinaryExpression) { - KtBinaryExpression binaryExpression = (KtBinaryExpression)callElement; - KtSimpleNameExpression operationReference = binaryExpression.getOperationReference(); - - Name operationString = operationReference.getReferencedNameElementType() == KtTokens.IDENTIFIER ? - Name.identifier(operationReference.getText()) : - OperatorConventions.getNameForOperationSymbol((KtToken) operationReference.getReferencedNameElementType()); - - KtExpression left = binaryExpression.getLeft(); - KtExpression right = binaryExpression.getRight(); - if (left != null && right != null) { - trace.report(UNSAFE_INFIX_CALL.on(reference, left.getText(), operationString.asString(), right.getText())); - } + reportUnsafeCallOnBinaryExpression(trace, (KtBinaryExpression) callElement); } else if (isCallForImplicitInvoke) { trace.report(UNSAFE_IMPLICIT_INVOKE_CALL.on(reference, type)); @@ -195,6 +184,30 @@ public abstract class AbstractTracingStrategy implements TracingStrategy { } } + private void reportUnsafeCallOnBinaryExpression(@NotNull BindingTrace trace, @NotNull KtBinaryExpression binaryExpression) { + KtSimpleNameExpression operationReference = binaryExpression.getOperationReference(); + boolean isInfixCall = operationReference.getReferencedNameElementType() == KtTokens.IDENTIFIER; + Name operationString = isInfixCall ? + Name.identifier(operationReference.getText()) : + OperatorConventions.getNameForOperationSymbol((KtToken) operationReference.getReferencedNameElementType()); + + if (operationString == null) return; + + KtExpression left = binaryExpression.getLeft(); + KtExpression right = binaryExpression.getRight(); + if (left == null || right == null) return; + + if (isInfixCall) { + trace.report(UNSAFE_INFIX_CALL.on(reference, left, operationString.asString(), right)); + } + else { + boolean inOperation = KtPsiUtil.isInOrNotInOperation(binaryExpression); + KtExpression receiver = inOperation ? right : left; + KtExpression argument = inOperation ? left : right; + trace.report(UNSAFE_OPERATOR_CALL.on(reference, receiver, operationString.asString(), argument)); + } + } + @Override public void invisibleMember(@NotNull BindingTrace trace, @NotNull DeclarationDescriptorWithVisibility descriptor) { trace.report(INVISIBLE_MEMBER.on(call.getCallElement(), descriptor, descriptor.getVisibility(), descriptor)); diff --git a/compiler/testData/diagnostics/tests/BinaryCallsOnNullableValues.kt b/compiler/testData/diagnostics/tests/BinaryCallsOnNullableValues.kt index 4cdef06567c..d95e000f29f 100644 --- a/compiler/testData/diagnostics/tests/BinaryCallsOnNullableValues.kt +++ b/compiler/testData/diagnostics/tests/BinaryCallsOnNullableValues.kt @@ -1,31 +1,31 @@ class A() { - override fun equals(other : Any?) : Boolean = false + override fun equals(other : Any?) : Boolean = false } fun f(): Unit { - var x: Int? = 1 - x = null - x + 1 - x plus 1 - x < 1 - x += 1 + var x: Int? = 1 + x = null + x + 1 + x plus 1 + x < 1 + x += 1 - x == 1 - x != 1 + x == 1 + x != 1 - A() == 1 + A() == 1 - x === "1" - x !== "1" + x === "1" + x !== "1" - x === 1 - x !== 1 + x === 1 + x !== 1 - x..2 - x in 1..2 + x..2 + x in 1..2 - val y : Boolean? = true - false || y - y && true - y && 1 + val y : Boolean? = true + false || y + y && true + y && 1 } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/InfixCallNullability.kt b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/InfixCallNullability.kt index 0f4790f933a..593f8024677 100644 --- a/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/InfixCallNullability.kt +++ b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/InfixCallNullability.kt @@ -1,7 +1,7 @@ class A() { - operator infix fun plus(i : Int) {} - operator fun unaryMinus() {} - operator infix fun contains(a : Any?) : Boolean = true + operator infix fun plus(i : Int) {} + operator fun unaryMinus() {} + operator infix fun contains(a : Any?) : Boolean = true } operator infix fun A.div(i : Int) {} @@ -10,7 +10,7 @@ operator infix fun A?.times(i : Int) {} fun test(x : Int?, a : A?) { x.plus(1) x?.plus(1) - x + 1 + x + 1 -x x.unaryMinus() x?.unaryMinus() @@ -18,13 +18,13 @@ fun test(x : Int?, a : A?) { a.plus(1) a?.plus(1) a plus 1 - a + 1 + a + 1 -a a.unaryMinus() a?.unaryMinus() a.div(1) - a / 1 + a / 1 a div 1 a?.div(1) @@ -33,7 +33,7 @@ fun test(x : Int?, a : A?) { a times 1 a?.times(1) - 1 in a + 1 in a a contains 1 a.contains(1) a?.contains(1) diff --git a/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/kt2216.kt b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/kt2216.kt index 540ebeca8ad..5d2bab62438 100644 --- a/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/kt2216.kt +++ b/compiler/testData/diagnostics/tests/nullabilityAndSmartCasts/kt2216.kt @@ -7,15 +7,15 @@ fun baz(a: Int, b: Int, c: Int, d: Int) = a + b + c + d fun foo() { val x: Int? = 0 - + bar(if (x != null) x else return, x) x + 2 bar(x, x!!) - + val y: Int? = 0 val z: Int? = 0 bar(if (y != null) y else z, y) - y + 2 + y + 2 baz(y, y, if (y == null) return else y, y) baz(y, z!!, z, y) } diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/arithmetic.kt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/arithmetic.kt index 26ad2b48d8b..3bd7234f6fe 100644 --- a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/arithmetic.kt +++ b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/arithmetic.kt @@ -37,7 +37,7 @@ fun test() { 1 + platformJ platformNN + 1 - platformN + 1 + platformN + 1 platformJ + 1 1 plus platformNN @@ -49,6 +49,6 @@ fun test() { platformJ plus 1 platformNN += 1 - platformN += 1 + platformN += 1 platformJ += 1 } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/regressions/kt282.kt b/compiler/testData/diagnostics/tests/regressions/kt282.kt index f6c2f81433a..014273d384e 100644 --- a/compiler/testData/diagnostics/tests/regressions/kt282.kt +++ b/compiler/testData/diagnostics/tests/regressions/kt282.kt @@ -11,8 +11,8 @@ operator fun Int?.contains(x : Int) : Boolean = false fun f(): Unit { var set : Set? = null val i : Int? = null - i + 1 + i + 1 set + 1 - 1 in set + 1 in set 1 in 2 } diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt b/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt index efb8a58241c..77d1650f60d 100644 --- a/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt @@ -22,13 +22,13 @@ fun testDataFlowInfo1(a: Int?, b: Int?) { val c: Int = a ?: b!! doInt(c) // b is nullable if a != null - b + 1 + b + 1 } fun testDataFlowInfo2(a: Int?, b: Int?) { doInt(a ?: b!!) // b is nullable if a != null - b + 1 + b + 1 } fun testTypeMismatch(a: String?, b: Any) { diff --git a/compiler/testData/diagnostics/tests/smartCasts/objectLiterals/exclexcl.kt b/compiler/testData/diagnostics/tests/smartCasts/objectLiterals/exclexcl.kt index 6f0635d16a3..7cd2a3f5ae9 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/objectLiterals/exclexcl.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/objectLiterals/exclexcl.kt @@ -15,7 +15,7 @@ fun foo(): Int { k.run() val d: Int = c // a is not null because of k constructor, but we do not know it - return a + d + return a + d } else return -1 } diff --git a/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt b/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt index d5ff179660f..3e38fcbeb30 100755 --- a/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt +++ b/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt @@ -498,6 +498,7 @@ fun main(args: Array) { model("checker/scripts", extension = "kts") model("checker/duplicateJvmSignature") model("checker/infos", testMethod = "doTestWithInfos") + model("checker/diagnosticsMessage") } testClass { diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index 29d13253479..53fabf214c2 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -215,18 +215,22 @@ class QuickFixRegistrar : QuickFixContributor { UNSAFE_CALL.registerFactory(SurroundWithNullCheckFix) UNSAFE_IMPLICIT_INVOKE_CALL.registerFactory(SurroundWithNullCheckFix) UNSAFE_INFIX_CALL.registerFactory(SurroundWithNullCheckFix) + UNSAFE_OPERATOR_CALL.registerFactory(SurroundWithNullCheckFix) ITERATOR_ON_NULLABLE.registerFactory(SurroundWithNullCheckFix.IteratorOnNullableFactory) TYPE_MISMATCH.registerFactory(SurroundWithNullCheckFix.TypeMismatchFactory) UNSAFE_CALL.registerFactory(WrapWithSafeLetCallFix.UnsafeFactory) UNSAFE_IMPLICIT_INVOKE_CALL.registerFactory(WrapWithSafeLetCallFix.UnsafeFactory) UNSAFE_INFIX_CALL.registerFactory(WrapWithSafeLetCallFix.UnsafeFactory) + UNSAFE_OPERATOR_CALL.registerFactory(WrapWithSafeLetCallFix.UnsafeFactory) TYPE_MISMATCH.registerFactory(WrapWithSafeLetCallFix.TypeMismatchFactory) UNSAFE_CALL.registerFactory(AddExclExclCallFix) UNSAFE_INFIX_CALL.registerFactory(AddExclExclCallFix) + UNSAFE_OPERATOR_CALL.registerFactory(AddExclExclCallFix) UNNECESSARY_NOT_NULL_ASSERTION.registerFactory(RemoveExclExclCallFix) UNSAFE_INFIX_CALL.registerFactory(ReplaceInfixOrOperatorCallFix) + UNSAFE_OPERATOR_CALL.registerFactory(ReplaceInfixOrOperatorCallFix) UNSAFE_CALL.registerFactory(ReplaceInfixOrOperatorCallFix) // [] only UNSAFE_IMPLICIT_INVOKE_CALL.registerFactory(ReplaceInfixOrOperatorCallFix) UNSAFE_CALL.registerFactory(ReplaceWithSafeCallForScopeFunctionFix) diff --git a/idea/testData/checker/diagnosticsMessage/operatorCallDiagnosticsOnInOperator.kt b/idea/testData/checker/diagnosticsMessage/operatorCallDiagnosticsOnInOperator.kt new file mode 100644 index 00000000000..13ddcf6d0d8 --- /dev/null +++ b/idea/testData/checker/diagnosticsMessage/operatorCallDiagnosticsOnInOperator.kt @@ -0,0 +1,8 @@ +fun call() { + val list : String? = "sdfknsdkfm" + "x" in list + + "x" !in list +} + +operator fun CharSequence.contains(other: CharSequence, ignoreCase: Boolean = false): Boolean = true \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java b/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java index 409e4ddbf6d..1a57b8b867b 100644 --- a/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java @@ -977,4 +977,19 @@ public class PsiCheckerTestGenerated extends AbstractPsiCheckerTest { doTestWithInfos(fileName); } } + + @TestMetadata("idea/testData/checker/diagnosticsMessage") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class DiagnosticsMessage extends AbstractPsiCheckerTest { + public void testAllFilesPresentInDiagnosticsMessage() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/checker/diagnosticsMessage"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("operatorCallDiagnosticsOnInOperator.kt") + public void testOperatorCallDiagnosticsOnInOperator() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/checker/diagnosticsMessage/operatorCallDiagnosticsOnInOperator.kt"); + doTest(fileName); + } + } }