From 46f419118513a0325ef681e943254766904a07a4 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 24 Nov 2015 12:06:33 +0300 Subject: [PATCH] Unnecessary safe call is now reported in CallExpressionResolver, removed code duplication for safe call receivers See also KT-10175 --- .../resolve/calls/ArgumentTypeResolver.java | 16 +------- .../resolve/calls/CallExpressionResolver.java | 37 ++++++++++++++----- .../kotlin/resolve/calls/CandidateResolver.kt | 6 --- .../calls/tasks/AbstractTracingStrategy.java | 13 ------- .../resolve/calls/tasks/TracingStrategy.java | 5 --- ...egyForImplicitConstructorDelegationCall.kt | 4 -- .../ControlStructureTypingUtils.java | 7 ---- .../classGenericsInParamsIndexMismatch.kt | 2 +- ...classVsFunctionGenericsInParamsMismatch.kt | 2 +- .../duplicateMethod/simpleWithInheritance.kt | 2 +- .../substitutedGenericInParams.kt | 2 +- .../checker/infos/SmartCastsWithSafeAccess.kt | 2 +- 12 files changed, 33 insertions(+), 65 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ArgumentTypeResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ArgumentTypeResolver.java index 33e9c32da61..c7dc1047d2a 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ArgumentTypeResolver.java +++ b/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(); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallExpressionResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallExpressionResolver.java index e48c47a7c46..0230bdb82bd 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallExpressionResolver.java +++ b/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); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt index 285ab3551ac..e5f891d9518 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt +++ b/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 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 408c83c363f..3b4749bcc9c 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 @@ -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)); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategy.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategy.java index 26d553416db..f06870c1362 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategy.java +++ b/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); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt index c8549d44d16..430022c8058 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt +++ b/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") } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingUtils.java b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingUtils.java index 01f666f9c8f..f12cc12b929 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingUtils.java +++ b/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 diff --git a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classGenericsInParamsIndexMismatch.kt b/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classGenericsInParamsIndexMismatch.kt index af66c73c557..eaf8e3034e8 100644 --- a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classGenericsInParamsIndexMismatch.kt +++ b/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classGenericsInParamsIndexMismatch.kt @@ -29,6 +29,6 @@ import p.* fun test(b: B?) { if (b is C) { - b?.foo(null) + b?.foo(null) } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classVsFunctionGenericsInParamsMismatch.kt b/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classVsFunctionGenericsInParamsMismatch.kt index 10e5ac41d46..f7e2f091a53 100644 --- a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/classVsFunctionGenericsInParamsMismatch.kt +++ b/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?.foo(1, 1) + b?.foo(1, 1) } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/simpleWithInheritance.kt b/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/simpleWithInheritance.kt index e91afea2b00..edc17bba4e5 100644 --- a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/simpleWithInheritance.kt +++ b/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?.getParent() + b?.getParent() } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/substitutedGenericInParams.kt b/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/substitutedGenericInParams.kt index 99c243becf6..4048de592bf 100644 --- a/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/substitutedGenericInParams.kt +++ b/compiler/testData/diagnostics/tests/multimodule/duplicateMethod/substitutedGenericInParams.kt @@ -29,6 +29,6 @@ import p.* fun test(b: B?) { if (b is C) { - b?.foo("") + b?.foo("") } } \ No newline at end of file diff --git a/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt b/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt index d8cf1fdd1c6..5cfb03c2275 100644 --- a/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt +++ b/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt @@ -20,7 +20,7 @@ fun test(a: A?) { } if (a is B? && a is C?) { - a?.bar() + a?.bar() } a?.foo()