From df4dcc0f8e83b9ff9d0d2a56147091e99e0a0da8 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Fri, 25 May 2018 16:00:11 +0300 Subject: [PATCH] Ignore data flow info from assert calls if assertions mode is not LEGACY. This is done since assertions can be disabled (in both compile time and runtime) and thus, the data flow info is not reliable anymore. #KT-24529: Fixed --- .../arguments/K2JVMCompilerArguments.kt | 16 ++------ .../kotlin/config/JVMAssertionsMode.kt | 3 ++ .../kotlin/resolve/calls/CallResolver.java | 26 +++++++++++-- .../testsWithStdLib/assert/cast.kt | 20 ++++++++++ .../testsWithStdLib/assert/safeCall.kt | 37 +++++++++++++++++++ .../CompilerTestLanguageVersionSettings.kt | 21 +++-------- .../DiagnosticsTestWithStdLibGenerated.java | 23 ++++++++++++ ...ticsTestWithStdLibUsingJavacGenerated.java | 23 ++++++++++++ .../jetbrains/kotlin/config/AnalysisFlag.kt | 18 +++------ 9 files changed, 143 insertions(+), 44 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithStdLib/assert/cast.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/assert/safeCall.kt diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt index 7bda4d546af..7262e29d584 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt @@ -1,17 +1,6 @@ /* - * Copyright 2010-2016 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2010-2018 JetBrains s.r.o. 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.cli.common.arguments @@ -259,6 +248,7 @@ class K2JVMCompilerArguments : CommonCompilerArguments() { supportCompatqualCheckerFrameworkAnnotations ) result[AnalysisFlag.enableJvmDefault] = enableJvmDefault + result[AnalysisFlag.ignoreDataFlowInAssert] = JVMAssertionsMode.fromString(assertionsMode) != JVMAssertionsMode.LEGACY return result } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/config/JVMAssertionsMode.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/config/JVMAssertionsMode.kt index 2dc63d8f740..a7fefbe1ca3 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/config/JVMAssertionsMode.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/config/JVMAssertionsMode.kt @@ -17,5 +17,8 @@ enum class JVMAssertionsMode(val description: String) { @JvmStatic fun fromStringOrNull(string: String?) = values().find { it.description == string } + + @JvmStatic + fun fromString(string: String?) = fromStringOrNull(string) ?: DEFAULT } } \ No newline at end of file diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java index 3cc2b339143..243d581a2ab 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license * that can be found in the license/LICENSE.txt file. */ @@ -12,6 +12,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.builtins.FunctionTypesKt; import org.jetbrains.kotlin.builtins.KotlinBuiltIns; +import org.jetbrains.kotlin.config.AnalysisFlag; import org.jetbrains.kotlin.config.LanguageFeature; import org.jetbrains.kotlin.config.LanguageVersionSettings; import org.jetbrains.kotlin.descriptors.*; @@ -655,6 +656,7 @@ public class CallResolver { @NotNull ResolutionTask resolutionTask, @NotNull TracingStrategy tracing ) { + DataFlowInfo initialInfo = context.dataFlowInfoForArguments.getResultInfo(); if (context.checkArguments == CheckArgumentTypesMode.CHECK_VALUE_ARGUMENTS) { argumentTypeResolver.analyzeArgumentsAndRecordTypes(context, ResolveArgumentsMode.SHAPE_FUNCTION_ARGUMENTS); } @@ -673,14 +675,32 @@ public class CallResolver { } } + OverloadResolutionResultsImpl result; if (!(resolutionTask.resolutionKind instanceof NewResolutionOldInference.ResolutionKind.GivenCandidates)) { assert resolutionTask.name != null; - return newResolutionOldInference.runResolution(context, resolutionTask.name, resolutionTask.resolutionKind, tracing); + result = newResolutionOldInference.runResolution(context, resolutionTask.name, resolutionTask.resolutionKind, tracing); } else { assert resolutionTask.givenCandidates != null; - return newResolutionOldInference.runResolutionForGivenCandidates(context, tracing, resolutionTask.givenCandidates); + result = newResolutionOldInference.runResolutionForGivenCandidates(context, tracing, resolutionTask.givenCandidates); } + + // in code like + // assert(a!!.isEmpty()) + // a.length + // we should ignore data flow info from assert argument, since assertions can be disabled and + // thus it will lead to NPE in runtime otherwise + if (languageVersionSettings.getFlag(AnalysisFlag.getIgnoreDataFlowInAssert()) && result.isSingleResult()) { + D descriptor = result.getResultingDescriptor(); + if (descriptor.getName().equals(Name.identifier("assert"))) { + DeclarationDescriptor declaration = descriptor.getContainingDeclaration(); + if (declaration instanceof PackageFragmentDescriptor && + ((PackageFragmentDescriptor) declaration).getFqName().asString().equals("kotlin")) { + context.dataFlowInfoForArguments.updateInfo(context.call.getValueArguments().get(0), initialInfo); + } + } + } + return result; } ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/compiler/testData/diagnostics/testsWithStdLib/assert/cast.kt b/compiler/testData/diagnostics/testsWithStdLib/assert/cast.kt new file mode 100644 index 00000000000..358f56deb41 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/assert/cast.kt @@ -0,0 +1,20 @@ +// !IGNORE_DATA_FLOW_IN_ASSERT +// SKIP_TXT +// WITH_RUNTIME + +interface A {} + +class B: A { + fun bool() = true +} + +fun test1(a: A) { + assert((a as B).bool()) + a.bool() +} + +fun test2() { + val a: A? = null; + assert((a as B).bool()) + a?.bool() +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/assert/safeCall.kt b/compiler/testData/diagnostics/testsWithStdLib/assert/safeCall.kt new file mode 100644 index 00000000000..02acefd5785 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/assert/safeCall.kt @@ -0,0 +1,37 @@ +// !IGNORE_DATA_FLOW_IN_ASSERT +// SKIP_TXT +// WITH_RUNTIME + +fun test1(s: String?) { + assert(s!!.isEmpty()) + s?.length +} + +fun test2(s: String?) { + assert(s!!.isEmpty()) + s!!.length +} + +fun test3(s: String?) { + assert(s!!.isEmpty()) + s.length +} + +fun test4() { + val s: String? = null; + assert(s!!.isEmpty()) + s?.length +} + +fun test5() { + val s: String? = null; + assert(s!!.isEmpty()) + s!!.length +} + +fun test6() { + val s: String? = null; + assert(s!!.isEmpty()) + s.length +} + diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/CompilerTestLanguageVersionSettings.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/CompilerTestLanguageVersionSettings.kt index 2446892e6fc..2fbf3998a92 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/CompilerTestLanguageVersionSettings.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/CompilerTestLanguageVersionSettings.kt @@ -1,17 +1,6 @@ /* - * Copyright 2010-2017 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2010-2018 JetBrains s.r.o. 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.checkers @@ -28,6 +17,7 @@ const val API_VERSION_DIRECTIVE = "API_VERSION" const val EXPERIMENTAL_DIRECTIVE = "EXPERIMENTAL" const val USE_EXPERIMENTAL_DIRECTIVE = "USE_EXPERIMENTAL" +const val IGNORE_DATA_FLOW_IN_ASSERT_DIRECTIVE = "IGNORE_DATA_FLOW_IN_ASSERT" const val ENABLE_JVM_DEFAULT = "ENABLE_JVM_DEFAULT" data class CompilerTestLanguageVersionSettings( @@ -60,9 +50,10 @@ fun parseLanguageVersionSettings(directiveMap: Map): LanguageVer val languageFeaturesString = directiveMap[LANGUAGE_DIRECTIVE] val experimental = directiveMap[EXPERIMENTAL_DIRECTIVE]?.split(' ')?.let { AnalysisFlag.experimental to it } val useExperimental = directiveMap[USE_EXPERIMENTAL_DIRECTIVE]?.split(' ')?.let { AnalysisFlag.useExperimental to it } + val ignoreDataFlowInAssert = AnalysisFlag.ignoreDataFlowInAssert to directiveMap.containsKey(IGNORE_DATA_FLOW_IN_ASSERT_DIRECTIVE) val enableJvmDefault = AnalysisFlag.enableJvmDefault to directiveMap.containsKey(ENABLE_JVM_DEFAULT) - if (apiVersionString == null && languageFeaturesString == null && experimental == null && useExperimental == null) return null + if (apiVersionString == null && languageFeaturesString == null && experimental == null && useExperimental == null && !ignoreDataFlowInAssert.second) return null val apiVersion = (if (apiVersionString != null) ApiVersion.parse(apiVersionString) else ApiVersion.LATEST_STABLE) ?: error("Unknown API version: $apiVersionString") @@ -73,7 +64,7 @@ fun parseLanguageVersionSettings(directiveMap: Map): LanguageVer return CompilerTestLanguageVersionSettings( languageFeatures, apiVersion, languageVersion, - mapOf(*listOfNotNull(experimental, useExperimental, enableJvmDefault).toTypedArray()) + mapOf(*listOfNotNull(experimental, useExperimental, enableJvmDefault, ignoreDataFlowInAssert).toTypedArray()) ) } diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java index 55e905ed10b..80b741d9791 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java @@ -776,6 +776,29 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW } } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/assert") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Assert extends AbstractDiagnosticsTestWithStdLib { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInAssert() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/assert"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("cast.kt") + public void testCast() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/assert/cast.kt"); + } + + @TestMetadata("safeCall.kt") + public void testSafeCall() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/assert/safeCall.kt"); + } + } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/builtins") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java index e953802d77b..4b30d026ab4 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java @@ -776,6 +776,29 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno } } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/assert") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Assert extends AbstractDiagnosticsTestWithStdLibUsingJavac { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInAssert() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/assert"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("cast.kt") + public void testCast() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/assert/cast.kt"); + } + + @TestMetadata("safeCall.kt") + public void testSafeCall() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/assert/safeCall.kt"); + } + } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/builtins") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/util/src/org/jetbrains/kotlin/config/AnalysisFlag.kt b/compiler/util/src/org/jetbrains/kotlin/config/AnalysisFlag.kt index 8b96aebcd3f..ad74f775eb9 100644 --- a/compiler/util/src/org/jetbrains/kotlin/config/AnalysisFlag.kt +++ b/compiler/util/src/org/jetbrains/kotlin/config/AnalysisFlag.kt @@ -1,17 +1,6 @@ /* - * Copyright 2010-2017 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2010-2018 JetBrains s.r.o. 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.config @@ -72,5 +61,8 @@ class AnalysisFlag internal constructor( @JvmStatic val enableJvmDefault by Flag.Boolean + + @JvmStatic + val ignoreDataFlowInAssert by Flag.Boolean } }