Unnecessary safe call is now reported in CallExpressionResolver, removed code duplication for safe call receivers

See also KT-10175

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ArgumentTypeResolver.java

@@ -320,21 +320,7 @@ public class ArgumentTypeResolver {
     ) {
         MutableDataFlowInfoForArguments infoForArguments = context.dataFlowInfoForArguments;
         Call call = context.call;
-        Receiver receiver = call.getExplicitReceiver();
-        DataFlowInfo initialDataFlowInfo = context.dataFlowInfo;
-        // QualifierReceiver is a thing like Collections. which has no type or value
-        if (receiver.exists() && receiver instanceof ReceiverValue) {
-            DataFlowValue receiverDataFlowValue = DataFlowValueFactory.createDataFlowValue((ReceiverValue) receiver, context);
-            // Additional "receiver != null" information for KT-5840
-            // Should be applied if we consider a safe call
-            // For an unsafe call, we should not do it,
-            // otherwise not-null will propagate to successive statements
-            // Sample: x?.foo(x.bar()) // Inside foo call, x is not-nullable
-            if (CallUtilKt.isSafeCall(call)) {
-                initialDataFlowInfo = initialDataFlowInfo.disequate(receiverDataFlowValue, DataFlowValue.nullValue(builtIns));
-            }
-        }
-        infoForArguments.setInitialDataFlowInfo(initialDataFlowInfo);
+        infoForArguments.setInitialDataFlowInfo(context.dataFlowInfo);
 
         for (ValueArgument argument : call.getValueArguments()) {
             KtExpression expression = argument.getArgumentExpression();

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallExpressionResolver.java

@@ -153,16 +153,6 @@ public class CallExpressionResolver {
                 context, "trace to resolve as variable", nameExpression);
         KotlinType type =
                 getVariableType(nameExpression, receiver, callOperationNode, context.replaceTraceAndCache(temporaryForVariable), result);
-        // NB: we have duplicating code in ArgumentTypeResolver.
-        // It would be better to do it in getSelectorTypeInfo, but it breaks call expression analysis
-        // (all safe calls become unnecessary after it)
-        // QualifierReceiver is a thing like Collections. which has no type or value
-        if (receiver.exists() && receiver instanceof ReceiverValue) {
-            DataFlowValue receiverDataFlowValue = DataFlowValueFactory.createDataFlowValue((ReceiverValue) receiver, context);
-            if (callOperationNode != null && callOperationNode.getElementType() == KtTokens.SAFE_ACCESS) {
-                context = context.replaceDataFlowInfo(context.dataFlowInfo.disequate(receiverDataFlowValue, DataFlowValue.nullValue(builtIns)));
-            }
-        }
 
         if (result[0]) {
             temporaryForVariable.commit();
@@ -326,6 +316,19 @@ public class CallExpressionResolver {
         return TypeInfoFactoryKt.noTypeInfo(context);
     }
 
+    public static void reportUnnecessarySafeCall(
+            @NotNull BindingTrace trace, @NotNull KotlinType type,
+            @NotNull ASTNode callOperationNode, @Nullable Receiver explicitReceiver
+    ) {
+        if (explicitReceiver instanceof ExpressionReceiver &&
+            ((ExpressionReceiver) explicitReceiver).getExpression() instanceof KtSuperExpression) {
+            trace.report(UNEXPECTED_SAFE_CALL.on(callOperationNode.getPsi()));
+        }
+        else {
+            trace.report(UNNECESSARY_SAFE_CALL.on(callOperationNode.getPsi(), type));
+        }
+    }
+
     /**
      * Visits a qualified expression like x.y or x?.z controlling data flow information changes.
      *
@@ -401,6 +404,20 @@ public class CallExpressionResolver {
             ExpressionTypingContext baseContext = lastStage ? context : currentContext;
             currentContext = baseContext.replaceDataFlowInfo(receiverDataFlowInfo);
 
+            if (receiver.exists() && receiver instanceof ReceiverValue) {
+                DataFlowValue receiverDataFlowValue = DataFlowValueFactory.createDataFlowValue((ReceiverValue) receiver, context);
+                // Additional "receiver != null" information
+                // Should be applied if we consider a safe call
+                if (element.getSafe()) {
+                    DataFlowInfo dataFlowInfo = currentContext.dataFlowInfo;
+                    if (!dataFlowInfo.getNullability(receiverDataFlowValue).canBeNull()) {
+                        reportUnnecessarySafeCall(context.trace, receiverType, element.getNode(), receiver);
+                    }
+                    currentContext = currentContext.replaceDataFlowInfo(
+                            dataFlowInfo.disequate(receiverDataFlowValue, DataFlowValue.nullValue(builtIns)));
+                }
+            }
+
             KtExpression selectorExpression = element.getSelector();
             KotlinTypeInfo selectorReturnTypeInfo =
                     getSelectorReturnTypeInfo(receiver, element.getNode(), selectorExpression, currentContext);

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt

@@ -476,12 +476,6 @@ public class CandidateResolver(
             return UNSAFE_CALL_ERROR
         }
 
-        val bindingContext = trace.bindingContext
-        val receiverValue = DataFlowValueFactory.createDataFlowValue(receiverArgument, bindingContext, scope.ownerDescriptor)
-        if (safeAccess && !dataFlowInfo.getPredictableNullability(receiverValue).canBeNull()) {
-            tracing.unnecessarySafeCall(trace, receiverArgument.type)
-        }
-
         additionalTypeCheckers.forEach { it.checkReceiver(receiverParameter, receiverArgument, safeAccess, this) }
 
         return SUCCESS

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/AbstractTracingStrategy.java

@@ -186,19 +186,6 @@ public abstract class AbstractTracingStrategy implements TracingStrategy {
         }
     }
 
-    @Override
-    public void unnecessarySafeCall(@NotNull BindingTrace trace, @NotNull KotlinType type) {
-        ASTNode callOperationNode = call.getCallOperationNode();
-        assert callOperationNode != null;
-        Receiver explicitReceiver = call.getExplicitReceiver();
-        if (explicitReceiver instanceof ExpressionReceiver && ((ExpressionReceiver)explicitReceiver).getExpression() instanceof KtSuperExpression) {
-            trace.report(UNEXPECTED_SAFE_CALL.on(callOperationNode.getPsi()));
-        }
-        else {
-            trace.report(UNNECESSARY_SAFE_CALL.on(callOperationNode.getPsi(), type));
-        }
-    }
-
     @Override
     public void invisibleMember(@NotNull BindingTrace trace, @NotNull DeclarationDescriptorWithVisibility descriptor) {
         trace.report(INVISIBLE_MEMBER.on(call.getCallElement(), descriptor, descriptor.getVisibility(), descriptor));

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategy.java

@@ -91,9 +91,6 @@ public interface TracingStrategy {
         @Override
         public void unsafeCall(@NotNull BindingTrace trace, @NotNull KotlinType type, boolean isCallForImplicitInvoke) {}
 
-        @Override
-        public void unnecessarySafeCall(@NotNull BindingTrace trace, @NotNull KotlinType type) {}
-
         @Override
         public void invisibleMember(@NotNull BindingTrace trace, @NotNull DeclarationDescriptorWithVisibility descriptor) {}
 
@@ -147,8 +144,6 @@ public interface TracingStrategy {
 
     void unsafeCall(@NotNull BindingTrace trace, @NotNull KotlinType type, boolean isCallForImplicitInvoke);
 
-    void unnecessarySafeCall(@NotNull BindingTrace trace, @NotNull KotlinType type);
-
     void invisibleMember(@NotNull BindingTrace trace, @NotNull DeclarationDescriptorWithVisibility descriptor);
 
     void typeInferenceFailed(@NotNull BindingTrace trace, @NotNull InferenceErrorData inferenceErrorData);

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt

@@ -113,10 +113,6 @@ public class TracingStrategyForImplicitConstructorDelegationCall(
         unexpectedError("unsafeCall")
     }
 
-    override fun unnecessarySafeCall(trace: BindingTrace, type: KotlinType) {
-        unexpectedError("unnecessarySafeCall")
-    }
-
     override fun missingReceiver(trace: BindingTrace, expectedReceiver: ReceiverParameterDescriptor) {
         unexpectedError("missingReceiver")
     }

compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingUtils.java

@@ -534,13 +534,6 @@ public class ControlStructureTypingUtils {
             logError();
         }
 
-        @Override
-        public void unnecessarySafeCall(
-                @NotNull BindingTrace trace, @NotNull KotlinType type
-        ) {
-            logError();
-        }
-
         @Override
         public void invisibleMember(
                 @NotNull BindingTrace trace, @NotNull DeclarationDescriptorWithVisibility descriptor

compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classGenericsInParamsIndexMismatch.kt

@@ -29,6 +29,6 @@ import p.*
 
 fun <Y, Z> test(b: B<Y, Z>?) {
     if (b is C<Y, Z>) {
-        b?.<!OVERLOAD_RESOLUTION_AMBIGUITY!>foo<!>(null)
+        b<!UNNECESSARY_SAFE_CALL!>?.<!><!OVERLOAD_RESOLUTION_AMBIGUITY!>foo<!>(null)
     }
 }

compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classVsFunctionGenericsInParamsMismatch.kt

@@ -31,6 +31,6 @@ fun test(b: B?, c: C) {
     b?.foo(1, 1)
     c.foo(1, 1)
     if (b is C) {
-        b?.<!CANNOT_COMPLETE_RESOLVE!>foo<!>(1, 1)
+        b<!UNNECESSARY_SAFE_CALL!>?.<!><!CANNOT_COMPLETE_RESOLVE!>foo<!>(1, 1)
     }
 }

compiler/testData/diagnostics/tests/multimodule/duplicateMethod/simpleWithInheritance.kt

@@ -33,6 +33,6 @@ import p.*
 
 fun test(b: B?) {
     if (b is C && b is D) {
-        b?.<!OVERLOAD_RESOLUTION_AMBIGUITY!>getParent<!>()
+        b<!UNNECESSARY_SAFE_CALL!>?.<!><!OVERLOAD_RESOLUTION_AMBIGUITY!>getParent<!>()
     }
 }

compiler/testData/diagnostics/tests/multimodule/duplicateMethod/substitutedGenericInParams.kt

@@ -29,6 +29,6 @@ import p.*
 
 fun test(b: B?) {
     if (b is C) {
-        b?.<!OVERLOAD_RESOLUTION_AMBIGUITY!>foo<!>("")
+        b<!UNNECESSARY_SAFE_CALL!>?.<!><!OVERLOAD_RESOLUTION_AMBIGUITY!>foo<!>("")
     }
 }

idea/testData/checker/infos/SmartCastsWithSafeAccess.kt

@@ -20,7 +20,7 @@ fun test(a: A?) {
     }
 
     if (a is B? && a is C?) {
-        <info descr="Smart cast to B?">a</info><info>?.</info>bar()
+        <info descr="Smart cast to B">a</info><info>?.</info>bar()
     }
 
     a<info>?.</info>foo()