From 57bafcf9e962b2866e94ef68a94356ee048ca5e8 Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Thu, 9 Aug 2018 14:35:25 +0300 Subject: [PATCH] Do not lose information about contracts if call uses default value It is safe to treat DefaultValueArgument as UNKNOWN_COMPUTATION, because default arguments can't break smartcasts. Possibly, they can add new ones, but it can be supported later. ^KT-25278 Fixed --- .../contracts/EffectsExtractingVisitor.kt | 41 ++++++++++++++----- .../smartcasts/callWithDefaultValue.kt | 16 ++++++++ .../smartcasts/callWithDefaultValue.txt | 6 +++ .../DiagnosticsTestWithStdLibGenerated.java | 5 +++ ...ticsTestWithStdLibUsingJavacGenerated.java | 5 +++ 5 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt b/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt index 58d9c031fbb..c4811e108f9 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt @@ -34,8 +34,7 @@ import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingTrace import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall -import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument -import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.calls.model.* import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory import org.jetbrains.kotlin.resolve.calls.tasks.ExplicitReceiverKind @@ -170,14 +169,36 @@ class EffectsExtractingVisitor( arguments.addIfNotNull(extensionReceiver?.toComputation()) arguments.addIfNotNull(dispatchReceiver?.toComputation()) - valueArgumentsByIndex?.mapTo(arguments) { - val valueArgument = (it as? ExpressionValueArgument)?.valueArgument ?: return null - when (valueArgument) { - is KtLambdaArgument -> valueArgument.getLambdaExpression()?.let { ESLambda(it) } ?: return null - else -> extractOrGetCached(valueArgument.getArgumentExpression() ?: return null) - } - } ?: return null + val passedValueArguments = valueArgumentsByIndex ?: return null + + passedValueArguments.mapTo(arguments) { it.toComputation() ?: return null } return arguments } -} \ No newline at end of file + + private fun ResolvedValueArgument.toComputation(): Computation? { + return when (this) { + // Assume that we don't know anything about default arguments + // Note that we don't want to return 'null' here, because 'null' indicates that we can't + // analyze whole call, which is undesired for cases like `kotlin.test.assertNotNull` + is DefaultValueArgument -> UNKNOWN_COMPUTATION + + // We prefer to throw away calls with varags completely, just to be safe + // Potentially, we could return UNKNOWN_COMPUTATION here too + is VarargValueArgument -> null + + is ExpressionValueArgument -> valueArgument?.toComputation() + + // Should be exhaustive + else -> throw IllegalStateException("Unexpected ResolvedValueArgument $this") + } + } + + private fun ValueArgument.toComputation(): Computation? { + return when (this) { + is KtLambdaArgument -> getLambdaExpression()?.let { ESLambda(it) } + is KtValueArgument -> getArgumentExpression()?.let { extractOrGetCached(it) } + else -> null + } + } +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.kt b/compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.kt new file mode 100644 index 00000000000..6f635f53eec --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.kt @@ -0,0 +1,16 @@ +// !LANGUAGE: +AllowContractsForCustomFunctions +UseReturnsEffect +// !DIAGNOSTICS: -INVISIBLE_REFERENCE -INVISIBLE_MEMBER + +import kotlin.internal.contracts.* + +fun myAssert(condition: Boolean, message: String = "") { + contract { + returns() implies (condition) + } + if (!condition) throw kotlin.IllegalArgumentException(message) +} + +fun test(x: Any?) { + myAssert(x is String) + x.length +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.txt b/compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.txt new file mode 100644 index 00000000000..dd02badfc16 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.txt @@ -0,0 +1,6 @@ +package + +public fun myAssert(/*0*/ condition: kotlin.Boolean, /*1*/ message: kotlin.String = ...): kotlin.Unit + Returns(WILDCARD) -> condition + +public fun test(/*0*/ x: kotlin.Any?): kotlin.Unit diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java index 91f4381ddd4..de9e713fa3b 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java @@ -1211,6 +1211,11 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("callWithDefaultValue.kt") + public void testCallWithDefaultValue() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.kt"); + } + @TestMetadata("catchExceptionSpilling.kt") public void testCatchExceptionSpilling() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/catchExceptionSpilling.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java index f0a0fe95ef6..5b81c1323f8 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java @@ -1211,6 +1211,11 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("callWithDefaultValue.kt") + public void testCallWithDefaultValue() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/callWithDefaultValue.kt"); + } + @TestMetadata("catchExceptionSpilling.kt") public void testCatchExceptionSpilling() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/catchExceptionSpilling.kt");