From 8f4ee0528ef7ec23e38b6188d866dc1c7a546642 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 19 May 2016 11:45:03 +0300 Subject: [PATCH] Implement operator checks for controller's handleResult --- .../kotlin/coroutines/coroutineUtil.kt | 19 ++----- .../kotlin/resolve/calls/CallResolverUtil.kt | 4 +- .../tests/coroutines/lambdaExpectedType.kt | 6 +-- .../tests/coroutines/lambdaExpectedType.txt | 6 +-- .../tests/coroutines/wrongHandleResult.kt | 34 ++++++++++++ .../tests/coroutines/wrongHandleResult.txt | 52 +++++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 6 +++ .../kotlin/resolve/DescriptorUtils.java | 1 + .../jetbrains/kotlin/util/OperatorChecker.kt | 37 +++++++++---- .../kotlin/util/OperatorNameConventions.kt | 1 + 10 files changed, 135 insertions(+), 31 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.kt create mode 100644 compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/coroutines/coroutineUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/coroutines/coroutineUtil.kt index f04f9e3a417..857f1effe43 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/coroutines/coroutineUtil.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/coroutines/coroutineUtil.kt @@ -17,25 +17,14 @@ package org.jetbrains.kotlin.coroutines import org.jetbrains.kotlin.descriptors.SimpleFunctionDescriptor -import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe import org.jetbrains.kotlin.types.KotlinType - -@JvmField -val CONTINUATION_INTERFACE_FQ_NAME = FqName("kotlin.coroutines.Continuation") - -@JvmField -val HANDLE_RESULT_NAME = Name.identifier("handleResult") +import org.jetbrains.kotlin.util.OperatorNameConventions /** - * @returns type of first value parameter if function is 'operator handleResult' in coroutine controller + * @returns type of first value parameter if function is 'operator handleResult' in coroutines controller */ fun SimpleFunctionDescriptor.getExpectedTypeForCoroutineControllerHandleResult(): KotlinType? { - if (name != HANDLE_RESULT_NAME) return null - if (valueParameters.size != 2) return null + if (!isOperator || name != OperatorNameConventions.COROUTINE_HANDLE_RESULT) return null - if (valueParameters[1].type.constructor.declarationDescriptor?.fqNameUnsafe != CONTINUATION_INTERFACE_FQ_NAME.toUnsafe()) return null - - return valueParameters[0].type + return valueParameters.getOrNull(0)?.type } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt index 0c6b26fe69c..6204a6948d4 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt @@ -179,7 +179,9 @@ fun getEffectiveExpectedType(parameterDescriptor: ValueParameterDescriptor, argu val newExpectedLambdaReturnType = receiverType.memberScope - .getContributedFunctions(HANDLE_RESULT_NAME, KotlinLookupLocation(argument.asElement())).mapNotNull { + .getContributedFunctions( + OperatorNameConventions.COROUTINE_HANDLE_RESULT, KotlinLookupLocation(argument.asElement()) + ).mapNotNull { it.getExpectedTypeForCoroutineControllerHandleResult() }.singleOrNull() // If no handleResult function found, then expected return type for lambda is Unit diff --git a/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.kt b/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.kt index 44e41bf06a1..2fd72507c20 100644 --- a/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.kt +++ b/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.kt @@ -1,15 +1,15 @@ // !CHECK_TYPE // !DIAGNOSTICS: -UNUSED_PARAMETER -ASSIGNED_BUT_NEVER_ACCESSED_VARIABLE -UNUSED_VALUE -UNUSED_VARIABLE class IntController { - fun handleResult(x: Int, c: Continuation) { } + operator fun handleResult(x: Int, c: Continuation) { } } class GenericController { - fun handleResult(x: T, c: Continuation) { } + operator fun handleResult(x: T, c: Continuation) { } } class UnitController { - fun handleResult(x: Unit, c: Continuation) { } + operator fun handleResult(x: Unit, c: Continuation) { } } class EmptyController diff --git a/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.txt b/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.txt index 30a4ea3335e..c1fe5f89928 100644 --- a/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.txt +++ b/compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.txt @@ -18,7 +18,7 @@ public final class EmptyController { public final class GenericController { public constructor GenericController() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final fun handleResult(/*0*/ x: T, /*1*/ c: kotlin.coroutines.Continuation): kotlin.Unit + public final operator fun handleResult(/*0*/ x: T, /*1*/ c: kotlin.coroutines.Continuation): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } @@ -26,7 +26,7 @@ public final class GenericController { public final class IntController { public constructor IntController() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final fun handleResult(/*0*/ x: kotlin.Int, /*1*/ c: kotlin.coroutines.Continuation): kotlin.Unit + public final operator fun handleResult(/*0*/ x: kotlin.Int, /*1*/ c: kotlin.coroutines.Continuation): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } @@ -34,7 +34,7 @@ public final class IntController { public final class UnitController { public constructor UnitController() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final fun handleResult(/*0*/ x: kotlin.Unit, /*1*/ c: kotlin.coroutines.Continuation): kotlin.Unit + public final operator fun handleResult(/*0*/ x: kotlin.Unit, /*1*/ c: kotlin.coroutines.Continuation): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } diff --git a/compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.kt b/compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.kt new file mode 100644 index 00000000000..d59cc53e7fd --- /dev/null +++ b/compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.kt @@ -0,0 +1,34 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER +class A1 { + operator fun handleResult(x: Int) { + } +} + +class A2 { + operator fun handleResult(x: Int, y: Int) { + } +} + +class A3 { + operator fun handleResult(x: Int, y: Continuation) { + } +} + +class A4 { + operator fun handleResult(x: Int, y: Continuation) { + } +} + +class A5 { + // TODO: Allow? + operator fun handleResult(x: Int, y: Continuation): Int = 1 +} + +class A6 { + operator fun handleResult(x: Int, y: Continuation) { + } + + operator fun handleResult(x: String = "", y: Continuation) {} + operator fun handleResult(x: Double = 1.0, vararg y: Continuation) {} + operator fun handleResult(x: Double = 1.0, z: Int, y: Continuation) {} +} diff --git a/compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.txt b/compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.txt new file mode 100644 index 00000000000..caa6fe76a40 --- /dev/null +++ b/compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.txt @@ -0,0 +1,52 @@ +package + +public final class A1 { + public constructor A1() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun handleResult(/*0*/ x: kotlin.Int): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class A2 { + public constructor A2() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun handleResult(/*0*/ x: kotlin.Int, /*1*/ y: kotlin.Int): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class A3 { + public constructor A3() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun handleResult(/*0*/ x: kotlin.Int, /*1*/ y: kotlin.coroutines.Continuation): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class A4 { + public constructor A4() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun handleResult(/*0*/ x: kotlin.Int, /*1*/ y: kotlin.coroutines.Continuation): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class A5 { + public constructor A5() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun handleResult(/*0*/ x: kotlin.Int, /*1*/ y: kotlin.coroutines.Continuation): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class A6 { + public constructor A6() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun handleResult(/*0*/ x: kotlin.Double = ..., /*1*/ vararg y: kotlin.coroutines.Continuation /*kotlin.Array>*/): kotlin.Unit + public final operator fun handleResult(/*0*/ x: kotlin.Double = ..., /*1*/ z: kotlin.Int, /*2*/ y: kotlin.coroutines.Continuation): kotlin.Unit + public final operator fun handleResult(/*0*/ x: kotlin.Int, /*1*/ y: kotlin.coroutines.Continuation): kotlin.Unit + public final operator fun handleResult(/*0*/ x: kotlin.String = ..., /*1*/ y: kotlin.coroutines.Continuation): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 339a90473c1..d6e517ff02d 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -3878,6 +3878,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/lambdaExpectedType.kt"); doTest(fileName); } + + @TestMetadata("wrongHandleResult.kt") + public void testWrongHandleResult() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/coroutines/wrongHandleResult.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/diagnostics/tests/cyclicHierarchy") diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.java b/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.java index 29fa27de045..c0ee7838fc3 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.java +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.java @@ -51,6 +51,7 @@ public class DescriptorUtils { public static final FqName JVM_NAME = new FqName("kotlin.jvm.JvmName"); public static final FqName VOLATILE = new FqName("kotlin.jvm.Volatile"); public static final FqName SYNCHRONIZED = new FqName("kotlin.jvm.Synchronized"); + public static final FqName CONTINUATION_INTERFACE_FQ_NAME = new FqName("kotlin.coroutines.Continuation"); private DescriptorUtils() { } diff --git a/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt b/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt index ca40a6345fb..cdb18eab35f 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt @@ -22,23 +22,20 @@ import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns -import org.jetbrains.kotlin.resolve.descriptorUtil.hasDefaultValue -import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension -import org.jetbrains.kotlin.resolve.descriptorUtil.module +import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.descriptorUtil.* import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.types.typeUtil.isNothing import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf import org.jetbrains.kotlin.types.typeUtil.makeNotNullable import org.jetbrains.kotlin.util.MemberKindCheck.Member import org.jetbrains.kotlin.util.MemberKindCheck.MemberOrExtension -import org.jetbrains.kotlin.util.ReturnsCheck.* -import org.jetbrains.kotlin.util.ValueParameterCountCheck.NoValueParameters -import org.jetbrains.kotlin.util.ValueParameterCountCheck.SingleValueParameter import org.jetbrains.kotlin.util.OperatorNameConventions.ASSIGNMENT_OPERATIONS import org.jetbrains.kotlin.util.OperatorNameConventions.BINARY_OPERATION_NAMES import org.jetbrains.kotlin.util.OperatorNameConventions.COMPARE_TO import org.jetbrains.kotlin.util.OperatorNameConventions.COMPONENT_REGEX import org.jetbrains.kotlin.util.OperatorNameConventions.CONTAINS +import org.jetbrains.kotlin.util.OperatorNameConventions.COROUTINE_HANDLE_RESULT import org.jetbrains.kotlin.util.OperatorNameConventions.DEC import org.jetbrains.kotlin.util.OperatorNameConventions.EQUALS import org.jetbrains.kotlin.util.OperatorNameConventions.GET @@ -52,6 +49,9 @@ import org.jetbrains.kotlin.util.OperatorNameConventions.RANGE_TO import org.jetbrains.kotlin.util.OperatorNameConventions.SET import org.jetbrains.kotlin.util.OperatorNameConventions.SET_VALUE import org.jetbrains.kotlin.util.OperatorNameConventions.SIMPLE_UNARY_OPERATION_NAMES +import org.jetbrains.kotlin.util.ReturnsCheck.* +import org.jetbrains.kotlin.util.ValueParameterCountCheck.NoValueParameters +import org.jetbrains.kotlin.util.ValueParameterCountCheck.SingleValueParameter sealed class CheckResult(val isSuccess: Boolean) { class IllegalSignature(val error: String) : CheckResult(false) @@ -85,6 +85,9 @@ sealed class ValueParameterCountCheck(override val description: String) : Check class AtLeast(val n: Int) : ValueParameterCountCheck("must have at least $n value parameter" + (if (n > 1) "s" else "")) { override fun check(functionDescriptor: FunctionDescriptor) = functionDescriptor.valueParameters.size >= n } + class Equals(val n: Int) : ValueParameterCountCheck("must have exactly $n value parameters") { + override fun check(functionDescriptor: FunctionDescriptor) = functionDescriptor.valueParameters.size == n + } } private object NoDefaultAndVarargsCheck : Check { @@ -93,6 +96,11 @@ private object NoDefaultAndVarargsCheck : Check { functionDescriptor.valueParameters.all { !it.hasDefaultValue() && it.varargElementType == null } } +private object NoTypeParametersCheck : Check { + override val description = "should not have type parameters" + override fun check(functionDescriptor: FunctionDescriptor) = functionDescriptor.typeParameters.isEmpty() +} + private object IsKPropertyCheck : Check { override val description = "second parameter must have a KProperty type or its supertype" override fun check(functionDescriptor: FunctionDescriptor): Boolean { @@ -179,7 +187,18 @@ object OperatorChecks { } }, Checks(ASSIGNMENT_OPERATIONS, MemberOrExtension, ReturnsUnit, SingleValueParameter, NoDefaultAndVarargsCheck), - Checks(COMPONENT_REGEX, MemberOrExtension, NoValueParameters) + Checks(COMPONENT_REGEX, MemberOrExtension, NoValueParameters), + Checks( + COROUTINE_HANDLE_RESULT, Member, ValueParameterCountCheck.Equals(2), ReturnsUnit, + NoDefaultAndVarargsCheck, NoTypeParametersCheck) { + val secondParameter = valueParameters[1] + ensure( + secondParameter.type.constructor.declarationDescriptor?.fqNameUnsafe == DescriptorUtils.CONTINUATION_INTERFACE_FQ_NAME.toUnsafe() + && secondParameter.type.arguments[0].type.isNothing() + ) { + "Second parameter should be Continuation" + } + } ) fun checkOperator(functionDescriptor: FunctionDescriptor): CheckResult { @@ -192,4 +211,4 @@ object OperatorChecks { } } -fun FunctionDescriptor.isValidOperator() = isOperator && OperatorChecks.checkOperator(this).isSuccess \ No newline at end of file +fun FunctionDescriptor.isValidOperator() = isOperator && OperatorChecks.checkOperator(this).isSuccess diff --git a/core/descriptors/src/org/jetbrains/kotlin/util/OperatorNameConventions.kt b/core/descriptors/src/org/jetbrains/kotlin/util/OperatorNameConventions.kt index bb64efb2986..ac423719a28 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/util/OperatorNameConventions.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/util/OperatorNameConventions.kt @@ -56,6 +56,7 @@ object OperatorNameConventions { @JvmField val MOD_ASSIGN = Name.identifier("modAssign") @JvmField val PLUS_ASSIGN = Name.identifier("plusAssign") @JvmField val MINUS_ASSIGN = Name.identifier("minusAssign") + @JvmField val COROUTINE_HANDLE_RESULT = Name.identifier("handleResult") // If you add new unary, binary or assignment operators, add it to OperatorConventions as well