From 8fa9b9923f5489013221f3120fa471f349d76ed9 Mon Sep 17 00:00:00 2001
From: Ilya Kirillov <ilya.kirillov@jetbrains.com>
Date: Wed, 29 May 2019 13:51:08 +0300
Subject: [PATCH] Allow removing trivial property accessor body when the
 accessor can not be fully deleted in redundant getter/setter inspections

---
 .../diagnostics/PositioningStrategies.kt      |  2 +-
 .../kotlin/psi/KtPropertyAccessor.java        |  9 ++++-
 .../KotlinPropertySetterParametersFixer.kt    |  2 +-
 .../inspections/RedundantGetterInspection.kt  | 39 +++++++++++++------
 .../inspections/RedundantSetterInspection.kt  | 32 +++++----------
 .../withAnnotaionAndNonTrivialBlockBody.kt    |  8 ++++
 .../withAnnotaionAndTrivialBlockBody.kt       |  6 +++
 .../withAnnotaionAndTrivialBlockBody.kt.after |  4 ++
 .../withAnnotaionAndTrivialExpressionBody.kt  |  4 ++
 ...AnnotaionAndTrivialExpressionBody.kt.after |  4 ++
 .../withAnnotaionAndNonTrivialBlockBody.kt    |  8 ++++
 .../withAnnotaionAndTrivialBlockBody.kt       |  6 +++
 .../withAnnotaionAndTrivialBlockBody.kt.after |  4 ++
 .../LocalInspectionTestGenerated.java         | 25 ++++++++++++
 14 files changed, 116 insertions(+), 37 deletions(-)
 create mode 100644 idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndNonTrivialBlockBody.kt
 create mode 100644 idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt
 create mode 100644 idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt.after
 create mode 100644 idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt
 create mode 100644 idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt.after
 create mode 100644 idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndNonTrivialBlockBody.kt
 create mode 100644 idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt
 create mode 100644 idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt.after

diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt
index 74fbf21cf09..82ad23a2c47 100644
--- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt
+++ b/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)
diff --git a/compiler/psi/src/org/jetbrains/kotlin/psi/KtPropertyAccessor.java b/compiler/psi/src/org/jetbrains/kotlin/psi/KtPropertyAccessor.java
index c5d0e385479..1d1cf2470f7 100644
--- a/compiler/psi/src/org/jetbrains/kotlin/psi/KtPropertyAccessor.java
+++ b/compiler/psi/src/org/jetbrains/kotlin/psi/KtPropertyAccessor.java
@@ -157,8 +157,13 @@ public class KtPropertyAccessor extends KtDeclarationStub<KotlinPropertyAccessor
     }
 
     @Nullable
-    public ASTNode getRightParenthesis() {
-        return getNode().findChildByType(KtTokens.RPAR);
+    public PsiElement getRightParenthesis() {
+        return findChildByType(KtTokens.RPAR);
+    }
+
+    @Nullable
+    public PsiElement getLeftParenthesis() {
+        return findChildByType(KtTokens.LPAR);
     }
 
     @Nullable
diff --git a/idea/src/org/jetbrains/kotlin/idea/editor/fixers/KotlinPropertySetterParametersFixer.kt b/idea/src/org/jetbrains/kotlin/idea/editor/fixers/KotlinPropertySetterParametersFixer.kt
index 108c72c17cc..ef8f7b22b23 100644
--- a/idea/src/org/jetbrains/kotlin/idea/editor/fixers/KotlinPropertySetterParametersFixer.kt
+++ b/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) {
diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt
index cd3b95dafe2..a3a237fbe82 100644
--- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt
+++ b/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
-    if (annotationEntries.isNotEmpty()) return false
-    val expression = bodyExpression ?: return true
-    if (expression is KtNameReferenceExpression) {
-        return expression.isFieldText()
-    }
+    val expression = bodyExpression ?: return canBeCompletelyDeleted()
+    if (expression.isBackingFieldReferenceTo(property)) 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 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
         }
-
-        accessor.delete()
+        if (accessor.canBeCompletelyDeleted()) {
+            accessor.delete()
+        } else {
+            accessor.deleteBody()
+        }
     }
 }
\ No newline at end of file
diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt
index 7d583acd455..316b7117c86 100644
--- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt
+++ b/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
-    if (hasLowerVisibilityThanProperty()) return false
-    val expression = bodyExpression ?: return true
+    val expression = bodyExpression ?: return canBeCompletelyDeleted()
     if (expression is KtBlockExpression) {
-        val statement = expression.statements.takeIf { it.size == 1 }?.firstOrNull() ?: return false
-        val parameter = valueParameters.takeIf { it.size == 1 }?.firstOrNull() ?: return false
+        val statement = expression.statements.singleOrNull() ?: 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()
+        }
     }
 }
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndNonTrivialBlockBody.kt b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndNonTrivialBlockBody.kt
new file mode 100644
index 00000000000..384bb203053
--- /dev/null
+++ b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndNonTrivialBlockBody.kt
@@ -0,0 +1,8 @@
+// PROBLEM: none
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get() {
+            1 + 2
+            return field
+        }
+}
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt
new file mode 100644
index 00000000000..6dab944607c
--- /dev/null
+++ b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt
@@ -0,0 +1,6 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get() {
+            return field
+        }
+}
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt.after b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt.after
new file mode 100644
index 00000000000..7bae5bda20a
--- /dev/null
+++ b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialBlockBody.kt.after
@@ -0,0 +1,4 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get
+}
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt
new file mode 100644
index 00000000000..fb668e748e0
--- /dev/null
+++ b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt
@@ -0,0 +1,4 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") <caret>get() = field
+}
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt.after b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt.after
new file mode 100644
index 00000000000..2aa00616904
--- /dev/null
+++ b/idea/testData/inspectionsLocal/redundantGetter/withAnnotaionAndTrivialExpressionBody.kt.after
@@ -0,0 +1,4 @@
+class Foo {
+    val foo: String = ""
+        @Deprecated("") get
+}
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndNonTrivialBlockBody.kt b/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndNonTrivialBlockBody.kt
new file mode 100644
index 00000000000..667f3577396
--- /dev/null
+++ b/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
+        }
+}
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt b/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt
new file mode 100644
index 00000000000..974e806d7d4
--- /dev/null
+++ b/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt
@@ -0,0 +1,6 @@
+class Foo {
+    var foo: String = ""
+        @Deprecated("") <caret>set(x) {
+            field = x
+        }
+}
\ No newline at end of file
diff --git a/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt.after b/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt.after
new file mode 100644
index 00000000000..1c5f72406dd
--- /dev/null
+++ b/idea/testData/inspectionsLocal/redundantSetter/withAnnotaionAndTrivialBlockBody.kt.after
@@ -0,0 +1,4 @@
+class Foo {
+    var foo: String = ""
+        @Deprecated("") set
+}
\ 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 1245caacf46..594b4a90369 100644
--- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java
+++ b/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")