From b1cd49dd7bf6cf73acb09c662669f11d364e8dee Mon Sep 17 00:00:00 2001 From: Mikhail Zarechenskiy Date: Mon, 10 Sep 2018 05:57:22 +0300 Subject: [PATCH] Prohibit using `kotlin.Result` as a return type in most cases #KT-26659 In Progress --- .../jetbrains/kotlin/diagnostics/Errors.java | 4 + .../rendering/DefaultErrorMessages.java | 2 + .../kotlin/resolve/TargetPlatform.kt | 3 +- .../ResultClassInReturnTypeChecker.kt | 56 +++++++++ .../usageOfResultTypeInReturnType.kt | 116 ++++++++++++++++++ .../usageOfResultTypeInReturnType.txt | 109 ++++++++++++++++ .../DiagnosticsTestWithStdLibGenerated.java | 5 + ...ticsTestWithStdLibUsingJavacGenerated.java | 5 + 8 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ResultClassInReturnTypeChecker.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index fa80e63e671..bb63c050a46 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -334,6 +334,10 @@ public interface Errors { DiagnosticFactory1 RESERVED_MEMBER_INSIDE_INLINE_CLASS = DiagnosticFactory1.create(ERROR); DiagnosticFactory0 SECONDARY_CONSTRUCTOR_WITH_BODY_INSIDE_INLINE_CLASS = DiagnosticFactory0.create(ERROR); + // Result class + + DiagnosticFactory0 RESULT_CLASS_IN_RETURN_TYPE = DiagnosticFactory0.create(ERROR); + // Secondary constructors DiagnosticFactory0 CYCLIC_CONSTRUCTOR_DELEGATION_CALL = DiagnosticFactory0.create(ERROR); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index d1e3b361122..a6ae400a284 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -665,6 +665,8 @@ public class DefaultErrorMessages { MAP.put(RESERVED_MEMBER_INSIDE_INLINE_CLASS, "Member with the name ''{0}'' is reserved for future releases", STRING); MAP.put(SECONDARY_CONSTRUCTOR_WITH_BODY_INSIDE_INLINE_CLASS, "Secondary constructors with bodies are reserved for for future releases"); + MAP.put(RESULT_CLASS_IN_RETURN_TYPE, "'kotlin.Result' cannot be used as a return type"); + MAP.put(VARIANCE_ON_TYPE_PARAMETER_NOT_ALLOWED, "Variance annotations are only allowed for type parameters of classes and interfaces"); MAP.put(BOUND_ON_TYPE_ALIAS_PARAMETER_NOT_ALLOWED, "Bounds are not allowed on type alias parameters"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt index 95ca34e46f3..d40a9babade 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TargetPlatform.kt @@ -120,7 +120,8 @@ private val DEFAULT_DECLARATION_CHECKERS = listOf( InlineClassDeclarationChecker, PropertiesWithBackingFieldsInsideInlineClass(), AnnotationClassTargetAndRetentionChecker(), - ReservedMembersAndConstructsForInlineClass() + ReservedMembersAndConstructsForInlineClass(), + ResultClassInReturnTypeChecker() ) private val DEFAULT_CALL_CHECKERS = listOf( diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ResultClassInReturnTypeChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ResultClassInReturnTypeChecker.kt new file mode 100644 index 00000000000..df0779c4655 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ResultClassInReturnTypeChecker.kt @@ -0,0 +1,56 @@ +/* + * 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.resolve.checkers + +import org.jetbrains.kotlin.builtins.KotlinBuiltIns +import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.diagnostics.reportDiagnosticOnce +import org.jetbrains.kotlin.psi.KtCallableDeclaration +import org.jetbrains.kotlin.psi.KtDeclaration +import org.jetbrains.kotlin.types.KotlinType + +class ResultClassInReturnTypeChecker : DeclarationChecker { + companion object { + private const val RESULT_NAME = "Result" + } + + override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) { + if (declaration !is KtCallableDeclaration || descriptor !is CallableMemberDescriptor) return + + val returnType = descriptor.returnType ?: return + if (isForbiddenReturnType(returnType, descriptor)) { + val typeReferenceOrDeclarationName = declaration.typeReference ?: declaration.nameIdentifier ?: return + context.trace.reportDiagnosticOnce(Errors.RESULT_CLASS_IN_RETURN_TYPE.on(typeReferenceOrDeclarationName)) + } + } + + private fun isForbiddenReturnType(returnType: KotlinType, declarationDescriptor: DeclarationDescriptor): Boolean { + val descriptor = returnType.constructor.declarationDescriptor ?: return false + if (!descriptor.isResultClass()) return false + + if (declarationDescriptor is PropertyDescriptor || declarationDescriptor is PropertyGetterDescriptor) { + val visibility = (declarationDescriptor as DeclarationDescriptorWithVisibility).effectiveVisibility() + return when (visibility) { + is EffectiveVisibility.Private, is EffectiveVisibility.Local, + is EffectiveVisibility.InternalOrPackage, is EffectiveVisibility.InternalProtected, + is EffectiveVisibility.InternalProtectedBound -> false + + is EffectiveVisibility.Public, is EffectiveVisibility.Protected, + is EffectiveVisibility.ProtectedBound -> true + } + } + + return true + } + + private fun DeclarationDescriptor.isResultClass(): Boolean { + val container = containingDeclaration ?: return false + return container is PackageFragmentDescriptor && + container.fqName == KotlinBuiltIns.BUILT_INS_PACKAGE_FQ_NAME && + name.asString() == RESULT_NAME + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.kt new file mode 100644 index 00000000000..ef657379712 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.kt @@ -0,0 +1,116 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER, -UNUSED_EXPRESSION, -UNUSED_VARIABLE +// !LANGUAGE: +InlineClasses + +typealias ResultAlias = Result + +inline class InlineResult(private val r: Result) + +fun params( + r1: Result, + r2: Result?, + r3: ResultAlias, + r4: List>, + r5: InlineResult, + vararg r6: Result +) {} + +class CtorParams(r1: Result) + +fun returnTypePublic(): Result = TODO() +internal fun returnTypeInternal(): Result = TODO() +private fun returnTypePrivate(): Result = TODO() +fun returnTypeNullable(): Result? = TODO() +fun returnTypeAlias(): ResultAlias = TODO() +fun returnInferred(r1: Result, r2: ResultAlias, b: Boolean) = if (b) r1 else r2 + +fun returnTypeInline(): InlineResult = TODO() +fun returnContainer(): List> = TODO() + +val topLevelP: Result = TODO() +val topLevelPInferred = topLevelP +internal val topLevelPInternal: Result = TODO() +private val topLevelPPrivate: Result = TODO() +private val topLevelPPrivateInferred = topLevelP + +val asFunctional: () -> Result = TODO() + +open class PublicCls( + val r1: Result, + val r2: Result?, + val r3: ResultAlias, + val r4: ResultAlias?, + + val r5: InlineResult, + internal val r6: Result, + private val r7: Result, + val r8: List> +) { + val p1: Result = TODO() + var p2: Result = TODO() + val p3: ResultAlias? = TODO() + + val p4 = p1 + + internal val p5: Result = TODO() + private var p6: Result = TODO() + + internal val p7 = p1 + + protected val p8 = p1 + + fun returnInCls(): Result = TODO() + protected fun returnInClsProtected(): Result = TODO() + private fun returnInClsPrivate(): Result = TODO() +} + +internal open class InternalCls( + val r1: Result, + val r2: ResultAlias?, + val r3: List> +) { + companion object { + val cr1: Result = TODO() + private val cr2: Result = TODO() + } + + val p1 = r1 + val p2: Result = TODO() + + protected val p3 = p1 + + fun returnInInternal(): Result = TODO() + protected fun returnInClsProtected(): Result = TODO() +} + +private class PrivateCls( + val r1: Result, + val r2: ResultAlias?, + val r3: List> +) { + companion object { + val cr1: Result = TODO() + private val cr2: Result = TODO() + } + + val p1 = r1 + val p2: Result = TODO() + + fun returnInPrivate(): Result = TODO() +} + +fun local(r: Result) { + val l1: Result? = null + val l2 = r + + fun localFun(): Result = TODO() + + class F { + val p1: Result = r + val p2 = r + } +} + +fun resultInGenericFun(r: Result): T = r as T + +val asFunPublic: () -> Result = TODO() +private val asFun: () -> Result? = TODO() diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.txt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.txt new file mode 100644 index 00000000000..2f64b4383af --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.txt @@ -0,0 +1,109 @@ +package + +private val asFun: () -> kotlin.Result? +public val asFunPublic: () -> kotlin.Result +public val asFunctional: () -> kotlin.Result +public val topLevelP: kotlin.Result +public val topLevelPInferred: kotlin.Result +internal val topLevelPInternal: kotlin.Result +private val topLevelPPrivate: kotlin.Result +private val topLevelPPrivateInferred: kotlin.Result +public fun local(/*0*/ r: kotlin.Result): kotlin.Unit +public fun params(/*0*/ r1: kotlin.Result, /*1*/ r2: kotlin.Result?, /*2*/ r3: ResultAlias /* = kotlin.Result */, /*3*/ r4: kotlin.collections.List>, /*4*/ r5: InlineResult, /*5*/ vararg r6: kotlin.Result /*kotlin.Array>*/): kotlin.Unit +public fun resultInGenericFun(/*0*/ r: kotlin.Result): T +public fun returnContainer(): kotlin.collections.List> +public fun returnInferred(/*0*/ r1: kotlin.Result, /*1*/ r2: ResultAlias /* = kotlin.Result */, /*2*/ b: kotlin.Boolean): kotlin.Result +public fun returnTypeAlias(): ResultAlias /* = kotlin.Result */ +public fun returnTypeInline(): InlineResult +internal fun returnTypeInternal(): kotlin.Result +public fun returnTypeNullable(): kotlin.Result? +private fun returnTypePrivate(): kotlin.Result +public fun returnTypePublic(): kotlin.Result + +public final class CtorParams { + public constructor CtorParams(/*0*/ r1: kotlin.Result) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final inline class InlineResult { + public constructor InlineResult(/*0*/ r: kotlin.Result) + private final val r: kotlin.Result + public open override /*1*/ /*synthesized*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*synthesized*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*synthesized*/ fun toString(): kotlin.String +} + +internal open class InternalCls { + public constructor InternalCls(/*0*/ r1: kotlin.Result, /*1*/ r2: ResultAlias? /* = kotlin.Result? */, /*2*/ r3: kotlin.collections.List>) + public final val p1: kotlin.Result + public final val p2: kotlin.Result + protected final val p3: kotlin.Result + public final val r1: kotlin.Result + public final val r2: ResultAlias? /* = kotlin.Result? */ + public final val r3: kotlin.collections.List> + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + protected final fun returnInClsProtected(): kotlin.Result + public final fun returnInInternal(): kotlin.Result + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public companion object Companion { + private constructor Companion() + public final val cr1: kotlin.Result + private final val cr2: kotlin.Result + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} + +private final class PrivateCls { + public constructor PrivateCls(/*0*/ r1: kotlin.Result, /*1*/ r2: ResultAlias? /* = kotlin.Result? */, /*2*/ r3: kotlin.collections.List>) + public final val p1: kotlin.Result + public final val p2: kotlin.Result + public final val r1: kotlin.Result + public final val r2: ResultAlias? /* = kotlin.Result? */ + public final val r3: kotlin.collections.List> + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final fun returnInPrivate(): kotlin.Result + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public companion object Companion { + private constructor Companion() + public final val cr1: kotlin.Result + private final val cr2: kotlin.Result + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} + +public open class PublicCls { + public constructor PublicCls(/*0*/ r1: kotlin.Result, /*1*/ r2: kotlin.Result?, /*2*/ r3: ResultAlias /* = kotlin.Result */, /*3*/ r4: ResultAlias? /* = kotlin.Result? */, /*4*/ r5: InlineResult, /*5*/ r6: kotlin.Result, /*6*/ r7: kotlin.Result, /*7*/ r8: kotlin.collections.List>) + public final val p1: kotlin.Result + public final var p2: kotlin.Result + public final val p3: ResultAlias? /* = kotlin.Result? */ + public final val p4: kotlin.Result + internal final val p5: kotlin.Result + private final var p6: kotlin.Result + internal final val p7: kotlin.Result + protected final val p8: kotlin.Result + public final val r1: kotlin.Result + public final val r2: kotlin.Result? + public final val r3: ResultAlias /* = kotlin.Result */ + public final val r4: ResultAlias? /* = kotlin.Result? */ + public final val r5: InlineResult + internal final val r6: kotlin.Result + private final val r7: kotlin.Result + public final val r8: kotlin.collections.List> + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final fun returnInCls(): kotlin.Result + private final fun returnInClsPrivate(): kotlin.Result + protected final fun returnInClsProtected(): kotlin.Result + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} +public typealias ResultAlias = kotlin.Result diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java index 1f45d0368fb..5f52ec6d1c3 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java @@ -1658,6 +1658,11 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/unsupported.kt"); } + @TestMetadata("usageOfResultTypeInReturnType.kt") + public void testUsageOfResultTypeInReturnType() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.kt"); + } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/coroutines/callableReference") @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 36e4de4416c..f3eabf132e6 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java @@ -1658,6 +1658,11 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/unsupported.kt"); } + @TestMetadata("usageOfResultTypeInReturnType.kt") + public void testUsageOfResultTypeInReturnType() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/usageOfResultTypeInReturnType.kt"); + } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/coroutines/callableReference") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)