More precise diagnostic messages about "operator modifier is not applicable"

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java

@@ -596,7 +596,7 @@ public interface Errors {
     DiagnosticFactory0<PsiElement> UNDERSCORE_IS_RESERVED = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory1<PsiElement, String> INVALID_CHARACTERS = DiagnosticFactory1.create(ERROR);
 
-    DiagnosticFactory0<PsiElement> INAPPLICABLE_OPERATOR_MODIFIER = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory1<PsiElement, String> INAPPLICABLE_OPERATOR_MODIFIER = DiagnosticFactory1.create(ERROR);
     DiagnosticFactory0<PsiElement> INAPPLICABLE_INFIX_MODIFIER = DiagnosticFactory0.create(ERROR);
 
     DiagnosticFactory2<PsiElement, FunctionDescriptor, String> OPERATOR_MODIFIER_REQUIRED = DiagnosticFactory2.create(ERROR);

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);

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)) {
+        val checkResult = OperatorChecks.checkOperator(functionDescriptor)
-            diagnosticHolder.report(Errors.INAPPLICABLE_OPERATOR_MODIFIER.on(modifier))
+        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))
     }
 }

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;
     }
 

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 {
+sealed class CheckResult(val isSuccess: Boolean) {
-    fun canBeOperator(functionDescriptor: FunctionDescriptor): Boolean {
+    class IllegalSignature(val error: String) : CheckResult(false)
-        val name = functionDescriptor.name
+    object IllegalFunctionName : CheckResult(false)
-
+    object SuccessCheck : CheckResult(true)
-        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 }
-
 }
 
-fun FunctionDescriptor.isValidOperator() = isOperator && OperatorChecks.canBeOperator(this)
+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<Name>?,
+        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<Name>, 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

idea/src/org/jetbrains/kotlin/idea/intentions/AddOperatorModifierIntention.kt

@@ -28,7 +28,7 @@ class AddOperatorModifierIntention : SelfTargetingRangeIntention<KtNamedFunction
     override fun applicabilityRange(element: KtNamedFunction): TextRange? {
         val nameIdentifier = element.nameIdentifier ?: return null
         val functionDescriptor = element.resolveToDescriptor() as? FunctionDescriptor ?: return null
-        if (functionDescriptor.isOperator || !OperatorChecks.canBeOperator(functionDescriptor)) return null
+        if (functionDescriptor.isOperator || !OperatorChecks.checkOperator(functionDescriptor).isSuccess) return null
         return nameIdentifier.textRange
     }
 

idea/src/org/jetbrains/kotlin/idea/quickfix/ExclExclCallFixes.kt

@@ -37,8 +37,8 @@ import org.jetbrains.kotlin.psi.KtPostfixExpression
 import org.jetbrains.kotlin.psi.KtPsiFactory
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.types.TypeUtils
-import org.jetbrains.kotlin.util.OperatorChecks
 import org.jetbrains.kotlin.util.OperatorNameConventions
+import org.jetbrains.kotlin.util.isValidOperator
 
 abstract class ExclExclCallFix : IntentionAction {
     override fun getFamilyName(): String = text
@@ -126,7 +126,7 @@ object MissingIteratorExclExclFixFactory : KotlinSingleIntentionActionFactory()
             val memberScope = descriptor.unsubstitutedMemberScope
             val functions = memberScope.getContributedFunctions(OperatorNameConventions.ITERATOR, NoLookupLocation.FROM_IDE)
 
-            return functions.any { it.isOperator && OperatorChecks.canBeOperator(it) }
+            return functions.any { it.isValidOperator() }
         }
 
         when (descriptor) {

idea/testData/intentions/conventionNameCalls/replaceGetOrSet/setWithVararg.kt

@@ -1,5 +1,5 @@
 // IS_APPLICABLE: false
-// ERROR: operator modifier is inapplicable on this function: last parameter must not have a default value or be a vararg
+// ERROR: operator modifier is inapplicable on this function: last parameter should not have a default value or be a vararg
 
 class C {
     operator fun set(s: String, vararg value: Int): Boolean = true

idea/testData/intentions/toInfixCall/noExplicitReceiver.kt

@@ -1,3 +1,4 @@
 // IS_APPLICABLE: false
+// ERROR: 'infix' modifier is inapplicable on this function
 infix fun id(s: String) = s
 val x = <caret>id("0").get(0)