Redundant override: do not report when class has derived property

#KT-32479 Fixed
2019-07-23 12:31:51 +09:00
parent 729ed1a44e
commit 64d4cca589
14 changed files with 180 additions and 6 deletions
@@ -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
@@ -0,0 +1,10 @@
+// PROBLEM: none
+open class A {
+    open fun getFoo(): String? = null
+}
+
+class B : A() {
+    private val foo = ""
+
+    <caret>override fun getFoo(): String? = super.getFoo()
+}
@@ -0,0 +1,10 @@
+// PROBLEM: none
+open class A {
+    open fun isFoo(): Boolean = true
+}
+
+class B : A() {
+    private val isFoo: Boolean = false
+
+    <caret>override fun isFoo(): Boolean = super.isFoo()
+}
@@ -0,0 +1,9 @@
+open class A {
+    open fun getFoo(i: Int): String? = null
+}
+
+class B : A() {
+    private val foo = ""
+
+    <caret>override fun getFoo(i: Int): String? = super.getFoo(i)
+}
@@ -0,0 +1,8 @@
+open class A {
+    open fun getFoo(i: Int): String? = null
+}
+
+class B : A() {
+    private val foo = ""
+
+}
@@ -0,0 +1,9 @@
+open class A {
+    open fun isFoo(): Boolean = true
+}
+
+class B : A() {
+    private val isFoo: Int = 42
+
+    <caret>override fun isFoo(): Boolean = super.isFoo()
+}
@@ -0,0 +1,8 @@
+open class A {
+    open fun isFoo(): Boolean = true
+}
+
+class B : A() {
+    private val isFoo: Int = 42
+
+}
@@ -0,0 +1,10 @@
+// PROBLEM: none
+open class A {
+    open fun setFoo(s: String) = Unit
+}
+
+class B : A() {
+    private var foo: String = ""
+
+    <caret>override fun setFoo(s: String) = super.setFoo(s)
+}
@@ -0,0 +1,10 @@
+// PROBLEM: none
+open class A {
+    open fun setFoo(s: String) = Unit
+}
+
+class B : A() {
+    private var isFoo: String = ""
+
+    <caret>override fun setFoo(s: String) = super.setFoo(s)
+}
@@ -0,0 +1,9 @@
+open class A {
+    open fun setFoo(s: String, i: Int) = Unit
+}
+
+class B : A() {
+    private var foo: String = ""
+
+    <caret>override fun setFoo(s: String, i: Int) = super.setFoo(s, i)
+}
@@ -0,0 +1,8 @@
+open class A {
+    open fun setFoo(s: String, i: Int) = Unit
+}
+ 
+class B : A() {
+    private var foo: String = ""
+
+}
@@ -0,0 +1,9 @@
+open class A {
+    open fun setFoo(s: String) = Unit
+}
+
+class B : A() {
+    private var foo: Int = 42
+
+    <caret>override fun setFoo(s: String) = super.setFoo(s)
+}
@@ -0,0 +1,8 @@
+open class A {
+    open fun setFoo(s: String) = Unit
+}
+
+class B : A() {
+    private var foo: Int = 42
+
+}
@@ -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");