From 0304bd1dc18080fa668f8a082e00a532eb3d5e03 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Fri, 25 Dec 2015 18:37:36 +0300 Subject: [PATCH] More precise diagnostic messages about "operator modifier is not applicable" --- .../jetbrains/kotlin/diagnostics/Errors.java | 2 +- .../rendering/DefaultErrorMessages.java | 2 +- .../kotlin/resolve/OperatorModifierChecker.kt | 13 +- .../descriptors/JavaMethodDescriptor.java | 2 +- .../jetbrains/kotlin/util/OperatorChecker.kt | 225 +++++++++++------- .../AddOperatorModifierIntention.kt | 2 +- .../kotlin/idea/quickfix/ExclExclCallFixes.kt | 4 +- .../replaceGetOrSet/setWithVararg.kt | 2 +- .../toInfixCall/noExplicitReceiver.kt | 1 + 9 files changed, 163 insertions(+), 90 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index c703e14a421..0ad94a702f5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -596,7 +596,7 @@ public interface Errors { DiagnosticFactory0 UNDERSCORE_IS_RESERVED = DiagnosticFactory0.create(ERROR); DiagnosticFactory1 INVALID_CHARACTERS = DiagnosticFactory1.create(ERROR); - DiagnosticFactory0 INAPPLICABLE_OPERATOR_MODIFIER = DiagnosticFactory0.create(ERROR); + DiagnosticFactory1 INAPPLICABLE_OPERATOR_MODIFIER = DiagnosticFactory1.create(ERROR); DiagnosticFactory0 INAPPLICABLE_INFIX_MODIFIER = DiagnosticFactory0.create(ERROR); DiagnosticFactory2 OPERATOR_MODIFIER_REQUIRED = DiagnosticFactory2.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 7ab3b01f7ec..39beb156f6c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -381,7 +381,7 @@ public class DefaultErrorMessages { MAP.put(UNDERSCORE_IS_RESERVED, "Names _, __, ___, ..., are reserved in Kotlin"); MAP.put(INVALID_CHARACTERS, "Name {0}", STRING); - MAP.put(INAPPLICABLE_OPERATOR_MODIFIER, "'operator' modifier is inapplicable on this function"); + MAP.put(INAPPLICABLE_OPERATOR_MODIFIER, "'operator' modifier is inapplicable on this function: {0}", STRING); MAP.put(INAPPLICABLE_INFIX_MODIFIER, "'infix' modifier is inapplicable on this function"); MAP.put(OPERATOR_MODIFIER_REQUIRED, "'operator' modifier is required on ''{0}'' in ''{1}''", NAME, STRING); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OperatorModifierChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OperatorModifierChecker.kt index 848de909af3..598489f08ca 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OperatorModifierChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OperatorModifierChecker.kt @@ -38,6 +38,7 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticSink import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtDeclaration +import org.jetbrains.kotlin.util.CheckResult import org.jetbrains.kotlin.util.OperatorChecks class OperatorModifierChecker : DeclarationChecker { @@ -51,8 +52,14 @@ class OperatorModifierChecker : DeclarationChecker { if (!functionDescriptor.isOperator) return val modifier = declaration.modifierList?.getModifier(KtTokens.OPERATOR_KEYWORD) ?: return - if (!OperatorChecks.canBeOperator(functionDescriptor)) { - diagnosticHolder.report(Errors.INAPPLICABLE_OPERATOR_MODIFIER.on(modifier)) - } + val checkResult = OperatorChecks.checkOperator(functionDescriptor) + if (checkResult.isSuccess) return + + val errorDescription = if (checkResult is CheckResult.IllegalSignature) + checkResult.error + else + "illegal function name" + + diagnosticHolder.report(Errors.INAPPLICABLE_OPERATOR_MODIFIER.on(modifier, errorDescription)) } } \ No newline at end of file diff --git a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/descriptors/JavaMethodDescriptor.java b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/descriptors/JavaMethodDescriptor.java index e38258aa173..3cca55c9bd2 100644 --- a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/descriptors/JavaMethodDescriptor.java +++ b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/descriptors/JavaMethodDescriptor.java @@ -87,7 +87,7 @@ public class JavaMethodDescriptor extends SimpleFunctionDescriptorImpl implement SimpleFunctionDescriptorImpl descriptor = super.initialize( receiverParameterType, dispatchReceiverParameter, typeParameters, unsubstitutedValueParameters, unsubstitutedReturnType, modality, visibility); - setOperator(OperatorChecks.INSTANCE.canBeOperator(descriptor)); + setOperator(OperatorChecks.INSTANCE.checkOperator(descriptor).isSuccess()); return descriptor; } diff --git a/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt b/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt index 7ca5fc8b71e..ca40a6345fb 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/util/OperatorChecker.kt @@ -21,13 +21,19 @@ import org.jetbrains.kotlin.builtins.ReflectionTypes import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor -import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor +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.types.KotlinType 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 @@ -47,84 +53,143 @@ 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 -object OperatorChecks { - fun canBeOperator(functionDescriptor: FunctionDescriptor): Boolean { - val name = functionDescriptor.name - - return with (functionDescriptor) { - if (!functionDescriptor.isMemberOrExtension) return false - - when { - GET == name -> valueParameters.size >= 1 - SET == name -> { - val lastIsOk = valueParameters.lastOrNull()?.let { !it.hasDefaultValue() && it.varargElementType == null } ?: false - valueParameters.size >= 2 && lastIsOk - } - - GET_VALUE == name -> noDefaultsAndVarargs && valueParameters.size >= 2 && valueParameters[1].isKProperty - SET_VALUE == name -> noDefaultsAndVarargs && valueParameters.size >= 3 && valueParameters[1].isKProperty - - INVOKE == name -> isMemberOrExtension - CONTAINS == name -> singleValueParameter && noDefaultsAndVarargs && returnsBoolean - - ITERATOR == name -> noValueParameters - NEXT == name -> noValueParameters - HAS_NEXT == name -> noValueParameters && returnsBoolean - - RANGE_TO == name -> singleValueParameter && noDefaultsAndVarargs - - EQUALS == name -> { - fun DeclarationDescriptor.isAny() = (this as? ClassDescriptor)?.let { KotlinBuiltIns.isAny(it) } ?: false - isMember && overriddenDescriptors.any { it.containingDeclaration.isAny() } - } - COMPARE_TO == name -> returnsInt && singleValueParameter && noDefaultsAndVarargs - - name in BINARY_OPERATION_NAMES -> singleValueParameter && noDefaultsAndVarargs - - name in SIMPLE_UNARY_OPERATION_NAMES -> noValueParameters - - INC == name || DEC == name -> { - val receiver = dispatchReceiverParameter ?: extensionReceiverParameter - isMemberOrExtension && (receiver != null) && (returnType?.let { it.isSubtypeOf(receiver.type) } ?: false) - } - - name in ASSIGNMENT_OPERATIONS -> - returnsUnit && singleValueParameter && noDefaultsAndVarargs - - name.asString().matches(COMPONENT_REGEX) -> noValueParameters - - else -> false - } - } - } - - private val ValueParameterDescriptor.isKProperty: Boolean - get() = ReflectionTypes.createKPropertyStarType(module)?.isSubtypeOf(type.makeNotNullable()) ?: false - - private val FunctionDescriptor.isMember: Boolean - get() = containingDeclaration is ClassDescriptor - - private val FunctionDescriptor.isMemberOrExtension: Boolean - get() = isExtension || containingDeclaration is ClassDescriptor - - private val FunctionDescriptor.noValueParameters: Boolean - get() = valueParameters.isEmpty() - - private val FunctionDescriptor.singleValueParameter: Boolean - get() = valueParameters.size == 1 - - private val FunctionDescriptor.returnsBoolean: Boolean - get() = returnType?.let { KotlinBuiltIns.isBoolean(it) } ?: false - - private val FunctionDescriptor.returnsInt: Boolean - get() = returnType?.let { builtIns.intType == it } ?: false //TODO - - private val FunctionDescriptor.returnsUnit: Boolean - get() = returnType?.let { builtIns.unitType == it } ?: false //TODO - - private val FunctionDescriptor.noDefaultsAndVarargs: Boolean - get() = valueParameters.all { !it.hasDefaultValue() && it.varargElementType == null } - +sealed class CheckResult(val isSuccess: Boolean) { + class IllegalSignature(val error: String) : CheckResult(false) + object IllegalFunctionName : CheckResult(false) + object SuccessCheck : CheckResult(true) } -fun FunctionDescriptor.isValidOperator() = isOperator && OperatorChecks.canBeOperator(this) \ No newline at end of file +interface Check { + val description: String + fun check(functionDescriptor: FunctionDescriptor): Boolean + operator fun invoke(functionDescriptor: FunctionDescriptor): String? = if (!check(functionDescriptor)) description else null +} + +sealed class MemberKindCheck(override val description: String) : Check { + object MemberOrExtension : MemberKindCheck("must be a member or an extension") { + override fun check(functionDescriptor: FunctionDescriptor) = + functionDescriptor.isExtension || functionDescriptor.containingDeclaration is ClassDescriptor + } + object Member : MemberKindCheck("must be a member") { + override fun check(functionDescriptor: FunctionDescriptor) = functionDescriptor.containingDeclaration is ClassDescriptor + } +} + +sealed class ValueParameterCountCheck(override val description: String) : Check { + object NoValueParameters : ValueParameterCountCheck("must have no value parameters") { + override fun check(functionDescriptor: FunctionDescriptor) = functionDescriptor.valueParameters.isEmpty() + } + object SingleValueParameter : ValueParameterCountCheck("must have a single value parameter") { + override fun check(functionDescriptor: FunctionDescriptor) = functionDescriptor.valueParameters.size == 1 + } + 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 + } +} + +private object NoDefaultAndVarargsCheck : Check { + override val description = "should not have varargs or parameters with default values" + override fun check(functionDescriptor: FunctionDescriptor) = + functionDescriptor.valueParameters.all { !it.hasDefaultValue() && it.varargElementType == null } +} + +private object IsKPropertyCheck : Check { + override val description = "second parameter must have a KProperty type or its supertype" + override fun check(functionDescriptor: FunctionDescriptor): Boolean { + val secondParameter = functionDescriptor.valueParameters[1] + return ReflectionTypes.createKPropertyStarType(secondParameter.module)?.isSubtypeOf(secondParameter.type.makeNotNullable()) ?: false + } +} + +sealed class ReturnsCheck(val name: String, val type: KotlinBuiltIns.() -> KotlinType) : Check { + override val description = "must return $name" + override fun check(functionDescriptor: FunctionDescriptor) = functionDescriptor.returnType == functionDescriptor.builtIns.type() + + object ReturnsBoolean : ReturnsCheck("Boolean", { booleanType }) + object ReturnsInt : ReturnsCheck("Int", { intType }) + object ReturnsUnit : ReturnsCheck("Unit", { unitType }) +} + +private class Checks private constructor( + val name: Name?, + val regex: Regex?, + val nameList: Collection?, + val additionalCheck: (FunctionDescriptor) -> String?, + vararg val checks: Check +) { + fun isApplicable(functionDescriptor: FunctionDescriptor): Boolean { + if (name != null && functionDescriptor.name != name) return false + if (regex != null && !functionDescriptor.name.asString().matches(regex)) return false + if (nameList != null && functionDescriptor.name !in nameList) return false + return true + } + + fun checkAll(functionDescriptor: FunctionDescriptor): CheckResult { + for (check in checks) { + val checkResult = check(functionDescriptor) + if (checkResult != null) { + return CheckResult.IllegalSignature(checkResult) + } + } + + val additionalCheckResult = additionalCheck(functionDescriptor) + if (additionalCheckResult != null) { + return CheckResult.IllegalSignature(additionalCheckResult) + } + + return CheckResult.SuccessCheck + } + + constructor(name: Name, vararg checks: Check, additionalChecks: FunctionDescriptor.() -> String? = { null }) + : this(name, null, null, additionalChecks, *checks) + constructor(regex: Regex, vararg checks: Check, additionalChecks: FunctionDescriptor.() -> String? = { null }) + : this(null, regex, null, additionalChecks, *checks) + constructor(nameList: Collection, vararg checks: Check, additionalChecks: FunctionDescriptor.() -> String? = { null }) + : this(null, null, nameList, additionalChecks, *checks) +} + +object OperatorChecks { + private inline fun ensure(cond: Boolean, msg: () -> String) = if (!cond) msg() else null + + private val CHECKS = listOf( + Checks(GET, MemberOrExtension, ValueParameterCountCheck.AtLeast(1)), + Checks(SET, MemberOrExtension, ValueParameterCountCheck.AtLeast(2)) { + val lastIsOk = valueParameters.lastOrNull()?.let { !it.hasDefaultValue() && it.varargElementType == null } ?: false + ensure(lastIsOk) { "last parameter should not have a default value or be a vararg" } + }, + Checks(GET_VALUE, MemberOrExtension, NoDefaultAndVarargsCheck, ValueParameterCountCheck.AtLeast(2), IsKPropertyCheck), + Checks(SET_VALUE, MemberOrExtension, NoDefaultAndVarargsCheck, ValueParameterCountCheck.AtLeast(3), IsKPropertyCheck), + Checks(INVOKE, MemberOrExtension), + Checks(CONTAINS, MemberOrExtension, SingleValueParameter, NoDefaultAndVarargsCheck, ReturnsBoolean), + Checks(ITERATOR, MemberOrExtension, NoValueParameters), + Checks(NEXT, MemberOrExtension, NoValueParameters), + Checks(HAS_NEXT, MemberOrExtension, NoValueParameters, ReturnsBoolean), + Checks(RANGE_TO, MemberOrExtension, SingleValueParameter, NoDefaultAndVarargsCheck), + Checks(EQUALS, Member) { + fun DeclarationDescriptor.isAny() = this is ClassDescriptor && KotlinBuiltIns.isAny(this) + ensure(overriddenDescriptors.any { it.containingDeclaration.isAny() }) { "must override ''equals()'' in Any" } + }, + Checks(COMPARE_TO, MemberOrExtension, ReturnsInt, SingleValueParameter, NoDefaultAndVarargsCheck), + Checks(BINARY_OPERATION_NAMES, MemberOrExtension, SingleValueParameter, NoDefaultAndVarargsCheck), + Checks(SIMPLE_UNARY_OPERATION_NAMES, MemberOrExtension, NoValueParameters), + Checks(listOf(INC, DEC), MemberOrExtension) { + val receiver = dispatchReceiverParameter ?: extensionReceiverParameter + ensure(receiver != null && (returnType?.let { it.isSubtypeOf(receiver.type) } ?: false)) { + "receiver must be a supertype of the return type" + } + }, + Checks(ASSIGNMENT_OPERATIONS, MemberOrExtension, ReturnsUnit, SingleValueParameter, NoDefaultAndVarargsCheck), + Checks(COMPONENT_REGEX, MemberOrExtension, NoValueParameters) + ) + + fun checkOperator(functionDescriptor: FunctionDescriptor): CheckResult { + for (check in CHECKS) { + if (!check.isApplicable(functionDescriptor)) continue + return check.checkAll(functionDescriptor) + } + + return CheckResult.IllegalFunctionName + } +} + +fun FunctionDescriptor.isValidOperator() = isOperator && OperatorChecks.checkOperator(this).isSuccess \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/AddOperatorModifierIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/AddOperatorModifierIntention.kt index 29d1eabc2bf..98d4cb28dbc 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/AddOperatorModifierIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/AddOperatorModifierIntention.kt @@ -28,7 +28,7 @@ class AddOperatorModifierIntention : SelfTargetingRangeIntentionid("0").get(0) \ No newline at end of file