diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt index 5f3b20bb718..4abf6f5415d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinRedundantOverrideInspection.kt @@ -10,13 +10,19 @@ 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.FunctionDescriptor 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.synthetic.SyntheticJavaPropertyDescriptor +import org.jetbrains.kotlin.synthetic.canBePropertyAccessor +import org.jetbrains.kotlin.types.typeUtil.isUnit +import org.jetbrains.kotlin.types.typeUtil.makeNotNullable class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) = @@ -32,8 +38,7 @@ class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLoc val qualifiedExpression = when (bodyExpression) { is KtDotQualifiedExpression -> bodyExpression is KtBlockExpression -> { - val body = bodyExpression.statements.singleOrNull() - when (body) { + when (val body = bodyExpression.statements.singleOrNull()) { is KtReturnExpression -> body.returnedExpression is KtDotQualifiedExpression -> body.takeIf { _ -> function.typeReference.let { it == null || it.text == "Unit" } @@ -51,7 +56,10 @@ class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLoc val superCallElement = qualifiedExpression.selectorExpression as? KtCallElement ?: return if (!isSameFunctionName(superCallElement, function)) return if (!isSameArguments(superCallElement, function)) return - if (function.isAmbiguouslyDerived()) return + val context = function.analyze() + val functionDescriptor = context[BindingContext.FUNCTION, function] + if (function.hasDerivedProperty(functionDescriptor, context)) return + if (function.isAmbiguouslyDerived(functionDescriptor, context)) return val descriptor = holder.manager.createProblemDescriptor( function, @@ -77,6 +85,25 @@ class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLoc val superCallMethodName = superSelectorExpression.getCallNameExpression()?.text ?: return false return function.name == superCallMethodName } + + private fun KtNamedFunction.hasDerivedProperty(functionDescriptor: FunctionDescriptor?, context: BindingContext): Boolean { + if (functionDescriptor == null) return false + val functionName = nameAsName ?: return false + if (!canBePropertyAccessor(functionName.asString())) return false + val functionType = functionDescriptor.returnType + val isSetter = functionType?.isUnit() == true + val valueParameters = valueParameters + val singleValueParameter = valueParameters.singleOrNull() + if (isSetter && singleValueParameter == null || !isSetter && valueParameters.isNotEmpty()) return false + val propertyType = + (if (isSetter) context[BindingContext.VALUE_PARAMETER, singleValueParameter]?.type else functionType)?.makeNotNullable() + return SyntheticJavaPropertyDescriptor.propertyNamesByAccessorName(functionName).any { + val propertyName = it.asString() + containingClassOrObject?.declarations?.find { d -> + d is KtProperty && d.name == propertyName && d.resolveToDescriptorIfAny()?.type?.makeNotNullable() == propertyType + } != null + } + } private class RedundantOverrideFix : LocalQuickFix { override fun getName() = "Remove redundant overriding method" @@ -92,9 +119,8 @@ class KotlinRedundantOverrideInspection : AbstractKotlinInspection(), CleanupLoc } } -private fun KtNamedFunction.isAmbiguouslyDerived(): Boolean { - val context = analyze() - val original = context[BindingContext.FUNCTION, this]?.original +private fun KtNamedFunction.isAmbiguouslyDerived(functionDescriptor: FunctionDescriptor?, context: BindingContext): Boolean { + val original = functionDescriptor?.original val overriddenDescriptors = original?.overriddenDescriptors ?: return false if (overriddenDescriptors.size < 2) return false // Two+ functions diff --git a/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty.kt b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty.kt new file mode 100644 index 00000000000..4a9892cda69 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +open class A { + open fun getFoo(): String? = null +} + +class B : A() { + private val foo = "" + + override fun getFoo(): String? = super.getFoo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty2.kt b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty2.kt new file mode 100644 index 00000000000..91fd1d0c76a --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty2.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +open class A { + open fun isFoo(): Boolean = true +} + +class B : A() { + private val isFoo: Boolean = false + + override fun isFoo(): Boolean = super.isFoo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty3.kt b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty3.kt new file mode 100644 index 00000000000..e2e51d3ee9f --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty3.kt @@ -0,0 +1,9 @@ +open class A { + open fun getFoo(i: Int): String? = null +} + +class B : A() { + private val foo = "" + + override fun getFoo(i: Int): String? = super.getFoo(i) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty3.kt.after b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty3.kt.after new file mode 100644 index 00000000000..9480d06c52a --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty3.kt.after @@ -0,0 +1,8 @@ +open class A { + open fun getFoo(i: Int): String? = null +} + +class B : A() { + private val foo = "" + +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty4.kt b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty4.kt new file mode 100644 index 00000000000..d7b03994d22 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty4.kt @@ -0,0 +1,9 @@ +open class A { + open fun isFoo(): Boolean = true +} + +class B : A() { + private val isFoo: Int = 42 + + override fun isFoo(): Boolean = super.isFoo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty4.kt.after b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty4.kt.after new file mode 100644 index 00000000000..c29b6568e7a --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty4.kt.after @@ -0,0 +1,8 @@ +open class A { + open fun isFoo(): Boolean = true +} + +class B : A() { + private val isFoo: Int = 42 + +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty.kt b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty.kt new file mode 100644 index 00000000000..5b33c5ab235 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +open class A { + open fun setFoo(s: String) = Unit +} + +class B : A() { + private var foo: String = "" + + override fun setFoo(s: String) = super.setFoo(s) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty2.kt b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty2.kt new file mode 100644 index 00000000000..068f71093a8 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty2.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +open class A { + open fun setFoo(s: String) = Unit +} + +class B : A() { + private var isFoo: String = "" + + override fun setFoo(s: String) = super.setFoo(s) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty3.kt b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty3.kt new file mode 100644 index 00000000000..d75d14e6c7a --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty3.kt @@ -0,0 +1,9 @@ +open class A { + open fun setFoo(s: String, i: Int) = Unit +} + +class B : A() { + private var foo: String = "" + + override fun setFoo(s: String, i: Int) = super.setFoo(s, i) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty3.kt.after b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty3.kt.after new file mode 100644 index 00000000000..abdcc39d8da --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty3.kt.after @@ -0,0 +1,8 @@ +open class A { + open fun setFoo(s: String, i: Int) = Unit +} + +class B : A() { + private var foo: String = "" + +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty4.kt b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty4.kt new file mode 100644 index 00000000000..1266ed8c979 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty4.kt @@ -0,0 +1,9 @@ +open class A { + open fun setFoo(s: String) = Unit +} + +class B : A() { + private var foo: Int = 42 + + override fun setFoo(s: String) = super.setFoo(s) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty4.kt.after b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty4.kt.after new file mode 100644 index 00000000000..2c9e85afd7e --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty4.kt.after @@ -0,0 +1,8 @@ +open class A { + open fun setFoo(s: String) = Unit +} + +class B : A() { + private var foo: Int = 42 + +} \ 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 4b2de55a465..574710ca1b9 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -7220,6 +7220,26 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/redundantOverride/delegatedMemberHidesSuperTypeOverride4.kt"); } + @TestMetadata("getterWithDerivedProperty.kt") + public void testGetterWithDerivedProperty() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty.kt"); + } + + @TestMetadata("getterWithDerivedProperty2.kt") + public void testGetterWithDerivedProperty2() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty2.kt"); + } + + @TestMetadata("getterWithDerivedProperty3.kt") + public void testGetterWithDerivedProperty3() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty3.kt"); + } + + @TestMetadata("getterWithDerivedProperty4.kt") + public void testGetterWithDerivedProperty4() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/getterWithDerivedProperty4.kt"); + } + @TestMetadata("notCallSuper.kt") public void testNotCallSuper() throws Exception { runTest("idea/testData/inspectionsLocal/redundantOverride/notCallSuper.kt"); @@ -7235,6 +7255,26 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/redundantOverride/overrideModifireVisibility.kt"); } + @TestMetadata("setterWithDerivedProperty.kt") + public void testSetterWithDerivedProperty() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty.kt"); + } + + @TestMetadata("setterWithDerivedProperty2.kt") + public void testSetterWithDerivedProperty2() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty2.kt"); + } + + @TestMetadata("setterWithDerivedProperty3.kt") + public void testSetterWithDerivedProperty3() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty3.kt"); + } + + @TestMetadata("setterWithDerivedProperty4.kt") + public void testSetterWithDerivedProperty4() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantOverride/setterWithDerivedProperty4.kt"); + } + @TestMetadata("singleExpressionFunction.kt") public void testSingleExpressionFunction() throws Exception { runTest("idea/testData/inspectionsLocal/redundantOverride/singleExpressionFunction.kt");