Change Signature: Do not add default values to parameters of overriding functions

#KT-6160 Fixed

idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeInfo.java

@@ -31,6 +31,7 @@ import org.jetbrains.jet.lang.types.JetType;
 import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns;
 import org.jetbrains.jet.lexer.JetTokens;
 import org.jetbrains.jet.plugin.JetLanguage;
+import org.jetbrains.jet.plugin.refactoring.changeSignature.usages.JetFunctionDefinitionUsage;
 
 import java.util.Collection;
 import java.util.HashMap;
@@ -262,7 +263,7 @@ public class JetChangeInfo implements ChangeInfo {
     }
 
     @NotNull
-    public Collection<PsiElement> getAffectedFunctions() {
+    public Collection<JetFunctionDefinitionUsage> getAffectedFunctions() {
         return oldDescriptor.getAffectedFunctions();
     }
 }

idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignature.kt

@@ -118,7 +118,7 @@ public class JetChangeSignature(val project: Project,
             return
         }
 
-        val affectedFunctions = dialog.getMethodDescriptor().getAffectedFunctions()
+        val affectedFunctions = dialog.getMethodDescriptor().getAffectedFunctions().map { it.getElement() }.filterNotNull()
 
         if (affectedFunctions.any { !checkModifiable(it) }) {
             return

idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureData.java

@@ -17,13 +17,14 @@
 package org.jetbrains.jet.plugin.refactoring.changeSignature;
 
 import com.google.common.collect.Sets;
-import com.intellij.openapi.util.Condition;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiMethod;
 import com.intellij.psi.search.searches.OverridingMethodsSearch;
 import com.intellij.refactoring.changeSignature.MethodDescriptor;
 import com.intellij.util.Function;
 import com.intellij.util.containers.ContainerUtil;
+import kotlin.Function1;
+import kotlin.KotlinPackage;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.asJava.KotlinLightMethod;
@@ -33,13 +34,10 @@ import org.jetbrains.jet.lang.descriptors.impl.AnonymousFunctionDescriptor;
 import org.jetbrains.jet.lang.psi.*;
 import org.jetbrains.jet.lang.types.JetType;
 import org.jetbrains.jet.plugin.codeInsight.DescriptorToDeclarationUtil;
+import org.jetbrains.jet.plugin.refactoring.changeSignature.usages.JetFunctionDefinitionUsage;
 import org.jetbrains.jet.plugin.util.IdeDescriptorRenderers;
-import org.jetbrains.jet.renderer.DescriptorRenderer;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collection;
-import java.util.List;
-import java.util.Set;
 
 public final class JetChangeSignatureData implements JetMethodDescriptor {
     @NotNull
@@ -50,7 +48,7 @@ public final class JetChangeSignatureData implements JetMethodDescriptor {
     private final List<JetParameterInfo> parameters;
     @NotNull
     private final Collection<FunctionDescriptor> descriptorsForSignatureChange;
-    private Collection<PsiElement> affectedFunctions = null;
+    private Collection<JetFunctionDefinitionUsage> affectedFunctions = null;
 
     public JetChangeSignatureData(
             @NotNull FunctionDescriptor baseDescriptor,
@@ -94,47 +92,53 @@ public final class JetChangeSignatureData implements JetMethodDescriptor {
 
     @Override
     @NotNull
-    public Collection<PsiElement> getAffectedFunctions() {
+    public Collection<JetFunctionDefinitionUsage> getAffectedFunctions() {
         if (affectedFunctions == null) {
-            affectedFunctions = Sets.newHashSet();
+            affectedFunctions = KotlinPackage.flatMapTo(
-            for (FunctionDescriptor descriptor : descriptorsForSignatureChange) {
+                    descriptorsForSignatureChange,
-                affectedFunctions.addAll(computeHierarchyFrom(descriptor));
+                    new HashSet<JetFunctionDefinitionUsage>(),
-            }
+                    new Function1<FunctionDescriptor, Iterable<? extends JetFunctionDefinitionUsage>>() {
+                        @NotNull
+                        private Collection<JetFunctionDefinitionUsage> computeHierarchyFrom(@NotNull FunctionDescriptor baseDescriptor) {
+                            PsiElement declaration = DescriptorToDeclarationUtil.INSTANCE$.getDeclaration(baseDeclaration.getProject(), baseDescriptor);
+                            assert declaration != null : "No declaration found for " + baseDescriptor;
+
+                            Set<JetFunctionDefinitionUsage> result = Sets.newHashSet();
+                            result.add(new JetFunctionDefinitionUsage(declaration, false));
+
+                            if (!(declaration instanceof JetNamedFunction)) return result;
+
+                            PsiMethod lightMethod = LightClassUtil.getLightClassMethod((JetNamedFunction) declaration);
+                            // there are valid situations when light method is null: local functions and literals
+                            if (lightMethod == null) return result;
+
+                            return KotlinPackage.filterNotNullTo(
+                                    KotlinPackage.map(
+                                            OverridingMethodsSearch.search(lightMethod).findAll(),
+                                            new Function1<PsiMethod, JetFunctionDefinitionUsage>() {
+                                                @Override
+                                                public JetFunctionDefinitionUsage invoke(PsiMethod method) {
+                                                    JetDeclaration declaration = (method instanceof KotlinLightMethod)
+                                                                                 ? ((KotlinLightMethod) method).getOrigin()
+                                                                                 : null;
+                                                    return declaration != null ? new JetFunctionDefinitionUsage(declaration, true) : null;
+                                                }
+                                            }
+                                    ),
+                                    result
+                            );
+                        }
+
+                        @Override
+                        public Iterable<? extends JetFunctionDefinitionUsage> invoke(FunctionDescriptor descriptor) {
+                            return computeHierarchyFrom(descriptor);
+                        }
+                    }
+            );
         }
         return affectedFunctions;
     }
 
-    @NotNull
-    private Collection<PsiElement> computeHierarchyFrom(@NotNull FunctionDescriptor baseDescriptor) {
-        PsiElement declaration = DescriptorToDeclarationUtil.INSTANCE$.getDeclaration(baseDeclaration.getProject(), baseDescriptor);
-        Set<PsiElement> result = Sets.newHashSet();
-        result.add(declaration);
-        if (!(declaration instanceof JetNamedFunction)) {
-            return result;
-        }
-        PsiMethod lightMethod = LightClassUtil.getLightClassMethod((JetNamedFunction) declaration);
-        // there are valid situations when light method is null: local functions and literals
-        if (lightMethod == null) {
-            return result;
-        }
-        Collection<PsiMethod> overridingMethods = OverridingMethodsSearch.search(lightMethod).findAll();
-        List<PsiMethod> jetLightMethods = ContainerUtil.filter(overridingMethods, new Condition<PsiMethod>() {
-            @Override
-            public boolean value(PsiMethod method) {
-                return method instanceof KotlinLightMethod;
-            }
-        });
-        List<JetDeclaration> jetFunctions = ContainerUtil.map(jetLightMethods, new Function<PsiMethod, JetDeclaration>() {
-            @Override
-            public JetDeclaration fun(PsiMethod method) {
-                return ((KotlinLightMethod) method).getOrigin();
-            }
-        });
-        result.addAll(jetFunctions);
-        return result;
-    }
-
-
     @Override
     public String getName() {
         if (baseDescriptor instanceof ConstructorDescriptor) {

idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureUsageProcessor.java

@@ -64,14 +64,22 @@ public class JetChangeSignatureUsageProcessor implements ChangeSignatureUsagePro
     }
 
     private static void findAllMethodUsages(JetChangeInfo changeInfo, Set<UsageInfo> result) {
-        for (PsiElement affectedFunction : changeInfo.getAffectedFunctions()) {
+        for (JetFunctionDefinitionUsage functionUsageInfo : changeInfo.getAffectedFunctions()) {
-            findOneMethodUsages(affectedFunction, changeInfo, result, false);
+            findOneMethodUsages(functionUsageInfo, changeInfo, result);
         }
     }
 
-    private static void findOneMethodUsages(@NotNull PsiElement functionPsi, JetChangeInfo changeInfo,
+    private static void findOneMethodUsages(
-                                            Set<UsageInfo> result, boolean isInherited) {
+            @NotNull JetFunctionDefinitionUsage functionUsageInfo,
-        result.add(new JetFunctionDefinitionUsage(functionPsi, isInherited));
+            JetChangeInfo changeInfo,
+            Set<UsageInfo> result
+    ) {
+        result.add(functionUsageInfo);
+
+        PsiElement functionPsi = functionUsageInfo.getElement();
+        if (functionPsi == null) return;
+
+        boolean isInherited = functionUsageInfo.isInherited();
 
         for (PsiReference reference : ReferencesSearch.search(functionPsi, functionPsi.getUseScope())) {
             PsiElement element = reference.getElement();

idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetMethodDescriptor.java

@@ -22,12 +22,13 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.descriptors.FunctionDescriptor;
 import org.jetbrains.jet.lang.descriptors.Visibility;
+import org.jetbrains.jet.plugin.refactoring.changeSignature.usages.JetFunctionDefinitionUsage;
 
 import java.util.Collection;
 
 public interface JetMethodDescriptor extends MethodDescriptor<JetParameterInfo, Visibility> {
     @NotNull
-    Collection<PsiElement> getAffectedFunctions();
+    Collection<JetFunctionDefinitionUsage> getAffectedFunctions();
 
     boolean isConstructor();
 

idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetParameterInfo.java

@@ -173,14 +173,16 @@ public class JetParameterInfo implements ParameterInfo {
         StringBuilder buffer = new StringBuilder();
         JetValVar valVar = getValOrVar();
 
-        if (valVar != JetValVar.None)
+        if (valVar != JetValVar.None) {
             buffer.append(valVar.toString()).append(' ');
+        }
 
         buffer.append(getInheritedName(isInherited, inheritedFunction, baseFunction));
         buffer.append(": ").append(getTypeText());
 
-        if (defaultValue != null)
+        if (defaultValue != null && !isInherited) {
             buffer.append(" = ").append(defaultValue.getText());
+        }
 
         return buffer.toString();
     }

idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionDefinitionUsage.java

@@ -37,6 +37,10 @@ public class JetFunctionDefinitionUsage extends JetUsageInfo<PsiElement> {
         this.isInherited = isInherited;
     }
 
+    public final boolean isInherited() {
+        return isInherited;
+    }
+
     @Override
     public boolean processUsage(JetChangeInfo changeInfo, PsiElement element) {
         JetParameterList parameterList;

idea/testData/refactoring/changeSignature/NoDefaultValuesInOverridesAfter.kt

@@ -0,0 +1,18 @@
+trait T {
+    fun foo(a: Int = 1,
+            b: String = "2")
+}
+
+open class A: T {
+    override fun foo(a: Int,
+                     b: String) {
+        throw UnsupportedOperationException()
+    }
+}
+
+class B: A() {
+    override fun foo(a: Int,
+                     b: String) {
+        throw UnsupportedOperationException()
+    }
+}

idea/testData/refactoring/changeSignature/NoDefaultValuesInOverridesBefore.kt

@@ -0,0 +1,18 @@
+trait T {
+    fun <caret>foo(b: String = "2",
+            a: Int = 1)
+}
+
+open class A: T {
+    override fun foo(b: String,
+                     a: Int) {
+        throw UnsupportedOperationException()
+    }
+}
+
+class B: A() {
+    override fun foo(b: String,
+                     a: Int) {
+        throw UnsupportedOperationException()
+    }
+}

idea/tests/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureTest.java

@@ -37,10 +37,7 @@ import org.jetbrains.jet.plugin.caches.resolve.ResolvePackage;
 import org.jetbrains.jet.plugin.refactoring.JetRefactoringBundle;
 
 import java.io.File;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
 
 import static org.jetbrains.jet.lang.psi.PsiPackage.JetPsiFactory;
 import static org.jetbrains.jet.plugin.refactoring.changeSignature.ChangeSignaturePackage.getChangeSignatureDialog;
@@ -275,6 +272,14 @@ public class JetChangeSignatureTest extends KotlinCodeInsightTestCase {
         doTestConflict(changeInfo);
     }
 
+    public void testNoDefaultValuesInOverrides() throws Exception {
+        JetChangeInfo changeInfo = getChangeInfo();
+        JetParameterInfo[] newParameters = changeInfo.getNewParameters();
+        changeInfo.setNewParameter(0, newParameters[1]);
+        changeInfo.setNewParameter(1, newParameters[0]);
+        doTest(changeInfo);
+    }
+
     public void testOverridesInEnumEntries() throws Exception {
         JetChangeInfo changeInfo = getChangeInfo();
         JetParameterInfo newParameter = new JetParameterInfo("s", KotlinBuiltIns.getInstance().getStringType());