From dead2516c291b04f4e1d23c099d14f001598928c Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 4 Sep 2018 15:45:26 +0300 Subject: [PATCH] Redundant override inspection: don't report for ambiguous derivation #KT-24405 Fixed #KT-25883 Fixed --- .../KotlinRedundantOverrideInspection.kt | 45 ++++++++++++------- .../abstractClassAndInterface.kt | 13 ++++++ .../redundantOverride/boxedParameters.kt | 10 +++++ .../redundantOverride/classAndInterface.kt | 11 +++++ .../classAndInterface.kt.after | 10 +++++ .../redundantOverride/twoInterfaces.kt | 13 ++++++ .../LocalInspectionTestGenerated.java | 20 +++++++++ 7 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 idea/testData/inspectionsLocal/redundantOverride/abstractClassAndInterface.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/boxedParameters.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt create mode 100644 idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantOverride/twoInterfaces.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt index 192dbbe64c9..ae3f043afee 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt @@ -8,13 +8,16 @@ package org.jetbrains.kotlin.idea.inspections import com.intellij.codeInspection.* import com.intellij.openapi.project.Project import com.intellij.openapi.util.TextRange +import org.jetbrains.kotlin.descriptors.ClassDescriptor +import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.* import org.jetbrains.kotlin.resolve.BindingContext -import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall -import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) = @@ -49,7 +52,7 @@ class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLoc val superCallElement = qualifiedExpression.selectorExpression as? KtCallElement ?: return if (!isSameFunctionName(superCallElement, function)) return if (!isSameArguments(superCallElement, function)) return - if (function.isDefinedInDelegatedSuperType(qualifiedExpression)) return + if (function.isAmbiguouslyDerived()) return val descriptor = holder.manager.createProblemDescriptor( function, @@ -90,23 +93,31 @@ class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLoc } } -private fun KtNamedFunction.isDefinedInDelegatedSuperType(superQualifiedExpression: KtDotQualifiedExpression): Boolean { +private fun KtNamedFunction.isAmbiguouslyDerived(): Boolean { + val context = analyze() + val original = context[BindingContext.FUNCTION, this]?.original + val overriddenDescriptors = original?.overriddenDescriptors ?: return false + if (overriddenDescriptors.size < 2) return false + // Two+ functions + // At least one default in interface or abstract in class, or just something from Java + if (overriddenDescriptors.any { overriddenFunction -> + overriddenFunction is JavaMethodDescriptor || when ((overriddenFunction.containingDeclaration as? ClassDescriptor)?.kind) { + ClassKind.CLASS -> overriddenFunction.modality == Modality.ABSTRACT + ClassKind.INTERFACE -> overriddenFunction.modality != Modality.ABSTRACT + else -> false + } + + } + ) return true + val delegatedSuperTypeEntries = containingClassOrObject?.superTypeListEntries?.filterIsInstance() ?: return false if (delegatedSuperTypeEntries.isEmpty()) return false - - val context = superQualifiedExpression.analyze() - val delegatedSuperTypes = delegatedSuperTypeEntries.mapNotNull { entry -> - context[BindingContext.TYPE, entry.typeReference] + val delegatedSuperDeclarations = delegatedSuperTypeEntries.mapNotNull { entry -> + context[BindingContext.TYPE, entry.typeReference]?.constructor?.declarationDescriptor } - - val superResolvedCall = superQualifiedExpression.getResolvedCall(context) ?: return false - val superCallResolvedDescriptor = superResolvedCall.resultingDescriptor - val superCallResolvedReceiverTypes = superCallResolvedDescriptor.overriddenDescriptors.mapNotNull { it.dispatchReceiverParameter?.type } - - return delegatedSuperTypes.any { delegatedSuperType -> - superCallResolvedReceiverTypes.any { superCallReceiverType -> - delegatedSuperType.isSubtypeOf(superCallReceiverType) - } + return overriddenDescriptors.any { + it.containingDeclaration in delegatedSuperDeclarations } } + diff --git a/idea/testData/inspectionsLocal/redundantOverride/abstractClassAndInterface.kt b/idea/testData/inspectionsLocal/redundantOverride/abstractClassAndInterface.kt new file mode 100644 index 00000000000..0cd52570853 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/abstractClassAndInterface.kt @@ -0,0 +1,13 @@ +// PROBLEM: none + +abstract class AbstractClass { + abstract fun foo(): Int +} + +interface Interface { + fun foo(): Int = 3 +} + +class ChildClass : AbstractClass(), Interface { + override fun foo() = super.foo() +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/boxedParameters.kt b/idea/testData/inspectionsLocal/redundantOverride/boxedParameters.kt new file mode 100644 index 00000000000..26277a63356 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/boxedParameters.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +// WITH_RUNTIME + +interface Foo { + fun add(i: Int): Boolean +} + +class Bar: ArrayList(), Foo { + override fun add(i: Int) = super.add(i) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt b/idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt new file mode 100644 index 00000000000..dac5b559441 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt @@ -0,0 +1,11 @@ +open class Class { + open fun foo(): Int = 4 +} + +interface Interface { + fun foo(): Int +} + +class ChildClass : Class(), Interface { + override fun foo() = super.foo() +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt.after b/idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt.after new file mode 100644 index 00000000000..5ecff074d5d --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt.after @@ -0,0 +1,10 @@ +open class Class { + open fun foo(): Int = 4 +} + +interface Interface { + fun foo(): Int +} + +class ChildClass : Class(), Interface { +} diff --git a/idea/testData/inspectionsLocal/redundantOverride/twoInterfaces.kt b/idea/testData/inspectionsLocal/redundantOverride/twoInterfaces.kt new file mode 100644 index 00000000000..67bd77c7263 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/twoInterfaces.kt @@ -0,0 +1,13 @@ +// PROBLEM: none + +interface Foo { + fun test() +} + +interface Gav { + fun test() {} +} + +class TwoInterfaces : Foo, Gav { + override fun test() = super.test() +} \ 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 442b25671c0..bf30541c0a0 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -4027,6 +4027,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); } + @TestMetadata("abstractClassAndInterface.kt") + public void testAbstractClassAndInterface() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/abstractClassAndInterface.kt"); + } + public void testAllFilesPresentInRedundantOverride() throws Exception { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/redundantOverride"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); } @@ -4051,11 +4056,21 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/redundantOverride/basic.kt"); } + @TestMetadata("boxedParameters.kt") + public void testBoxedParameters() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/boxedParameters.kt"); + } + @TestMetadata("callDifferentSuperMethod.kt") public void testCallDifferentSuperMethod() throws Exception { runTest("idea/testData/inspectionsLocal/redundantOverride/callDifferentSuperMethod.kt"); } + @TestMetadata("classAndInterface.kt") + public void testClassAndInterface() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/classAndInterface.kt"); + } + @TestMetadata("dataClass.kt") public void testDataClass() throws Exception { runTest("idea/testData/inspectionsLocal/redundantOverride/dataClass.kt"); @@ -4101,6 +4116,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt"); } + @TestMetadata("twoInterfaces.kt") + public void testTwoInterfaces() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/twoInterfaces.kt"); + } + @TestMetadata("useGenericsSuper.kt") public void testUseGenericsSuper() throws Exception { runTest("idea/testData/inspectionsLocal/redundantOverride/useGenericsSuper.kt");