Allow removing trivial property accessor body when the accessor can not be fully deleted in redundant getter/setter inspections

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt

@@ -238,7 +238,7 @@ object PositioningStrategies {
                 is KtPropertyAccessor -> {
                     val endOfSignatureElement =
                         element.returnTypeReference
-                                ?: element.rightParenthesis?.psi
+                                ?: element.rightParenthesis
                                 ?: element.namePlaceholder
 
                     return markRange(element, endOfSignatureElement)

compiler/psi/src/org/jetbrains/kotlin/psi/KtPropertyAccessor.java

@@ -157,8 +157,13 @@ public class KtPropertyAccessor extends KtDeclarationStub<KotlinPropertyAccessor
     }
 
     @Nullable
-    public ASTNode getRightParenthesis() {
+    public PsiElement getRightParenthesis() {
-        return getNode().findChildByType(KtTokens.RPAR);
+        return findChildByType(KtTokens.RPAR);
+    }
+
+    @Nullable
+    public PsiElement getLeftParenthesis() {
+        return findChildByType(KtTokens.LPAR);
     }
 
     @Nullable

idea/src/org/jetbrains/kotlin/idea/editor/fixers/KotlinPropertySetterParametersFixer.kt

@@ -41,7 +41,7 @@ class KotlinPropertySetterParametersFixer : SmartEnterProcessorWithFixers.Fixer<
 
         val doc = editor.document
 
-        val parameterOffset = (psiElement.leftParenthesis?.startOffset ?: return) + 1
+        val parameterOffset = (psiElement.leftParenthesis?.node?.startOffset ?: return) + 1
 
         if (parameter?.text.isNullOrBlank()) {
             if (psiElement.rightParenthesis == null) {

idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt

@@ -10,7 +10,9 @@ import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiElementVisitor
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.isAncestor
 import org.jetbrains.kotlin.psi.psiUtil.startOffset
+import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierTypeOrDefault
 
 class RedundantGetterInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
     override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
@@ -31,21 +33,33 @@ class RedundantGetterInspection : AbstractKotlinInspection(), CleanupLocalInspec
 
 private fun KtPropertyAccessor.isRedundantGetter(): Boolean {
     if (!isGetter) return false
-    if (hasModifier(KtTokens.EXTERNAL_KEYWORD)) return false
+    val expression = bodyExpression ?: return canBeCompletelyDeleted()
-    if (annotationEntries.isNotEmpty()) return false
+    if (expression.isBackingFieldReferenceTo(property)) return true
-    val expression = bodyExpression ?: return true
-    if (expression is KtNameReferenceExpression) {
-        return expression.isFieldText()
-    }
     if (expression is KtBlockExpression) {
-        val statement = expression.statements.takeIf { it.size == 1 }?.firstOrNull() ?: return false
+        val statement = expression.statements.singleOrNull() ?: return false
         val returnExpression = statement as? KtReturnExpression ?: return false
-        return returnExpression.returnedExpression?.isFieldText() == true
+        return returnExpression.returnedExpression?.isBackingFieldReferenceTo(property) == true
     }
     return false
 }
 
-private fun KtExpression.isFieldText(): Boolean = this.textMatches("field")
+fun KtExpression.isBackingFieldReferenceTo(property: KtProperty) =
+    this is KtNameReferenceExpression
+            && text == KtTokens.FIELD_KEYWORD.value
+            && property.isAncestor(this)
+
+
+fun KtPropertyAccessor.canBeCompletelyDeleted(): Boolean {
+    if (modifierList == null) return true
+    if (annotationEntries.isNotEmpty()) return false
+    if (hasModifier(KtTokens.EXTERNAL_KEYWORD)) return false
+    return visibilityModifierTypeOrDefault() == property.visibilityModifierTypeOrDefault()
+}
+
+fun KtPropertyAccessor.deleteBody() {
+    val leftParenthesis = leftParenthesis ?: return
+    deleteChildRange(leftParenthesis, lastChild)
+}
 
 private class RemoveRedundantGetterFix : LocalQuickFix {
     override fun getName() = "Remove redundant getter"
@@ -60,7 +74,10 @@ private class RemoveRedundantGetterFix : LocalQuickFix {
         if (accessorTypeReference != null && property.typeReference == null && property.initializer == null) {
             property.typeReference = accessorTypeReference
         }
-
+        if (accessor.canBeCompletelyDeleted()) {
-        accessor.delete()
+            accessor.delete()
+        } else {
+            accessor.deleteBody()
+        }
     }
 }

idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt

@@ -32,35 +32,18 @@ class RedundantSetterInspection : AbstractKotlinInspection(), CleanupLocalInspec
 
 private fun KtPropertyAccessor.isRedundantSetter(): Boolean {
     if (!isSetter) return false
-    if (annotationEntries.isNotEmpty()) return false
+    val expression = bodyExpression ?: return canBeCompletelyDeleted()
-    if (hasLowerVisibilityThanProperty()) return false
-    val expression = bodyExpression ?: return true
     if (expression is KtBlockExpression) {
-        val statement = expression.statements.takeIf { it.size == 1 }?.firstOrNull() ?: return false
+        val statement = expression.statements.singleOrNull() ?: return false
-        val parameter = valueParameters.takeIf { it.size == 1 }?.firstOrNull() ?: return false
+        val parameter = valueParameters.singleOrNull() ?: return false
         val binaryExpression = statement as? KtBinaryExpression ?: return false
         return binaryExpression.operationToken == KtTokens.EQ
-                && binaryExpression.left?.isFieldText() == true
+                && binaryExpression.left?.isBackingFieldReferenceTo(property) == true
                 && binaryExpression.right?.mainReference?.resolve() == parameter
     }
     return false
 }
 
-private fun KtPropertyAccessor.hasLowerVisibilityThanProperty(): Boolean {
-    val p = property
-    return when {
-        p.hasModifier(KtTokens.PRIVATE_KEYWORD) ->
-            false
-        p.hasModifier(KtTokens.PROTECTED_KEYWORD) ->
-            hasModifier(KtTokens.PRIVATE_KEYWORD)
-        p.hasModifier(KtTokens.INTERNAL_KEYWORD) ->
-            hasModifier(KtTokens.PRIVATE_KEYWORD) || hasModifier(KtTokens.PROTECTED_KEYWORD)
-        else ->
-            hasModifier(KtTokens.PRIVATE_KEYWORD) || hasModifier(KtTokens.PROTECTED_KEYWORD) || hasModifier(KtTokens.INTERNAL_KEYWORD)
-    }
-}
-
-private fun KtExpression.isFieldText(): Boolean = this.textMatches("field")
 
 private class RemoveRedundantSetterFix : LocalQuickFix {
     override fun getName() = "Remove redundant setter"
@@ -69,6 +52,11 @@ private class RemoveRedundantSetterFix : LocalQuickFix {
 
     override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
         val accessor = descriptor.psiElement as? KtPropertyAccessor ?: return
-        accessor.delete()
+
+        if (accessor.canBeCompletelyDeleted()) {
+            accessor.delete()
+        } else {
+            accessor.deleteBody()
+        }
     }
 }

idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndNonTrivialBlockBody.kt

@@ -0,0 +1,8 @@
+// PROBLEM: none
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get() {
+            1 + 2
+            return field
+        }
+}

idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt

@@ -0,0 +1,6 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get() {
+            return field
+        }
+}

idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt.after

@@ -0,0 +1,4 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get
+}

idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt

@@ -0,0 +1,4 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get() = field
+}

idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt.after

@@ -0,0 +1,4 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") get
+}

idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndNonTrivialBlockBody.kt

@@ -0,0 +1,8 @@
+// PROBLEM: none
+class Foo {
+    var foo: String = ""
+        @Deprecated("") <caret>set(foo) {
+            1 + 2
+            field = foo
+        }
+}

idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt

@@ -0,0 +1,6 @@
+class Foo {
+    var foo: String = ""
+        @Deprecated("") <caret>set(x) {
+            field = x
+        }
+}

idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt.after

@@ -0,0 +1,4 @@
+class Foo {
+    var foo: String = ""
+        @Deprecated("") set
+}

idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java

@@ -5741,6 +5741,21 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
         public void testOnlyReturnFieldBody() throws Exception {
             runTest("idea/testData/inspectionsLocal/redundantGetter/onlyReturnFieldBody.kt");
         }
+
+        @TestMetadata("withAnnotaionAndNonTrivialBlockBody.kt")
+        public void testWithAnnotaionAndNonTrivialBlockBody() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndNonTrivialBlockBody.kt");
+        }
+
+        @TestMetadata("withAnnotaionAndTrivialBlockBody.kt")
+        public void testWithAnnotaionAndTrivialBlockBody() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt");
+        }
+
+        @TestMetadata("withAnnotaionAndTrivialExpressionBody.kt")
+        public void testWithAnnotaionAndTrivialExpressionBody() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt");
+        }
     }
 
     @TestMetadata("idea/testData/inspectionsLocal/redundantLambdaArrow")
@@ -6370,6 +6385,16 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
         public void testSameVisibility3() throws Exception {
             runTest("idea/testData/inspectionsLocal/redundantSetter/sameVisibility3.kt");
         }
+
+        @TestMetadata("withAnnotaionAndNonTrivialBlockBody.kt")
+        public void testWithAnnotaionAndNonTrivialBlockBody() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndNonTrivialBlockBody.kt");
+        }
+
+        @TestMetadata("withAnnotaionAndTrivialBlockBody.kt")
+        public void testWithAnnotaionAndTrivialBlockBody() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt");
+        }
     }
 
     @TestMetadata("idea/testData/inspectionsLocal/redundantSuspend")