Separate UNSAFE_IMPLICIT_INVOKE_CALL diagnostics introduced (see KT-8252)

mglukhikh · mglukhikh · commit 6157ebe3b809 · 2016-01-15T17:22:54.000+03:00
diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java
@@ -646,6 +646,7 @@ enum BadNamedArgumentsTarget {
     // Nullability
 
     DiagnosticFactory1<PsiElement, KotlinType> UNSAFE_CALL = DiagnosticFactory1.create(ERROR);
+    DiagnosticFactory1<PsiElement, KotlinType> UNSAFE_IMPLICIT_INVOKE_CALL = DiagnosticFactory1.create(ERROR);
     DiagnosticFactory3<KtExpression, String, String, String> UNSAFE_INFIX_CALL = DiagnosticFactory3.create(ERROR);
     DiagnosticFactory1<PsiElement, KotlinType> UNNECESSARY_SAFE_CALL = DiagnosticFactory1.create(WARNING);
     DiagnosticFactory0<PsiElement> UNEXPECTED_SAFE_CALL = DiagnosticFactory0.create(ERROR);
diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java
@@ -469,6 +469,7 @@ public String render(@NotNull KtExpression expression) {
         MAP.put(DYNAMIC_SUPERTYPE, "A supertype cannot be dynamic");
         MAP.put(REDUNDANT_NULLABLE, "Redundant '?'");
         MAP.put(UNSAFE_CALL, "Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type {0}", RENDER_TYPE);
+        MAP.put(UNSAFE_IMPLICIT_INVOKE_CALL, "Reference has a nullable type {0}, use explicit '?.invoke()' to make function-like call instead", RENDER_TYPE);
         MAP.put(AMBIGUOUS_LABEL, "Ambiguous label");
         MAP.put(UNSUPPORTED, "Unsupported [{0}]", STRING);
         MAP.put(EXCEPTION_FROM_ANALYZER, "Internal Error occurred while analyzing this expression:\n{0}", THROWABLE);
diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt
@@ -464,13 +464,15 @@ class CandidateResolver(
         val dataFlowValue = DataFlowValueFactory.createDataFlowValue(receiverArgument, this)
         val nullability = dataFlowInfo.getPredictableNullability(dataFlowValue)
         var nullableImplicitInvokeReceiver = false
+        var receiverArgumentType = receiverArgument.type
         if (implicitInvokeCheck && call is CallForImplicitInvoke && call.isSafeCall()) {
             val outerCallReceiver = call.outerCall.explicitReceiver
             if (outerCallReceiver != call.explicitReceiver && outerCallReceiver is ReceiverValue) {
                 val outerReceiverDataFlowValue = DataFlowValueFactory.createDataFlowValue(outerCallReceiver, this)
                 val outerReceiverNullability = dataFlowInfo.getPredictableNullability(outerReceiverDataFlowValue)
                 if (outerReceiverNullability.canBeNull() && !TypeUtils.isNullableType(expectedReceiverParameterType)) {
                     nullableImplicitInvokeReceiver = true
+                    receiverArgumentType = TypeUtils.makeNullable(receiverArgumentType)
                 }
             }
         }
@@ -499,8 +501,6 @@ class CandidateResolver(
             }
         }
 
-        val receiverArgumentType = receiverArgument.type
-
         if (reportUnsafeCall || nullableImplicitInvokeReceiver) {
             tracing.unsafeCall(trace, receiverArgumentType, implicitInvokeCheck)
             return UNSAFE_CALL_ERROR
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
@@ -180,6 +180,9 @@ public void unsafeCall(@NotNull BindingTrace trace, @NotNull KotlinType type, bo
                     trace.report(UNSAFE_INFIX_CALL.on(reference, left.getText(), operationString.asString(), right.getText()));
                 }
             }
+            else if (isCallForImplicitInvoke) {
+                trace.report(UNSAFE_IMPLICIT_INVOKE_CALL.on(reference, type));
+            }
             else {
                 trace.report(UNSAFE_CALL.on(reference, type));
             }
diff --git a/compiler/testData/diagnostics/tests/SafeCallInvoke.kt b/compiler/testData/diagnostics/tests/SafeCallInvoke.kt
@@ -13,7 +13,7 @@ fun foo() {
     rule?.apply?.invoke()
 
     // this should be an error
-    rule?.<!UNSAFE_CALL!>apply<!>()
+    rule?.<!UNSAFE_IMPLICIT_INVOKE_CALL!>apply<!>()
 
     // these both also ok (with smart cast / unnecessary safe call)
     if (rule != null) {
diff --git a/compiler/testData/diagnostics/tests/extensions/kt1875.kt b/compiler/testData/diagnostics/tests/extensions/kt1875.kt
@@ -9,13 +9,13 @@ interface T {
 }
 
 fun test(t: T) {
-    t.<!UNSAFE_CALL!>f<!>(1) //unsafe call error
+    t.<!UNSAFE_IMPLICIT_INVOKE_CALL!>f<!>(1) //unsafe call error
     t.f?.invoke(1)
 }
 
 fun test1(t: T?) {
     t<!UNSAFE_CALL!>.<!><!FUNCTION_EXPECTED!>f<!>(1) // todo resolve f as value and report UNSAFE_CALL
-    t?.<!UNSAFE_CALL!>f<!>(1)
+    t?.<!UNSAFE_IMPLICIT_INVOKE_CALL!>f<!>(1)
     t<!UNSAFE_CALL!>.<!>f?.invoke(1)
     t?.f?.invoke(1)
 }
diff --git a/compiler/testData/diagnostics/tests/generics/nullability/functionalBound.kt b/compiler/testData/diagnostics/tests/generics/nullability/functionalBound.kt
@@ -1,6 +1,6 @@
 fun <E : String?, T : ((CharSequence) -> Unit)?> foo(x: E, y: T) {
     if (x != null) {
-        <!UNSAFE_CALL!>y<!>(<!DEBUG_INFO_SMARTCAST!>x<!>)
+        <!UNSAFE_IMPLICIT_INVOKE_CALL!>y<!>(<!DEBUG_INFO_SMARTCAST!>x<!>)
     }
 
     if (y != null) {
diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/invoke.kt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/invoke.kt
@@ -18,6 +18,6 @@ public class J {
 
 fun test() {
     J.staticNN()
-    J.<!UNSAFE_CALL!>staticN<!>()
+    J.<!UNSAFE_IMPLICIT_INVOKE_CALL!>staticN<!>()
     J.staticJ()
 }
diff --git a/compiler/testData/diagnostics/tests/resolve/invoke/invokeAndSmartCast.kt b/compiler/testData/diagnostics/tests/resolve/invoke/invokeAndSmartCast.kt
@@ -3,18 +3,18 @@ class A(val x: (String.() -> Unit)?)
 fun test(a: A) {
     if (a.x != null) {
         "".<!DEBUG_INFO_SMARTCAST!>(a.x)<!>()
-        a.<!UNSAFE_CALL!>x<!>("") // todo
+        a.<!UNSAFE_IMPLICIT_INVOKE_CALL!>x<!>("") // todo
         <!DEBUG_INFO_SMARTCAST!>(a.x)<!>("")
     }
-    "".<!UNSAFE_CALL!>(a.x)<!>()
-    a.<!UNSAFE_CALL!>x<!>("")
-    <!UNSAFE_CALL!>(a.x)<!>("")
+    "".<!UNSAFE_IMPLICIT_INVOKE_CALL!>(a.x)<!>()
+    a.<!UNSAFE_IMPLICIT_INVOKE_CALL!>x<!>("")
+    <!UNSAFE_IMPLICIT_INVOKE_CALL!>(a.x)<!>("")
 
     with("") {
-        a.<!UNSAFE_CALL!>x<!>(<!NO_VALUE_FOR_PARAMETER!>)<!>
-        <!UNSAFE_CALL!>(a.x)<!>()
+        a.<!UNSAFE_IMPLICIT_INVOKE_CALL!>x<!>(<!NO_VALUE_FOR_PARAMETER!>)<!>
+        <!UNSAFE_IMPLICIT_INVOKE_CALL!>(a.x)<!>()
         if (a.x != null) {
-            a.<!UNSAFE_CALL!>x<!>(<!NO_VALUE_FOR_PARAMETER!>)<!> // todo
+            a.<!UNSAFE_IMPLICIT_INVOKE_CALL!>x<!>(<!NO_VALUE_FOR_PARAMETER!>)<!> // todo
             <!DEBUG_INFO_SMARTCAST!>(a.x)<!>()
         }
     }
diff --git a/idea/testData/checker/SafeInvoke.kt b/idea/testData/checker/SafeInvoke.kt
@@ -0,0 +1,8 @@
+class Rule(val apply: () -> Unit)
+
+fun foo() {
+    val rule: Rule? = Rule { }
+    rule?.<error descr="[UNSAFE_IMPLICIT_INVOKE_CALL] Reference has a nullable type (() -> kotlin.Unit)?, use explicit ?.invoke() to make function-like call instead">apply</error>()
+    val apply = rule?.apply
+    <error descr="[UNSAFE_IMPLICIT_INVOKE_CALL] Reference has a nullable type (() -> kotlin.Unit)?, use explicit ?.invoke() to make function-like call instead">apply</error>()
+}
diff --git a/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java b/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java
@@ -295,6 +295,12 @@ public void testReturnTypeMismatchOnOverride() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("SafeInvoke.kt")
+        public void testSafeInvoke() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/checker/SafeInvoke.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("Shadowing.kt")
         public void testShadowing() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/checker/Shadowing.kt");

Original file line number	Diff line number	Diff line change
`@@ -464,13 +464,15 @@ class CandidateResolver(`
`464`	`464`	`val dataFlowValue = DataFlowValueFactory.createDataFlowValue(receiverArgument, this)`
`465`	`465`	`val nullability = dataFlowInfo.getPredictableNullability(dataFlowValue)`
`466`	`466`	`var nullableImplicitInvokeReceiver = false`
	`467`	`+ var receiverArgumentType = receiverArgument.type`
`467`	`468`	`if (implicitInvokeCheck && call is CallForImplicitInvoke && call.isSafeCall()) {`
`468`	`469`	`val outerCallReceiver = call.outerCall.explicitReceiver`
`469`	`470`	`if (outerCallReceiver != call.explicitReceiver && outerCallReceiver is ReceiverValue) {`
`470`	`471`	`val outerReceiverDataFlowValue = DataFlowValueFactory.createDataFlowValue(outerCallReceiver, this)`
`471`	`472`	`val outerReceiverNullability = dataFlowInfo.getPredictableNullability(outerReceiverDataFlowValue)`
`472`	`473`	`if (outerReceiverNullability.canBeNull() && !TypeUtils.isNullableType(expectedReceiverParameterType)) {`
`473`	`474`	`nullableImplicitInvokeReceiver = true`
	`475`	`+ receiverArgumentType = TypeUtils.makeNullable(receiverArgumentType)`
`474`	`476`	`}`
`475`	`477`	`}`
`476`	`478`	`}`
`@@ -499,8 +501,6 @@ class CandidateResolver(`
`499`	`501`	`}`
`500`	`502`	`}`
`501`	`503`
`502`		`- val receiverArgumentType = receiverArgument.type`
`503`		`-`
`504`	`504`	`if (reportUnsafeCall \|\| nullableImplicitInvokeReceiver) {`
`505`	`505`	`tracing.unsafeCall(trace, receiverArgumentType, implicitInvokeCheck)`
`506`	`506`	`return UNSAFE_CALL_ERROR`
Original file line number	Diff line number	Diff line change
`@@ -180,6 +180,9 @@ public void unsafeCall(@NotNull BindingTrace trace, @NotNull KotlinType type, bo`
`180`	`180`	`trace.report(UNSAFE_INFIX_CALL.on(reference, left.getText(), operationString.asString(), right.getText()));`
`181`	`181`	`}`
`182`	`182`	`}`
	`183`	`+ else if (isCallForImplicitInvoke) {`
	`184`	`+ trace.report(UNSAFE_IMPLICIT_INVOKE_CALL.on(reference, type));`
	`185`	`+ }`
`183`	`186`	`else {`
`184`	`187`	`trace.report(UNSAFE_CALL.on(reference, type));`
`185`	`188`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,13 +9,13 @@ interface T {`
`9`	`9`	`}`
`10`	`10`
`11`	`11`	`fun test(t: T) {`
`12`		`- t.<!UNSAFE_CALL!>f<!>(1) //unsafe call error`
	`12`	`+ t.<!UNSAFE_IMPLICIT_INVOKE_CALL!>f<!>(1) //unsafe call error`
`13`	`13`	`t.f?.invoke(1)`
`14`	`14`	`}`
`15`	`15`
`16`	`16`	`fun test1(t: T?) {`
`17`	`17`	`t<!UNSAFE_CALL!>.<!><!FUNCTION_EXPECTED!>f<!>(1) // todo resolve f as value and report UNSAFE_CALL`
`18`		`- t?.<!UNSAFE_CALL!>f<!>(1)`
	`18`	`+ t?.<!UNSAFE_IMPLICIT_INVOKE_CALL!>f<!>(1)`
`19`	`19`	`t<!UNSAFE_CALL!>.<!>f?.invoke(1)`
`20`	`20`	`t?.f?.invoke(1)`
`21`	`21`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`fun <E : String?, T : ((CharSequence) -> Unit)?> foo(x: E, y: T) {`
`2`	`2`	`if (x != null) {`
`3`		`- <!UNSAFE_CALL!>y<!>(<!DEBUG_INFO_SMARTCAST!>x<!>)`
	`3`	`+ <!UNSAFE_IMPLICIT_INVOKE_CALL!>y<!>(<!DEBUG_INFO_SMARTCAST!>x<!>)`
`4`	`4`	`}`
`5`	`5`
`6`	`6`	`if (y != null) {`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,6 @@ public class J {`
`18`	`18`
`19`	`19`	`fun test() {`
`20`	`20`	`J.staticNN()`
`21`		`- J.<!UNSAFE_CALL!>staticN<!>()`
	`21`	`+ J.<!UNSAFE_IMPLICIT_INVOKE_CALL!>staticN<!>()`
`22`	`22`	`J.staticJ()`
`23`	`23`	`}`