Report warnings on safe call + nullable extension operator

^KT-41034 In Progress

compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java

@@ -15821,6 +15821,11 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirOldFronte
             runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOnTypeWithNullableUpperBound.kt");
         }
 
+        @TestMetadata("safeCallOperators.kt")
+        public void testSafeCallOperators() throws Exception {
+            runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt");
+        }
+
         @TestMetadata("safeCallWithInvoke.kt")
         public void testSafeCallWithInvoke() throws Exception {
             runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallWithInvoke.kt");

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java

@@ -901,6 +901,8 @@ public interface Errors {
     DiagnosticFactory1<PsiElement, CallableDescriptor> DSL_SCOPE_VIOLATION = DiagnosticFactory1.create(ERROR);
     DiagnosticFactory1<PsiElement, CallableDescriptor> DSL_SCOPE_VIOLATION_WARNING = DiagnosticFactory1.create(WARNING);
 
+    DiagnosticFactory0<PsiElement> NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER = DiagnosticFactory0.create(WARNING);
+
     // Labels
 
     DiagnosticFactory0<KtSimpleNameExpression> LABEL_NAME_CLASH = DiagnosticFactory0.create(WARNING);

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java

@@ -528,6 +528,9 @@ public class DefaultErrorMessages {
         MAP.put(DSL_SCOPE_VIOLATION_WARNING, "''{0}'' shouldn't be called in this context by implicit receiver, it will become an error soon. " +
                                      "Use the explicit one if necessary", COMPACT);
 
+        MAP.put(NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER, "Semantics of such combination of safe call and operator will change in next compiler version. " +
+                                     "Namely, the right part of the safe call will not be evaluated if receiver is null");
+
         MAP.put(RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY,
                 "Returns are not allowed for functions with expression body. Use block body in '{...}'");
         MAP.put(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY, "A 'return' expression required in a function with a block body ('{...}')");

compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt

@@ -51,7 +51,7 @@ private val DEFAULT_CALL_CHECKERS = listOf(
     UselessElvisCallChecker(), ResultTypeWithNullableOperatorsChecker(), NullableVarargArgumentCallChecker,
     NamedFunAsExpressionChecker, ContractNotAllowedCallChecker, ReifiedTypeParameterSubstitutionChecker(),
     MissingDependencySupertypeChecker.ForCalls, AbstractClassInstantiationChecker, SuspendConversionCallChecker,
-    UnitConversionCallChecker, FunInterfaceConstructorReferenceChecker
+    UnitConversionCallChecker, FunInterfaceConstructorReferenceChecker, NullableExtensionOperatorWithSafeCallChecker
 )
 private val DEFAULT_TYPE_CHECKERS = emptyList<AdditionalTypeChecker>()
 private val DEFAULT_CLASSIFIER_USAGE_CHECKERS = listOf(

compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/NullableExtensionOperatorWithSafeCallChecker.kt

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.resolve.checkers
+
+import com.intellij.psi.PsiElement
+import org.jetbrains.kotlin.diagnostics.Errors
+import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtNameReferenceExpression
+import org.jetbrains.kotlin.psi.KtParenthesizedExpression
+import org.jetbrains.kotlin.psi.KtSafeQualifiedExpression
+import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker
+import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext
+import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
+import org.jetbrains.kotlin.resolve.calls.model.isReallySuccess
+import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver
+import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue
+import org.jetbrains.kotlin.types.isFlexible
+import org.jetbrains.kotlin.types.isNullable
+import org.jetbrains.kotlin.util.OperatorNameConventions
+
+object NullableExtensionOperatorWithSafeCallChecker : CallChecker {
+    private val RELEVANT_OPERATORS = mutableSetOf<Name>().apply {
+        addAll(OperatorNameConventions.ASSIGNMENT_OPERATIONS)
+        add(OperatorNameConventions.INC)
+        add(OperatorNameConventions.DEC)
+        add(OperatorNameConventions.GET)
+        add(OperatorNameConventions.SET)
+    }
+
+    override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) {
+        if (!resolvedCall.isReallySuccess()) return
+        val name = resolvedCall.resultingDescriptor.name
+        if (name !in RELEVANT_OPERATORS) return
+
+        if (!isNullableSafeCallReceiver(resolvedCall.extensionReceiver)) return
+
+        val callElement = resolvedCall.call.callElement
+
+        // It's an operator call, not a regular one
+        if (callElement is KtCallExpression && name.identifier == (callElement.calleeExpression as? KtNameReferenceExpression)?.getReferencedName()) return
+
+        context.trace.report(Errors.NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER.on(reportOn))
+    }
+
+    private fun isNullableSafeCallReceiver(receiverValue: ReceiverValue?): Boolean {
+        if (receiverValue !is ExpressionReceiver) return false
+        if (!receiverValue.type.isNullable() || receiverValue.type.isFlexible()) return false
+
+        val expression = receiverValue.expression
+
+        if (expression !is KtSafeQualifiedExpression) return false
+        if (expression.parent is KtParenthesizedExpression) return false
+
+        return true
+    }
+}

compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.fir.kt

@@ -0,0 +1,91 @@
+// SKIP_TXT
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+class A(
+    val l: MutableList<Int>,
+    val ll: MutableList<MutableList<Int>>,
+    var w: Int,
+    val q: () -> Unit
+)
+
+operator fun <T> MutableList<in T>?.plusAssign(t: Any?) {}
+operator fun <T> MutableList<T>?.set(t: Int, w: Any?) {}
+operator fun <T> List<T>?.get(t: Int): T? = this?.get(t)
+operator fun Int?.inc(): Int = 1
+operator fun (() -> Unit)?.invoke() {}
+
+fun foo(a: A?) {
+    a?.l += 1
+    a?.l[0]
+    a?.l[0]++
+    a?.l[0] = 1
+
+    a?.ll[0][0]
+    a?.ll[0][0]++
+    a?.ll[0][0] = 1
+    // No warning is reported because
+    // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway
+    // 2. In case of null value, the behavior is intended (no call performed)
+    a?.q()
+    a?.w++
+
+    (a?.l) += 1
+    (a?.l)[0]
+    (a?.l)[0]++
+    (a?.l)[0] = 1
+
+    (a?.ll)[0][0]
+    (a?.ll)[0][0]++
+    (a?.ll)[0][0] = 1
+    (a?.q)()
+    (a?.w)++
+
+    a?.l.plusAssign(1)
+    a?.l.get(0)
+    a?.l.get(0).inc()
+    a?.l.set(0, 1)
+
+    a?.ll.get(0).get(0)
+    a?.ll.get(0).get(0).inc()
+    a?.ll.get(0).set(0, 1)
+    a?.q.invoke()
+    a?.w.inc()
+
+    if (a != null) {
+        a?.l += 1
+        a?.l[0]
+        a?.l[0]++
+        a?.l[0] = 1
+
+        a?.ll[0][0]
+        a?.ll[0][0]++
+        a?.ll[0][0] = 1
+        // No warning is reported because
+        // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway
+        // 2. In case of null value, the behavior is intended (no call performed)
+        a?.q()
+        a?.w++
+
+        (a?.l) += 1
+        (a?.l)[0]
+        (a?.l)[0]++
+        (a?.l)[0] = 1
+
+        (a?.ll)[0][0]
+        (a?.ll)[0][0]++
+        (a?.ll)[0][0] = 1
+        (a?.q)()
+        (a?.w)++
+
+        a?.l.plusAssign(1)
+        a?.l.get(0)
+        a?.l.get(0).inc()
+        a?.l.set(0, 1)
+
+        a?.ll.get(0).get(0)
+        a?.ll.get(0).get(0).inc()
+        a?.ll.get(0).set(0, 1)
+        a?.q.invoke()
+        a?.w.inc()
+    }
+}

compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt

@@ -0,0 +1,91 @@
+// SKIP_TXT
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+class A(
+    val l: MutableList<Int>,
+    val ll: MutableList<MutableList<Int>>,
+    var w: Int,
+    val q: () -> Unit
+)
+
+operator fun <T> MutableList<in T>?.plusAssign(t: Any?) {}
+operator fun <T> MutableList<T>?.set(t: Int, w: Any?) {}
+operator fun <T> List<T>?.get(t: Int): T? = this?.get(t)
+operator fun Int?.inc(): Int = 1
+operator fun (() -> Unit)?.invoke() {}
+
+fun foo(a: A?) {
+    a?.l <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>+=<!> 1
+    <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>a?.l[0]<!>
+    <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER, NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>a?.l[0]<!>++
+    <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>a?.l[0]<!> = 1
+
+    <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>a?.ll[0]<!>[0]
+    <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>a?.ll[0]<!>[0]++
+    <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>a?.ll[0]<!>[0] = 1
+    // No warning is reported because
+    // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway
+    // 2. In case of null value, the behavior is intended (no call performed)
+    a?.q()
+    a?.w<!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>++<!>
+
+    (a?.l) += 1
+    (a?.l)[0]
+    (a?.l)[0]++
+    (a?.l)[0] = 1
+
+    (a?.ll)[0][0]
+    (a?.ll)[0][0]++
+    (a?.ll)[0][0] = 1
+    (a?.q)()
+    (a?.w)++
+
+    a?.l.plusAssign(1)
+    a?.l.get(0)
+    a?.l.get(0).inc()
+    a?.l.set(0, 1)
+
+    a?.ll.get(0).get(0)
+    a?.ll.get(0).get(0).inc()
+    a?.ll.get(0).set(0, 1)
+    a?.q.invoke()
+    a?.w.inc()
+
+    if (a != null) {
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l += 1
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l[0]
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l[0]++
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l[0] = 1
+
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>ll[0][0]
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>ll[0][0]++
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>ll[0][0] = 1
+        // No warning is reported because
+        // 1. All kinds of green code with safe+call + invoke we identified fails with CCE if `a != null`, anyway
+        // 2. In case of null value, the behavior is intended (no call performed)
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>q()
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>w++
+
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>l) += 1
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>l)[0]
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>l)[0]++
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>l)[0] = 1
+
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>ll)[0][0]
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>ll)[0][0]++
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>ll)[0][0] = 1
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>q)()
+        (a<!UNNECESSARY_SAFE_CALL!>?.<!>w)++
+
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l.plusAssign(1)
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l.get(0)
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l.get(0).inc()
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>l.set(0, 1)
+
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>ll.get(0).get(0)
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>ll.get(0).get(0).inc()
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>ll.get(0).set(0, 1)
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>q.invoke()
+        a<!UNNECESSARY_SAFE_CALL!>?.<!>w.inc()
+    }
+}

compiler/tests-spec/testData/diagnostics/linked/overload-resolution/building-the-overload-candidate-set-ocs/operator-call/p-1/pos/2.1.kt

@@ -54,15 +54,15 @@ package testPackCase2
 
 fun case2(a: A?, c: C) {
 
-    <!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>a?.b += c<!>
+    <!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>a?.b <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>+=<!> c<!>
     a?.b.<!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>plusAssign(c)<!>
 
     val x = {
-        <!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>a?.b += c<!>
+        <!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>a?.b <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>+=<!> c<!>
         a?.b.<!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>plusAssign(c)<!>
     }()
 
-    <!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>a?.b += { c }()<!>
+    <!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>a?.b <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>+=<!> { c }()<!>
 
     a?.b.<!DEBUG_INFO_CALL("fqName: testPackCase2.plusAssign; typeCall: operator extension function")!>plusAssign({ c }())<!>
 
@@ -92,15 +92,15 @@ package testPackCase3
 
 fun case3(a: A?, c: C) {
 
-    <!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>a?.b += c<!>
+    <!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>a?.b <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>+=<!> c<!>
     a?.b.<!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>plusAssign(c)<!>
 
     val x = {
-        <!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>a?.b += c<!>
+        <!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>a?.b <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>+=<!> c<!>
         a?.b.<!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>plusAssign(c)<!>
     }()
 
-    <!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>a?.b += { c }()<!>
+    <!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>a?.b <!NULLABLE_EXTENSION_OPERATOR_WITH_SAFE_CALL_RECEIVER!>+=<!> { c }()<!>
 
     a?.b.<!DEBUG_INFO_CALL("fqName: testPackCase3.plusAssign; typeCall: operator extension function")!>plusAssign({ c }())<!>
 

compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java

@@ -15828,6 +15828,11 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTestWithFirVali
                 runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOnTypeWithNullableUpperBound.kt");
             }
 
+            @TestMetadata("safeCallOperators.kt")
+            public void testSafeCallOperators() throws Exception {
+                runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt");
+            }
+
             @TestMetadata("safeCallWithInvoke.kt")
             public void testSafeCallWithInvoke() throws Exception {
                 runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallWithInvoke.kt");

compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java

@@ -15823,6 +15823,11 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing
                 runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOnTypeWithNullableUpperBound.kt");
             }
 
+            @TestMetadata("safeCallOperators.kt")
+            public void testSafeCallOperators() throws Exception {
+                runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallOperators.kt");
+            }
+
             @TestMetadata("safeCallWithInvoke.kt")
             public void testSafeCallWithInvoke() throws Exception {
                 runTest("compiler/testData/diagnostics/tests/nullableTypes/safeCallWithInvoke.kt");