Suggested change signature: do not ask to enter values for optional parameters

#KT-37797 Fixed

idea/src/org/jetbrains/kotlin/idea/refactoring/suggested/KotlinSuggestedRefactoringAvailability.kt.201

@@ -14,6 +14,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny
 import org.jetbrains.kotlin.idea.refactoring.isInterfaceClass
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.KtCallableDeclaration
+import org.jetbrains.kotlin.psi.KtParameter
 import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
 import org.jetbrains.kotlin.psi.psiUtil.hasBody
 import org.jetbrains.kotlin.renderer.DescriptorRenderer
@@ -135,19 +136,67 @@ class KotlinSuggestedRefactoringAvailability(refactoringSupport: SuggestedRefact
         val oldSignature = state.oldSignature
         val newSignature = state.newSignature
         val updateUsagesData = SuggestedChangeSignatureData.create(state, USAGES)
+        val updateOverridesData = overridesName?.let { updateUsagesData.copy(nameOfStuffToUpdate = it) }
 
+        if (newSignature.parameters.size > oldSignature.parameters.size) {
+            val newParametersAtEndWithDefaults = newSignature.parameters.drop(oldSignature.parameters.size)
+                .all { oldSignature.parameterById(it.id) == null && it.defaultValue != null }
+            // special case if added new parameters with default values to the end
+            // we don't need to update usages if it's the only change
+            if (newParametersAtEndWithDefaults) {
+                val truncatedNewSignature = Signature.create(
+                    newSignature.name,
+                    newSignature.type,
+                    newSignature.parameters.take(oldSignature.parameters.size),
+                    newSignature.additionalData
+                )!!
+                val refactoringData = detectAvailableRefactoring(
+                    oldSignature,
+                    truncatedNewSignature,
+                    updateUsagesData,
+                    updateOverridesData,
+                    declaration,
+                    declaration.valueParameters.take(oldSignature.parameters.size)
+                )
+
+                return when (refactoringData) {
+                    is SuggestedChangeSignatureData -> refactoringData
+                    is SuggestedRenameData -> updateUsagesData
+                    null -> updateOverridesData
+                }
+            }
+        }
+
+        return detectAvailableRefactoring(
+            oldSignature,
+            newSignature,
+            updateUsagesData,
+            updateOverridesData,
+            declaration,
+            declaration.valueParameters
+        )
+    }
+
+    private fun detectAvailableRefactoring(
+        oldSignature: Signature,
+        newSignature: Signature,
+        updateUsagesData: SuggestedChangeSignatureData,
+        updateOverridesData: SuggestedChangeSignatureData?,
+        declaration: PsiNamedElement,
+        parameters: List<KtParameter>
+    ): SuggestedRefactoringData? {
         if (hasParameterAddedRemovedOrReordered(oldSignature, newSignature)) return updateUsagesData
 
         // for non-virtual function we can add or remove receiver for usages but not change its type
         if ((oldSignature.receiverType == null) != (newSignature.receiverType == null)) return updateUsagesData
 
-        val (nameChanges, renameData) = nameChanges(oldSignature, newSignature, declaration, declaration.valueParameters)
+        val (nameChanges, renameData) = nameChanges(oldSignature, newSignature, declaration, parameters)
 
         if (hasTypeChanges(oldSignature, newSignature)) {
             return if (nameChanges > 0)
                 updateUsagesData
             else
-                overridesName?.let { updateUsagesData.copy(nameOfStuffToUpdate = it) }
+                updateOverridesData
         }
 
         return when {

idea/src/org/jetbrains/kotlin/idea/refactoring/suggested/KotlinSuggestedRefactoringExecution.kt.201

@@ -105,7 +105,11 @@ class KotlinSuggestedRefactoringExecution(
                     defaultValueForParameter = defaultValue,
                     modifierList = modifierList
                 ).apply {
-                    setNewParameterValue(newParameterValues[newParameterValueIndex++])
+                    // a few last added parameters may be missing in newParameterValues
+                    // because they have default values and we did not offer to enter value for them
+                    if (newParameterValueIndex < newParameterValues.size) {
+                        setNewParameterValue(newParameterValues[newParameterValueIndex++])
+                    }
                 }
             } else {
                 KotlinParameterInfo(

idea/src/org/jetbrains/kotlin/idea/refactoring/suggested/KotlinSuggestedRefactoringUI.kt.201

@@ -6,6 +6,7 @@ import com.intellij.refactoring.suggested.SuggestedRefactoringExecution.NewParam
 import com.intellij.refactoring.suggested.SuggestedRefactoringSupport.Signature
 import com.intellij.refactoring.suggested.SuggestedRefactoringUI
 import com.intellij.refactoring.suggested.SignaturePresentationBuilder
+import com.intellij.refactoring.suggested.SuggestedRefactoringSupport.Parameter
 import org.jetbrains.kotlin.psi.KtExpressionCodeFragment
 import org.jetbrains.kotlin.psi.KtPsiFactory
 
@@ -19,7 +20,7 @@ object KotlinSuggestedRefactoringUI : SuggestedRefactoringUI() {
     }
 
     override fun extractNewParameterData(data: SuggestedChangeSignatureData): List<NewParameterData> {
-        val newParameters = mutableListOf<NewParameterData>()
+        val result = mutableListOf<NewParameterData>()
 
         val declaration = data.declaration
         val factory = KtPsiFactory(declaration.project)
@@ -27,16 +28,20 @@ object KotlinSuggestedRefactoringUI : SuggestedRefactoringUI() {
         fun createCodeFragment() = factory.createExpressionCodeFragment("", declaration)
 
         if (data.newSignature.receiverType != null && data.oldSignature.receiverType == null) {
-            newParameters.add(NewParameterData("<receiver>", createCodeFragment(), false/*TODO*/))
+            result.add(NewParameterData("<receiver>", createCodeFragment(), false/*TODO*/))
         }
 
-        for (parameter in data.newSignature.parameters) {
+        fun isNewParameter(parameter: Parameter) = data.oldSignature.parameterById(parameter.id) == null
-            if (data.oldSignature.parameterById(parameter.id) == null) {
+
-                newParameters.add(NewParameterData(parameter.name, createCodeFragment(), false/*TODO*/))
+        val newParameters = data.newSignature.parameters
+        val dropLastN = newParameters.reversed().count { isNewParameter(it) && it.defaultValue != null }
+        for (parameter in newParameters.dropLast(dropLastN)) {
+            if (isNewParameter(parameter)) {
+                result.add(NewParameterData(parameter.name, createCodeFragment(), false/*TODO*/))
             }
         }
 
-        return newParameters
+        return result
     }
 
     override fun extractValue(fragment: PsiCodeFragment): NewParameterValue.Expression? {

idea/tests/org/jetbrains/kotlin/idea/refactoring/suggested/KotlinSuggestedRefactoringAvailabilityTest.kt.201

@@ -439,6 +439,97 @@ class KotlinSuggestedRefactoringAvailabilityTest : BaseSuggestedRefactoringAvail
         )
     }
 
+    fun testAddOptionalParameter() {
+        doTest(
+            """
+                fun foo(p1: Int<caret>) {
+                }
+                
+                fun bar() {
+                    foo(1)
+                }
+            """.trimIndent(),
+            {
+                myFixture.type(", p2: Int = 2")
+            },
+            expectedAvailability = Availability.Disabled,
+        )
+    }
+
+    fun testAddOptionalParameterWithOverrides() {
+        doTest(
+            """
+                interface I {
+                    fun foo(p1: Int<caret>)
+                }    
+                
+                class C : I {
+                    override fun foo(p1: Int) {
+                    }
+                }
+            """.trimIndent(),
+            {
+                myFixture.type(", p2: Int = 2")
+            },
+            expectedAvailability = Availability.Available(changeSignatureAvailableTooltip("foo", "implementations")),
+        )
+    }
+
+    fun testAddOptionalParameterNotLast() {
+        doTest(
+            """
+                fun foo(<caret>p1: Int) {
+                }
+                
+                fun bar() {
+                    foo(1)
+                }
+            """.trimIndent(),
+            {
+                myFixture.type("p0: Int = 0, ")
+            },
+            expectedAvailability = Availability.Available(changeSignatureAvailableTooltip("foo", "usages")),
+        )
+    }
+
+    fun testAddOptionalParameterAndRenameParameter() {
+        doTest(
+            """
+                fun foo(<caret>p1: Int) {
+                }
+                
+                fun bar() {
+                    foo(1)
+                }
+            """.trimIndent(),
+            {
+                replaceTextAtCaret("p1", "p1New")
+            },
+            {
+                editor.caretModel.moveToOffset(editor.caretModel.offset + "p1New: Int".length)
+                myFixture.type(", p2: Int = 2")
+            },
+            expectedAvailability = Availability.Available(changeSignatureAvailableTooltip("foo", "usages")),
+        )
+    }
+
+    fun testAddTwoParameters() {
+        doTest(
+            """
+                fun foo(p1: Int<caret>) {
+                }
+                
+                fun bar() {
+                    foo(1)
+                }
+            """.trimIndent(),
+            {
+                myFixture.type(", p2: Int, p3: Int = 3")
+            },
+            expectedAvailability = Availability.Available(changeSignatureAvailableTooltip("foo", "usages")),
+        )
+    }
+
     private fun addImport(fqName: String) {
         (file as KtFile).importList!!.add(KtPsiFactory(project).createImportDirective(ImportPath.fromString(fqName)))
     }

idea/tests/org/jetbrains/kotlin/idea/refactoring/suggested/KotlinSuggestedRefactoringTest.kt.201

@@ -1345,7 +1345,7 @@ class KotlinSuggestedRefactoringTest : BaseSuggestedRefactoringTest() {
         )
     }
 
-    fun testAddParameterWithDefaultValue() {
+    fun testAddOptionalParameter() {
         doTestChangeSignature(
             """
                 interface I {
@@ -1375,7 +1375,7 @@ class KotlinSuggestedRefactoringTest : BaseSuggestedRefactoringTest() {
                     i.foo(1)
                 }
             """.trimIndent(),
-            "usages",
+            "implementations",
             {
                 myFixture.type(", p2: Int = 10")
             },
@@ -1414,7 +1414,7 @@ class KotlinSuggestedRefactoringTest : BaseSuggestedRefactoringTest() {
         )
     }
 
-    fun testReorderParameterWithDefaultValue() {
+    fun testReorderOptionalParameter() {
         doTestChangeSignature(
             """
                 interface I {