From 92fac304e77ae633774902954ba62d3475139852 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Wed, 25 Aug 2021 17:53:48 +0200 Subject: [PATCH] Forbid @Synchronized annotation on suspend functions and lambdas. With warning for now. #KT-27333 --- ...irOldFrontendDiagnosticsTestGenerated.java | 6 +++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++ .../SynchronizedAnnotationOnLambdaChecker.kt | 39 +++++++++++++++++++ .../SynchronizedOnInlineMethodChecker.kt | 27 ------------- .../jvm/checkers/declarationCheckers.kt | 4 ++ .../diagnostics/DefaultErrorMessagesJvm.java | 2 + .../resolve/jvm/diagnostics/ErrorsJvm.java | 1 + .../jvm/platform/JvmPlatformConfigurator.kt | 6 +-- .../kotlin/resolve/AnnotationChecker.kt | 7 +++- .../inline/synchronizedOnSuspend.fir.kt | 24 ++++++++++++ .../inline/synchronizedOnSuspend.kt | 24 ++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ ...CompilerTestFE10TestdataTestGenerated.java | 6 +++ 13 files changed, 127 insertions(+), 31 deletions(-) create mode 100644 compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedAnnotationOnLambdaChecker.kt delete mode 100644 compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedOnInlineMethodChecker.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.fir.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index 60fb4fc5ccf..2956711f562 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -36134,6 +36134,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti public void testSynchronizedOnInline() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnInline.kt"); } + + @Test + @TestMetadata("synchronizedOnSuspend.kt") + public void testSynchronizedOnSuspend() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt"); + } } @Nested diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index c682483cc45..725d4140ad2 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -36134,6 +36134,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac public void testSynchronizedOnInline() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnInline.kt"); } + + @Test + @TestMetadata("synchronizedOnSuspend.kt") + public void testSynchronizedOnSuspend() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt"); + } } @Nested diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedAnnotationOnLambdaChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedAnnotationOnLambdaChecker.kt new file mode 100644 index 00000000000..0184a25c057 --- /dev/null +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedAnnotationOnLambdaChecker.kt @@ -0,0 +1,39 @@ +/* + * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. + * 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.resolve.jvm.checkers + +import org.jetbrains.kotlin.config.LanguageVersionSettings +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.annotations.KotlinTarget +import org.jetbrains.kotlin.psi.KtAnnotated +import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.psi.KtLambdaExpression +import org.jetbrains.kotlin.psi.psiUtil.unwrapParenthesesLabelsAndAnnotations +import org.jetbrains.kotlin.resolve.AdditionalAnnotationChecker +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.BindingTrace +import org.jetbrains.kotlin.resolve.jvm.annotations.SYNCHRONIZED_ANNOTATION_FQ_NAME +import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm + +object SynchronizedAnnotationOnLambdaChecker : AdditionalAnnotationChecker { + override fun checkEntries( + entries: List, + actualTargets: List, + trace: BindingTrace, + annotated: KtAnnotated?, + languageVersionSettings: LanguageVersionSettings + ) { + if (entries.isEmpty()) return + + val annotation = entries.find { trace.get(BindingContext.ANNOTATION, it)?.fqName == SYNCHRONIZED_ANNOTATION_FQ_NAME } ?: return + + val literal = (annotated?.unwrapParenthesesLabelsAndAnnotations() as? KtLambdaExpression)?.functionLiteral ?: return + val descriptor = trace.bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, literal] + if (descriptor is FunctionDescriptor && descriptor.isSuspend) { + trace.report(ErrorsJvm.SYNCHRONIZED_ON_SUSPEND.on(annotation)) + } + } +} \ No newline at end of file diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedOnInlineMethodChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedOnInlineMethodChecker.kt deleted file mode 100644 index ccf75951284..00000000000 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SynchronizedOnInlineMethodChecker.kt +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. - * 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.resolve.jvm.checkers - -import org.jetbrains.kotlin.descriptors.DeclarationDescriptor -import org.jetbrains.kotlin.descriptors.FunctionDescriptor -import org.jetbrains.kotlin.psi.KtDeclaration -import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils -import org.jetbrains.kotlin.resolve.checkers.DeclarationChecker -import org.jetbrains.kotlin.resolve.checkers.DeclarationCheckerContext -import org.jetbrains.kotlin.resolve.jvm.annotations.findSynchronizedAnnotation -import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm - -object SynchronizedOnInlineMethodChecker : DeclarationChecker { - override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) { - if (descriptor is FunctionDescriptor && descriptor.isInline) { - val annotation = descriptor.findSynchronizedAnnotation() - if (annotation != null) { - val reportOn = DescriptorToSourceUtils.getSourceFromAnnotation(annotation) ?: declaration - context.trace.report(ErrorsJvm.SYNCHRONIZED_ON_INLINE.on(reportOn)) - } - } - } -} diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt index aa35cc2bcde..6443f1627ea 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/declarationCheckers.kt @@ -195,6 +195,10 @@ class SynchronizedAnnotationChecker : DeclarationChecker { context.trace.report(ErrorsJvm.SYNCHRONIZED_IN_INTERFACE.on(annotationEntry)) } else if (descriptor.modality == Modality.ABSTRACT) { context.trace.report(ErrorsJvm.SYNCHRONIZED_ON_ABSTRACT.on(annotationEntry)) + } else if (descriptor.isInline) { + context.trace.report(ErrorsJvm.SYNCHRONIZED_ON_INLINE.on(annotationEntry)) + } else if (descriptor.isSuspend) { + context.trace.report(ErrorsJvm.SYNCHRONIZED_ON_SUSPEND.on(annotationEntry)) } } } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index a632f843203..7e087b6b68a 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -217,6 +217,8 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { MAP.put(VALUE_CLASS_WITHOUT_JVM_INLINE_ANNOTATION, "Value classes without @JvmInline annotation are not supported yet"); MAP.put(JVM_INLINE_WITHOUT_VALUE_CLASS, "@JvmInline annotation is only applicable to value classes"); + MAP.put(SYNCHRONIZED_ON_SUSPEND, "@Synchronized annotation is not applicable to suspend functions and lambdas"); + MAP.put(TYPEOF_SUSPEND_TYPE, "Suspend functional types are not supported in typeOf"); MAP.put(TYPEOF_EXTENSION_FUNCTION_TYPE, "Extension function types are not supported in typeOf"); MAP.put(TYPEOF_ANNOTATED_TYPE, "Annotated types are not supported in typeOf"); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index 743af97e12e..cb9dca2be16 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -52,6 +52,7 @@ public interface ErrorsJvm { DiagnosticFactory0 VOLATILE_ON_DELEGATE = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 SYNCHRONIZED_ON_ABSTRACT = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 SYNCHRONIZED_ON_INLINE = DiagnosticFactory0.create(WARNING); + DiagnosticFactory0 SYNCHRONIZED_ON_SUSPEND = DiagnosticFactory0.create(WARNING); DiagnosticFactory0 SYNCHRONIZED_IN_INTERFACE = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 OVERLOADS_WITHOUT_DEFAULT_ARGUMENTS = DiagnosticFactory0.create(WARNING); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt index ae9f9346d8d..7667cc426ab 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt @@ -39,7 +39,6 @@ object JvmPlatformConfigurator : PlatformConfiguratorBase( SuspendInFunInterfaceChecker(), BadInheritedJavaSignaturesChecker, JvmMultifileClassStateChecker, - SynchronizedOnInlineMethodChecker, DefaultCheckerInTailrec, FunctionDelegateMemberNameClashChecker, ClassInheritsJavaSealedClassChecker @@ -57,7 +56,7 @@ object JvmPlatformConfigurator : PlatformConfiguratorBase( RuntimeAssertionsOnExtensionReceiverCallChecker, ApiVersionIsAtLeastArgumentsChecker, InconsistentOperatorFromJavaCallChecker, - PolymorphicSignatureCallChecker + PolymorphicSignatureCallChecker, ), additionalTypeCheckers = listOf( @@ -74,7 +73,8 @@ object JvmPlatformConfigurator : PlatformConfiguratorBase( additionalAnnotationCheckers = listOf( FileClassAnnotationsChecker, - ExplicitMetadataChecker + ExplicitMetadataChecker, + SynchronizedAnnotationOnLambdaChecker, ), additionalClashResolvers = listOf( diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/AnnotationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/AnnotationChecker.kt index 125fbf81cdf..b5932774660 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/AnnotationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/AnnotationChecker.kt @@ -101,7 +101,12 @@ class AnnotationChecker( } fun checkExpression(expression: KtExpression, trace: BindingTrace) { - checkEntries(expression.getAnnotationEntries(), getActualTargetList(expression, null, trace.bindingContext), trace) + checkEntries( + expression.getAnnotationEntries(), + getActualTargetList(expression, null, trace.bindingContext), + trace, + expression.parent as? KtAnnotated + ) if (expression is KtCallElement && languageVersionSettings.supportsFeature(ProperCheckAnnotationsTargetInTypeUsePositions)) { val typeArguments = expression.typeArguments.mapNotNull { it.typeReference } for (typeArgument in typeArguments) { diff --git a/compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.fir.kt new file mode 100644 index 00000000000..2a8f629d671 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.fir.kt @@ -0,0 +1,24 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER +// SKIP_TXT + +@Synchronized +suspend fun foo(f: () -> Unit): Unit = f() + +fun builder(c: suspend () -> Unit) {} + +val a: suspend () -> Unit + get() = @Synchronized {} +val b: suspend () -> Unit = @Synchronized {} +val c = builder (@Synchronized {}) +val d = suspend @Synchronized {} +val e = @Synchronized suspend {} + +fun test() { + @Synchronized + suspend fun foo(f: () -> Unit): Unit = f() + + val b: suspend () -> Unit = @Synchronized {} + val c = builder (@Synchronized {}) + val d = suspend @Synchronized {} + val e = @Synchronized suspend {} +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt b/compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt new file mode 100644 index 00000000000..8f185a0335c --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt @@ -0,0 +1,24 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER +// SKIP_TXT + +@Synchronized +suspend fun foo(f: () -> Unit): Unit = f() + +fun builder(c: suspend () -> Unit) {} + +val a: suspend () -> Unit + get() = @Synchronized {} +val b: suspend () -> Unit = @Synchronized {} +val c = builder (@Synchronized {}) +val d = suspend @Synchronized {} +val e = @Synchronized suspend {} + +fun test() { + @Synchronized + suspend fun foo(f: () -> Unit): Unit = f() + + val b: suspend () -> Unit = @Synchronized {} + val c = builder (@Synchronized {}) + val d = suspend @Synchronized {} + val e = @Synchronized suspend {} +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 132f4b19a8d..e494e1b1dea 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -36230,6 +36230,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { public void testSynchronizedOnInline() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnInline.kt"); } + + @Test + @TestMetadata("synchronizedOnSuspend.kt") + public void testSynchronizedOnSuspend() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt"); + } } @Nested diff --git a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index abb1f168353..c04ec4f7463 100644 --- a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -36134,6 +36134,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag public void testSynchronizedOnInline() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnInline.kt"); } + + @Test + @TestMetadata("synchronizedOnSuspend.kt") + public void testSynchronizedOnSuspend() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/inline/synchronizedOnSuspend.kt"); + } } @Nested