From 8b0ccce4f193dbec5d65c5cb0949434ceba99ce3 Mon Sep 17 00:00:00 2001 From: Dmitry Jemerov Date: Thu, 24 Sep 2015 20:35:21 +0200 Subject: [PATCH] code review; add 'operator' also to next() and hasNext(); check toplevel functions --- .../inspections/OperatorModifierInspection.kt | 29 +++++++++++++------ .../migration/operatorModifier/extension.kt | 5 ++++ .../operatorModifier/extension.kt.after | 5 ++++ .../migration/operatorModifier/toplevel.kt | 7 +++++ .../idea/quickfix/QuickFixTestGenerated.java | 12 ++++++++ 5 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 idea/testData/quickfix/migration/operatorModifier/extension.kt create mode 100644 idea/testData/quickfix/migration/operatorModifier/extension.kt.after create mode 100644 idea/testData/quickfix/migration/operatorModifier/toplevel.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/OperatorModifierInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/OperatorModifierInspection.kt index 546b80d1199..cc2b1f5ec74 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/OperatorModifierInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/OperatorModifierInspection.kt @@ -21,13 +21,15 @@ import com.intellij.openapi.project.Project import com.intellij.psi.PsiElementVisitor import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.idea.caches.resolve.analyze -import org.jetbrains.kotlin.lexer.JetModifierKeywordToken import org.jetbrains.kotlin.lexer.JetTokens +import org.jetbrains.kotlin.psi.JetClassOrObject import org.jetbrains.kotlin.psi.JetModifierListOwner import org.jetbrains.kotlin.psi.JetNamedFunction import org.jetbrains.kotlin.psi.JetVisitorVoid import org.jetbrains.kotlin.psi.psiUtil.getNonStrictParentOfType +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.dataClassUtils.isComponentLike +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.types.expressions.OperatorConventions public class OperatorModifierInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool { @@ -35,9 +37,13 @@ public class OperatorModifierInspection : AbstractKotlinInspection(), CleanupLoc return object : JetVisitorVoid() { override fun visitNamedFunction(function: JetNamedFunction) { val nameIdentifier = function.nameIdentifier - if (nameIdentifier != null && function.isOperator() && !function.hasModifier(JetTokens.OPERATOR_KEYWORD)) { + if (nameIdentifier != null && + function.isMemberOrExtension() && + function.isOperator() && + !function.hasModifier(JetTokens.OPERATOR_KEYWORD)) { + holder.registerProblem(nameIdentifier, "Function defines an operator but isn't annotated as such", - AddModifierLocalQuickFix(JetTokens.OPERATOR_KEYWORD)) + AddModifierLocalQuickFix()) } } } @@ -49,7 +55,9 @@ public class OperatorModifierInspection : AbstractKotlinInspection(), CleanupLoc if (arity == 0 && (name in OperatorConventions.UNARY_OPERATION_NAMES.values() || name == OperatorConventions.ITERATOR || - isComponentLike(name))) { + isComponentLike(name) || + name == OperatorConventions.NEXT || + (name == OperatorConventions.HAS_NEXT && isBooleanReturnType()))) { return true } if (arity == 1 && (name in OperatorConventions.BINARY_OPERATION_NAMES.values() || @@ -70,21 +78,24 @@ public class OperatorModifierInspection : AbstractKotlinInspection(), CleanupLoc return false } + private fun JetNamedFunction.isMemberOrExtension(): Boolean = + receiverTypeReference != null || getStrictParentOfType() != null + private fun JetNamedFunction.isIntReturnType(): Boolean { - return KotlinBuiltIns.isInt(analyze().getType(this) ?: return false) + return KotlinBuiltIns.isInt(analyze(BodyResolveMode.PARTIAL).getType(this) ?: return false) } private fun JetNamedFunction.isBooleanReturnType(): Boolean { - return KotlinBuiltIns.isBoolean(analyze().getType(this) ?: return false) + return KotlinBuiltIns.isBoolean(analyze(BodyResolveMode.PARTIAL).getType(this) ?: return false) } } -class AddModifierLocalQuickFix(val modifier: JetModifierKeywordToken) : LocalQuickFix { - override fun getName(): String = "Add '${modifier.value}' modifier" +private class AddModifierLocalQuickFix() : LocalQuickFix { + override fun getName(): String = "Add 'operator' modifier" override fun getFamilyName(): String = getName() override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val modifierListOwner = descriptor.psiElement.getNonStrictParentOfType() - modifierListOwner?.addModifier(modifier) + modifierListOwner?.addModifier(JetTokens.OPERATOR_KEYWORD) } } diff --git a/idea/testData/quickfix/migration/operatorModifier/extension.kt b/idea/testData/quickfix/migration/operatorModifier/extension.kt new file mode 100644 index 00000000000..a8fd4aee6c9 --- /dev/null +++ b/idea/testData/quickfix/migration/operatorModifier/extension.kt @@ -0,0 +1,5 @@ +// "Add 'operator' modifier" "true" +class A { +} + +fun A.plus(other: A): A = A() diff --git a/idea/testData/quickfix/migration/operatorModifier/extension.kt.after b/idea/testData/quickfix/migration/operatorModifier/extension.kt.after new file mode 100644 index 00000000000..2873e8065e6 --- /dev/null +++ b/idea/testData/quickfix/migration/operatorModifier/extension.kt.after @@ -0,0 +1,5 @@ +// "Add 'operator' modifier" "true" +class A { +} + +operator fun A.plus(other: A): A = A() diff --git a/idea/testData/quickfix/migration/operatorModifier/toplevel.kt b/idea/testData/quickfix/migration/operatorModifier/toplevel.kt new file mode 100644 index 00000000000..4cccaf2cac8 --- /dev/null +++ b/idea/testData/quickfix/migration/operatorModifier/toplevel.kt @@ -0,0 +1,7 @@ +// "Add 'operator' modifier" "false" +// ACTION: Convert to block body +// ACTION: Remove explicit type specification +class A { +} + +fun plus(other: A): A = A() diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 2509da4ed6c..b12b0760c08 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -4201,11 +4201,23 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { doTest(fileName); } + @TestMetadata("extension.kt") + public void testExtension() throws Exception { + String fileName = JetTestUtils.navigationMetadata("idea/testData/quickfix/migration/operatorModifier/extension.kt"); + doTest(fileName); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { String fileName = JetTestUtils.navigationMetadata("idea/testData/quickfix/migration/operatorModifier/simple.kt"); doTest(fileName); } + + @TestMetadata("toplevel.kt") + public void testToplevel() throws Exception { + String fileName = JetTestUtils.navigationMetadata("idea/testData/quickfix/migration/operatorModifier/toplevel.kt"); + doTest(fileName); + } } @TestMetadata("idea/testData/quickfix/migration/removeNameFromFunctionExpression")