From 61f3e776a775db575c419d0aeee76623b70108a2 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Fri, 8 Mar 2019 18:25:14 +0900 Subject: [PATCH] Add 'Covariant equals' inspection #KT-29798 Fixed --- idea/resources/META-INF/plugin-common.xml | 9 +++ .../KotlinCovariantEquals.html | 7 +++ .../KotlinCovariantEqualsInspection.kt | 50 +++++++++++++++ .../covariantEquals/.inspection | 1 + .../inspectionsLocal/covariantEquals/basic.kt | 7 +++ .../covariantEquals/hasOverriddenAnyEquals.kt | 10 +++ .../covariantEquals/inCompanion.kt | 8 +++ .../covariantEquals/inObject.kt | 7 +++ .../covariantEquals/inObjectLiteral.kt | 9 +++ .../inspectionsLocal/covariantEquals/local.kt | 9 +++ .../covariantEquals/override.kt | 13 ++++ .../covariantEquals/overrideAnyEquals.kt | 6 ++ .../covariantEquals/topLevel.kt | 4 ++ .../covariantEquals/twoParameters.kt | 6 ++ .../LocalInspectionTestGenerated.java | 63 +++++++++++++++++++ 15 files changed, 209 insertions(+) create mode 100644 idea/resources/inspectionDescriptions/KotlinCovariantEquals.html create mode 100644 idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCovariantEqualsInspection.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/.inspection create mode 100644 idea/testData/inspectionsLocal/covariantEquals/basic.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/hasOverriddenAnyEquals.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/inCompanion.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/inObject.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/inObjectLiteral.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/local.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/override.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/overrideAnyEquals.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/topLevel.kt create mode 100644 idea/testData/inspectionsLocal/covariantEquals/twoParameters.kt diff --git a/idea/resources/META-INF/plugin-common.xml b/idea/resources/META-INF/plugin-common.xml index a729fc43bd4..4b08dd0ba34 100644 --- a/idea/resources/META-INF/plugin-common.xml +++ b/idea/resources/META-INF/plugin-common.xml @@ -3333,6 +3333,15 @@ language="kotlin" /> + + diff --git a/idea/resources/inspectionDescriptions/KotlinCovariantEquals.html b/idea/resources/inspectionDescriptions/KotlinCovariantEquals.html new file mode 100644 index 00000000000..b7190a5a9fe --- /dev/null +++ b/idea/resources/inspectionDescriptions/KotlinCovariantEquals.html @@ -0,0 +1,7 @@ + + +This inspection reports equals() methods taking an argument type other than Any?. +Only reports if the containing class does not have another equals() method which does take Any? as its argument type. +Normally, this is a mistake. + + \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCovariantEqualsInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCovariantEqualsInspection.kt new file mode 100644 index 00000000000..6ca8b1118ae --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCovariantEqualsInspection.kt @@ -0,0 +1,50 @@ +/* + * Copyright 2010-2019 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.idea.inspections + +import com.intellij.codeInspection.ProblemsHolder +import org.jetbrains.kotlin.builtins.KotlinBuiltIns +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.search.usagesSearch.descriptor +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtObjectDeclaration +import org.jetbrains.kotlin.psi.namedFunctionVisitor +import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.types.typeUtil.isBoolean +import org.jetbrains.kotlin.types.typeUtil.isNullableAny +import org.jetbrains.kotlin.util.OperatorNameConventions + +class KotlinCovariantEqualsInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = namedFunctionVisitor(fun(function) { + if (function.isTopLevel || function.isLocal) return + if (function.nameAsName != OperatorNameConventions.EQUALS) return + val nameIdentifier = function.nameIdentifier ?: return + val classOrObject = function.containingClassOrObject ?: return + if (classOrObject is KtObjectDeclaration && classOrObject.isCompanion()) return + + val parameter = function.valueParameters.singleOrNull() ?: return + val typeReference = parameter.typeReference ?: return + val type = parameter.analyze(BodyResolveMode.PARTIAL)[BindingContext.TYPE, typeReference] ?: return + if (KotlinBuiltIns.isNullableAny(type)) return + + if (classOrObject.body?.children?.any { (it as? KtNamedFunction)?.isEquals() == true } == true) return + + holder.registerProblem(nameIdentifier, "'equals' should take 'Any?' as its argument") + }) + + private fun KtNamedFunction.isEquals(): Boolean { + if (!hasModifier(KtTokens.OVERRIDE_KEYWORD)) return false + if (nameAsName != OperatorNameConventions.EQUALS) return false + val descriptor = this.descriptor as? FunctionDescriptor ?: return false + if (descriptor.valueParameters.singleOrNull()?.type?.isNullableAny() != true) return false + if (descriptor.returnType?.isBoolean() != true) return false + return true + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/covariantEquals/.inspection b/idea/testData/inspectionsLocal/covariantEquals/.inspection new file mode 100644 index 00000000000..0a96723cb18 --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.KotlinCovariantEqualsInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/covariantEquals/basic.kt b/idea/testData/inspectionsLocal/covariantEquals/basic.kt new file mode 100644 index 00000000000..34cfac67a8b --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/basic.kt @@ -0,0 +1,7 @@ +// PROBLEM: 'equals' should take 'Any?' as its argument +// FIX: none +class Foo { + fun equals(other: Foo?): Boolean { + return true + } +} diff --git a/idea/testData/inspectionsLocal/covariantEquals/hasOverriddenAnyEquals.kt b/idea/testData/inspectionsLocal/covariantEquals/hasOverriddenAnyEquals.kt new file mode 100644 index 00000000000..8168b95c215 --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/hasOverriddenAnyEquals.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +class Foo { + fun equals(other: Foo?): Boolean { + return true + } + + override fun equals(other: Any?): Boolean { + return true + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/covariantEquals/inCompanion.kt b/idea/testData/inspectionsLocal/covariantEquals/inCompanion.kt new file mode 100644 index 00000000000..4090273db28 --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/inCompanion.kt @@ -0,0 +1,8 @@ +// PROBLEM: none +class F { + companion object { + fun equals(other: F?): Boolean { + return true + } + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/covariantEquals/inObject.kt b/idea/testData/inspectionsLocal/covariantEquals/inObject.kt new file mode 100644 index 00000000000..61b0988dc4c --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/inObject.kt @@ -0,0 +1,7 @@ +// PROBLEM: 'equals' should take 'Any?' as its argument +// FIX: none +object F { + fun equals(other: F?): Boolean { + return true + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/covariantEquals/inObjectLiteral.kt b/idea/testData/inspectionsLocal/covariantEquals/inObjectLiteral.kt new file mode 100644 index 00000000000..1153e748f70 --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/inObjectLiteral.kt @@ -0,0 +1,9 @@ +// PROBLEM: 'equals' should take 'Any?' as its argument +// FIX: none +interface F + +val f = object : F { + fun equals(other: F?): Boolean { + return true + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/covariantEquals/local.kt b/idea/testData/inspectionsLocal/covariantEquals/local.kt new file mode 100644 index 00000000000..229dd1a2280 --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/local.kt @@ -0,0 +1,9 @@ +// PROBLEM: none +class Foo { + fun test() { + fun equals(other: Foo?): Boolean { + return true + } + equals(null) + } +} diff --git a/idea/testData/inspectionsLocal/covariantEquals/override.kt b/idea/testData/inspectionsLocal/covariantEquals/override.kt new file mode 100644 index 00000000000..3557dda19cb --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/override.kt @@ -0,0 +1,13 @@ +// PROBLEM: 'equals' should take 'Any?' as its argument +// FIX: none +open class Foo { + open fun equals(other: Foo?): Boolean { + return true + } +} + +class Bar : Foo() { + override fun equals(other: Foo?): Boolean { + return true + } +} diff --git a/idea/testData/inspectionsLocal/covariantEquals/overrideAnyEquals.kt b/idea/testData/inspectionsLocal/covariantEquals/overrideAnyEquals.kt new file mode 100644 index 00000000000..96c54d539eb --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/overrideAnyEquals.kt @@ -0,0 +1,6 @@ +// PROBLEM: none +class Foo { + override fun equals(other: Any?): Boolean { + return true + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/covariantEquals/topLevel.kt b/idea/testData/inspectionsLocal/covariantEquals/topLevel.kt new file mode 100644 index 00000000000..39e855a8435 --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/topLevel.kt @@ -0,0 +1,4 @@ +// PROBLEM: none +fun equals(other: Any?): Boolean { + return true +} diff --git a/idea/testData/inspectionsLocal/covariantEquals/twoParameters.kt b/idea/testData/inspectionsLocal/covariantEquals/twoParameters.kt new file mode 100644 index 00000000000..491bbf41be4 --- /dev/null +++ b/idea/testData/inspectionsLocal/covariantEquals/twoParameters.kt @@ -0,0 +1,6 @@ +// PROBLEM: none +class Foo { + fun equals(other: Foo?, other2: Foo?): Boolean { + return true + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index cb090a1fadd..13a4303636c 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -2459,6 +2459,69 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/covariantEquals") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class CovariantEquals extends AbstractLocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInCovariantEquals() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/covariantEquals"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); + } + + @TestMetadata("basic.kt") + public void testBasic() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/basic.kt"); + } + + @TestMetadata("hasOverriddenAnyEquals.kt") + public void testHasOverriddenAnyEquals() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/hasOverriddenAnyEquals.kt"); + } + + @TestMetadata("inCompanion.kt") + public void testInCompanion() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/inCompanion.kt"); + } + + @TestMetadata("inObject.kt") + public void testInObject() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/inObject.kt"); + } + + @TestMetadata("inObjectLiteral.kt") + public void testInObjectLiteral() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/inObjectLiteral.kt"); + } + + @TestMetadata("local.kt") + public void testLocal() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/local.kt"); + } + + @TestMetadata("override.kt") + public void testOverride() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/override.kt"); + } + + @TestMetadata("overrideAnyEquals.kt") + public void testOverrideAnyEquals() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/overrideAnyEquals.kt"); + } + + @TestMetadata("topLevel.kt") + public void testTopLevel() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/topLevel.kt"); + } + + @TestMetadata("twoParameters.kt") + public void testTwoParameters() throws Exception { + runTest("idea/testData/inspectionsLocal/covariantEquals/twoParameters.kt"); + } + } + @TestMetadata("idea/testData/inspectionsLocal/delegationToVarProperty") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)