From a9678010a8324f52738c4970efcf449b7f7cf361 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 16 Feb 2017 18:41:23 +0300 Subject: [PATCH] Invert LanguageFeature responsible for warning on coroutines The problem was that LanguageVersionSettingsImpl.DEFAULT did not have "WarnOnCoroutines" as a feature and so it was manually added to the settings, but only in two places: in the compiler and in the IDE --- .../kotlin/cli/common/CLICompiler.java | 18 +++++++-------- .../kotlin/resolve/ModifiersChecker.kt | 8 +++---- .../calls/checkers/coroutineCallChecker.kt | 2 +- .../coroutinesEnabledWithWarning.kt | 2 +- .../suspendCoroutineUnavailableWithNewAPI.kt | 1 + .../checkers/AbstractDiagnosticsTest.kt | 22 ++++++++++--------- .../kotlin/checkers/BaseDiagnosticsTest.kt | 4 ++++ .../AbstractDiagnosticsTestWithJsStdLib.kt | 3 +-- ...csTestWithJsStdLibAndBackendCompilation.kt | 2 +- .../kotlin/config/LanguageVersionSettings.kt | 3 +-- .../jetbrains/kotlin/idea/project/Platform.kt | 6 ++--- 11 files changed, 38 insertions(+), 33 deletions(-) diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java b/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java index d94312e7e6e..62a18bdeeaf 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java @@ -288,19 +288,19 @@ public abstract class CLICompiler { @Nullable private static LanguageFeature chooseCoroutinesApplicabilityLevel( - @NotNull CompilerConfiguration configuration, @NotNull CommonCompilerArguments arguments) { - if (!arguments.coroutinesEnable && !arguments.coroutinesError && !arguments.coroutinesWarn) { - return LanguageFeature.WarnOnCoroutines; - } - else if (arguments.coroutinesError && !arguments.coroutinesWarn && !arguments.coroutinesEnable) { + @NotNull CompilerConfiguration configuration, + @NotNull CommonCompilerArguments arguments + ) { + if (arguments.coroutinesError && !arguments.coroutinesWarn && !arguments.coroutinesEnable) { return LanguageFeature.ErrorOnCoroutines; } - else if (arguments.coroutinesWarn && !arguments.coroutinesError && !arguments.coroutinesEnable) { - return LanguageFeature.WarnOnCoroutines; - } else if (arguments.coroutinesEnable && !arguments.coroutinesWarn && !arguments.coroutinesError) { + return LanguageFeature.DoNotWarnOnCoroutines; + } + else if (!arguments.coroutinesEnable && !arguments.coroutinesError) { return null; - } else { + } + else { String message = "The -Xcoroutines can only have one value"; configuration.getNotNull(CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY).report( CompilerMessageSeverity.ERROR, message, CompilerMessageLocation.NO_LOCATION diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt index 510c238b9a0..bbd489f3944 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt @@ -103,8 +103,8 @@ object ModifierCheckerCore { LanguageFeature.Coroutines to LanguageFeature.ErrorOnCoroutines ) - val warningOnFeature = mapOf( - LanguageFeature.Coroutines to LanguageFeature.WarnOnCoroutines + val noWarningOnFeature = mapOf( + LanguageFeature.Coroutines to LanguageFeature.DoNotWarnOnCoroutines ) val featureDependenciesTargets = mapOf( @@ -299,8 +299,8 @@ object ModifierCheckerCore { return false } - val pairedWarningFeature = warningOnFeature[dependency] - if (pairedWarningFeature != null && languageVersionSettings.supportsFeature(pairedWarningFeature)) { + val pairedNoWarningFeature = noWarningOnFeature[dependency] + if (pairedNoWarningFeature != null && !languageVersionSettings.supportsFeature(pairedNoWarningFeature)) { trace.report(Errors.EXPERIMENTAL_FEATURE_WARNING.on(node.psi, diagnosticData)) } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/coroutineCallChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/coroutineCallChecker.kt index 28a6647163c..de828e2ddce 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/coroutineCallChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/coroutineCallChecker.kt @@ -96,7 +96,7 @@ fun checkCoroutinesFeature(languageVersionSettings: LanguageVersionSettings, dia else if (languageVersionSettings.supportsFeature(LanguageFeature.ErrorOnCoroutines)) { diagnosticHolder.report(Errors.EXPERIMENTAL_FEATURE_ERROR.on(reportOn, diagnosticData)) } - else if (languageVersionSettings.supportsFeature(LanguageFeature.WarnOnCoroutines)) { + else if (!languageVersionSettings.supportsFeature(LanguageFeature.DoNotWarnOnCoroutines)) { diagnosticHolder.report(Errors.EXPERIMENTAL_FEATURE_WARNING.on(reportOn, diagnosticData)) } } diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/coroutinesEnabledWithWarning.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/coroutinesEnabledWithWarning.kt index e940bbc76d1..7d7e5691a66 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/coroutinesEnabledWithWarning.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/coroutinesEnabledWithWarning.kt @@ -1,5 +1,5 @@ // !DIAGNOSTICS: -UNUSED_PARAMETER -// !LANGUAGE: +WarnOnCoroutines +// !LANGUAGE: -DoNotWarnOnCoroutines suspend fun suspendHere(): String = "OK" diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendCoroutineUnavailableWithNewAPI.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendCoroutineUnavailableWithNewAPI.kt index 19eb41c7542..3cf2f600336 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendCoroutineUnavailableWithNewAPI.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendCoroutineUnavailableWithNewAPI.kt @@ -1,4 +1,5 @@ // !API_VERSION: 1.1 +// !LANGUAGE: +DoNotWarnOnCoroutines // SKIP_TXT import kotlin.coroutines.experimental.* diff --git a/compiler/tests-common/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTest.kt b/compiler/tests-common/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTest.kt index 5b4d48263b9..567cf985964 100644 --- a/compiler/tests-common/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTest.kt +++ b/compiler/tests-common/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTest.kt @@ -25,6 +25,7 @@ import org.jetbrains.kotlin.analyzer.common.DefaultAnalyzerFacade import org.jetbrains.kotlin.cli.jvm.compiler.CliLightClassGenerationSupport import org.jetbrains.kotlin.cli.jvm.compiler.JvmPackagePartProvider import org.jetbrains.kotlin.config.LanguageVersionSettings +import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.container.get import org.jetbrains.kotlin.context.ModuleContext @@ -63,6 +64,7 @@ import org.jetbrains.kotlin.test.util.DescriptorValidator import org.jetbrains.kotlin.test.util.RecursiveDescriptorComparator import org.jetbrains.kotlin.test.util.RecursiveDescriptorComparator.RECURSIVE import org.jetbrains.kotlin.test.util.RecursiveDescriptorComparator.RECURSIVE_ALL +import org.jetbrains.kotlin.utils.keysToMap import org.junit.Assert import java.io.File import java.util.* @@ -198,7 +200,7 @@ abstract class AbstractDiagnosticsTest : BaseDiagnosticsTest() { // To be overridden by diagnostic-like tests. } - private fun loadLanguageVersionSettings(module: List): LanguageVersionSettings? { + private fun loadLanguageVersionSettings(module: List): LanguageVersionSettings { var result: LanguageVersionSettings? = null for (file in module) { val current = file.customLanguageVersionSettings @@ -213,7 +215,11 @@ abstract class AbstractDiagnosticsTest : BaseDiagnosticsTest() { } } - return result + return result ?: BaseDiagnosticsTest.DiagnosticTestLanguageVersionSettings( + BaseDiagnosticsTest.DEFAULT_DIAGNOSTIC_TESTS_FEATURES.keysToMap { true }, + LanguageVersionSettingsImpl.DEFAULT.apiVersion, + LanguageVersionSettingsImpl.DEFAULT.languageVersion + ) } private fun checkDynamicCallDescriptors(expectedFile: File, testFiles: List) { @@ -253,19 +259,15 @@ abstract class AbstractDiagnosticsTest : BaseDiagnosticsTest() { moduleContext: ModuleContext, files: List, moduleTrace: BindingTrace, - languageVersionSettings: LanguageVersionSettings?, + languageVersionSettings: LanguageVersionSettings, separateModules: Boolean ): AnalysisResult { @Suppress("NAME_SHADOWING") var files = files - val configuration = - if (languageVersionSettings != null) - environment.configuration.copy().apply { - this.languageVersionSettings = languageVersionSettings - } - else - environment.configuration + val configuration = environment.configuration.copy().apply { + this.languageVersionSettings = languageVersionSettings + } // New JavaDescriptorResolver is created for each module, which is good because it emulates different Java libraries for each module, // albeit with same class names diff --git a/compiler/tests-common/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt b/compiler/tests-common/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt index 8964908f50e..74aaca12dad 100644 --- a/compiler/tests-common/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt +++ b/compiler/tests-common/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt @@ -286,6 +286,10 @@ abstract class BaseDiagnosticsTest : KotlinMultiFileTestWithJava, moduleTrace: BindingTrace, - languageVersionSettings: LanguageVersionSettings?, + languageVersionSettings: LanguageVersionSettings, separateModules: Boolean ): JsAnalysisResult { // TODO: support LANGUAGE directive in JS diagnostic tests - assert(languageVersionSettings == null) { "$LANGUAGE_DIRECTIVE directive is not supported in JS diagnostic tests" } moduleTrace.record(MODULE_KIND, moduleContext.module, getModuleKind(files)) return TopDownAnalyzerFacadeForJS.analyzeFilesWithGivenTrace(files, moduleTrace, moduleContext, config) } diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt b/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt index b4c4e8a495d..4a5f160e4c5 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt +++ b/compiler/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt @@ -30,7 +30,7 @@ abstract class AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation : Abstra moduleContext: ModuleContext, files: List, moduleTrace: BindingTrace, - languageVersionSettings: LanguageVersionSettings?, + languageVersionSettings: LanguageVersionSettings, separateModules: Boolean ): JsAnalysisResult { val analysisResult = super.analyzeModuleContents(moduleContext, files, moduleTrace, languageVersionSettings, separateModules) diff --git a/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt b/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt index 653832ddcdb..47a2c635b97 100644 --- a/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt +++ b/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt @@ -52,7 +52,7 @@ enum class LanguageFeature( MultiPlatformProjects(null), MultiPlatformDoNotCheckImpl(null), - WarnOnCoroutines(null), + DoNotWarnOnCoroutines(null), ErrorOnCoroutines(null) ; @@ -100,7 +100,6 @@ interface LanguageVersionSettings { // Please do not use this to enable/disable specific features/checks. Instead add a new LanguageFeature entry and call supportsFeature val languageVersion: LanguageVersion - // TODO: refactor arguments related to coroutines so that this list is empty by default val additionalFeatures: Collection @Deprecated("This is a temporary solution, please do not use.") diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/project/Platform.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/project/Platform.kt index f8042c2c5e7..6bf40534d4b 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/project/Platform.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/project/Platform.kt @@ -34,7 +34,7 @@ import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.resolve.TargetPlatform val KtElement.platform: TargetPlatform - get() = TargetPlatformDetector.getPlatform(getContainingKtFile()) + get() = TargetPlatformDetector.getPlatform(containingKtFile) val KtElement.builtIns: KotlinBuiltIns get() = getResolutionFacade().moduleDescriptor.builtIns @@ -130,8 +130,8 @@ private fun getExtraLanguageFeatures( ): List { return mutableListOf().apply { when (coroutineSupport) { - CoroutineSupport.ENABLED -> {} - CoroutineSupport.ENABLED_WITH_WARNING -> add(LanguageFeature.WarnOnCoroutines) + CoroutineSupport.ENABLED -> add(LanguageFeature.DoNotWarnOnCoroutines) + CoroutineSupport.ENABLED_WITH_WARNING -> {} CoroutineSupport.DISABLED -> add(LanguageFeature.ErrorOnCoroutines) } if (targetPlatformKind == TargetPlatformKind.Common ||