[java-refactoring] Better vararg handling in change signature

1. Properly handle non-vararg call sites
2. Update callsites when vararg type was changed to non-vararg or vice versa (without reordering, etc.)
Fixes IDEA-318626 Change signature incorrectly wraps/unwraps arguments when changing between varargs and arrays

GitOrigin-RevId: 3372a144be4363fef2c40e8968a07d8ed6b916a6

java/java-impl-refactorings/src/com/intellij/refactoring/changeSignature/JavaChangeSignatureUsageProcessor.java

@@ -177,13 +177,15 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
         final MethodCallUsageInfo methodCallInfo = ((MethodReferenceUsageInfo)usage).createMethodCallInfo();
         if (methodCallInfo != null) {
           processMethodUsage(methodCallInfo.getElement(), javaChangeInfo, methodCallInfo.isToChangeArguments(),
-                             methodCallInfo.isToCatchExceptions(), methodCallInfo.getReferencedMethod(), methodCallInfo.getSubstitutor(), usages);
+                             methodCallInfo.isToCatchExceptions(), methodCallInfo.isVarArgCall(), 
+                             methodCallInfo.getReferencedMethod(), methodCallInfo.getSubstitutor(), usages);
           return true;
         }
       }
       else if (usage instanceof final MethodCallUsageInfo methodCallInfo) {
         processMethodUsage(methodCallInfo.getElement(), javaChangeInfo, methodCallInfo.isToChangeArguments(),
-                           methodCallInfo.isToCatchExceptions(), methodCallInfo.getReferencedMethod(), methodCallInfo.getSubstitutor(), usages);
+                           methodCallInfo.isToCatchExceptions(), methodCallInfo.isVarArgCall(), 
+                           methodCallInfo.getReferencedMethod(), methodCallInfo.getSubstitutor(), usages);
         return true;
       }
       else if (usage instanceof ChangeSignatureParameterUsageInfo) {
@@ -203,7 +205,7 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
         return true;
       }
       else if (element instanceof PsiEnumConstant enumConstant) {
-        fixActualArgumentsList(enumConstant.getArgumentList(), javaChangeInfo, true, PsiSubstitutor.EMPTY);
+        fixActualArgumentsList(enumConstant.getArgumentList(), javaChangeInfo, true, PsiSubstitutor.EMPTY, false);
         return true;
       }
       else if (!(usage instanceof OverriderUsageInfo) && !(usage instanceof UnresolvableCollisionUsageInfo)) {
@@ -258,7 +260,7 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
         final PsiExpressionList argumentList = ((PsiNewExpression)parent).getArgumentList();
         final PsiClass baseClass = changeInfo.getMethod().getContainingClass();
         final PsiSubstitutor substitutor = TypeConversionUtil.getSuperClassSubstitutor(baseClass, aClass, PsiSubstitutor.EMPTY);
-        fixActualArgumentsList(argumentList, changeInfo, true, substitutor);
+        fixActualArgumentsList(argumentList, changeInfo, true, substitutor, false);
       }
     }
   }
@@ -280,14 +282,18 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
     final PsiClass aClass = constructor.getContainingClass();
     final PsiClass baseClass = changeInfo.getMethod().getContainingClass();
     final PsiSubstitutor substitutor = TypeConversionUtil.getSuperClassSubstitutor(baseClass, aClass, PsiSubstitutor.EMPTY);
-    processMethodUsage(callExpression.getMethodExpression(), changeInfo, true, false, callee, substitutor, usages);
+    processMethodUsage(callExpression.getMethodExpression(), changeInfo, 
+                       true, false, changeInfo.wasVararg(), callee, substitutor, usages);
   }
 
   private static void processMethodUsage(PsiElement ref,
-                                  JavaChangeInfo changeInfo,
-                                  boolean toChangeArguments,
-                                  boolean toCatchExceptions,
-                                  PsiMethod callee, PsiSubstitutor substitutor, final UsageInfo[] usages) throws IncorrectOperationException {
+                                         JavaChangeInfo changeInfo,
+                                         boolean toChangeArguments,
+                                         boolean toCatchExceptions,
+                                         boolean varArgCall, 
+                                         PsiMethod callee, 
+                                         PsiSubstitutor substitutor,
+                                         final UsageInfo[] usages) throws IncorrectOperationException {
     if (changeInfo.isNameChanged()) {
       if (ref instanceof PsiJavaCodeReferenceElement) {
         PsiElement last = ((PsiJavaCodeReferenceElement)ref).getReferenceNameElement();
@@ -309,7 +315,7 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
         }
       }
 
-      fixActualArgumentsList(list, changeInfo, toInsertDefaultValue, substitutor);
+      fixActualArgumentsList(list, changeInfo, toInsertDefaultValue, substitutor, varArgCall);
     }
 
     if (toCatchExceptions) {
@@ -454,7 +460,8 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
   private static void fixActualArgumentsList(PsiExpressionList list,
                                              JavaChangeInfo changeInfo,
                                              boolean toInsertDefaultValue,
-                                             PsiSubstitutor substitutor) throws IncorrectOperationException {
+                                             PsiSubstitutor substitutor,
+                                             boolean varArgCall) throws IncorrectOperationException {
     final PsiElementFactory factory = JavaPsiFacade.getElementFactory(list.getProject());
     if (changeInfo.isParameterSetOrOrderChanged()) {
       if (changeInfo instanceof JavaChangeInfoImpl javaChangeInfo && javaChangeInfo.isPropagationEnabled) {
@@ -472,7 +479,7 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
       }
       else {
         final PsiExpression[] args = list.getExpressions();
-        final int nonVarargCount = getNonVarargCount(changeInfo, args);
+        final int nonVarargCount = varArgCall ? getNonVarargCount(changeInfo, args) : args.length;
         final int varargCount = args.length - nonVarargCount;
         if (varargCount<0) return;
         PsiExpression[] newVarargInitializers = null;
@@ -492,7 +499,7 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
           }
           newArgsLength = newVarargInitializers == null ? newParameters.length : newNonVarargCount + newVarargInitializers.length;
         }
-        else if (changeInfo.isRetainsVarargs()) {
+        else if (changeInfo.isRetainsVarargs() && varArgCall) {
           newNonVarargCount = newParameters.length - 1;
           newArgsLength = newNonVarargCount + varargCount;
         }
@@ -506,7 +513,7 @@ public class JavaChangeSignatureUsageProcessor implements ChangeSignatureUsagePr
         }
 
         String[] oldVarargs = null;
-        if (changeInfo.wasVararg() && !changeInfo.isRetainsVarargs()) {
+        if (varArgCall && changeInfo.wasVararg() && !changeInfo.isRetainsVarargs()) {
           oldVarargs = new String[varargCount];
           for (int i = nonVarargCount; i < args.length; i++) {
             oldVarargs[i - nonVarargCount] = args[i].getText();

java/java-impl-refactorings/src/com/intellij/refactoring/changeSignature/JavaChangeSignatureUsageSearcher.java

@@ -330,7 +330,7 @@ class JavaChangeSignatureUsageSearcher {
           result.add(new CallReferenceUsageInfo((PsiCallReference)ref));
         }
         else if (element instanceof PsiMethodReferenceExpression && MethodReferenceUsageInfo.needToExpand(myChangeInfo)) {
-          result.add(new MethodReferenceUsageInfo(element, method, isToModifyArgs, isToCatchExceptions));
+          result.add(new MethodReferenceUsageInfo(element, isToModifyArgs, isToCatchExceptions));
         }
         else {
           result.add(new MoveRenameUsageInfo(element, ref, method));

java/java-impl-refactorings/src/com/intellij/refactoring/changeSignature/MethodCallUsageInfo.java

@@ -1,28 +1,15 @@
-/*
- * Copyright 2000-2009 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.refactoring.changeSignature;
 
-import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.psi.*;
 import com.intellij.usageView.UsageInfo;
+import com.siyeh.ig.psiutils.MethodCallUtils;
+import org.jetbrains.annotations.NotNull;
 
 public class MethodCallUsageInfo extends UsageInfo {
-  private static final Logger LOG = Logger.getInstance(MethodCallUsageInfo.class);
   private final boolean myToChangeArguments;
   private final boolean myToCatchExceptions;
+  private final boolean myVarArgCall;
   private final PsiMethod myReferencedMethod;
   private final PsiSubstitutor mySubstitutor;
 
@@ -34,29 +21,38 @@ public class MethodCallUsageInfo extends UsageInfo {
     return myToChangeArguments;
   }
 
-  public MethodCallUsageInfo(final PsiElement ref, boolean isToChangeArguments, boolean isToCatchExceptions) {
+  public boolean isVarArgCall() {
+    return myVarArgCall;
+  }
+
+  public MethodCallUsageInfo(final @NotNull PsiElement ref, boolean isToChangeArguments, boolean isToCatchExceptions) {
     super(ref);
     myToChangeArguments = isToChangeArguments;
     myToCatchExceptions = isToCatchExceptions;
-    final JavaResolveResult resolveResult = resolveMethod(ref);
-    myReferencedMethod = (PsiMethod)resolveResult.getElement();
+    PsiCall call = getCall(ref);
+    if (call == null) {
+      throw new IllegalArgumentException("Unknown reference: " + ref.getClass());
+    }
+    myVarArgCall = MethodCallUtils.isVarArgCall(call);
+    JavaResolveResult resolveResult = call.resolveMethodGenerics();
+    if (resolveResult == null || !(resolveResult.getElement() instanceof PsiMethod method)) {
+      throw new IllegalArgumentException("Cannot resolve call to a method " + call);
+    }
+    myReferencedMethod = method;
     mySubstitutor = resolveResult.getSubstitutor();
   }
 
-  private static JavaResolveResult resolveMethod(final PsiElement ref) {
-    if (ref instanceof PsiEnumConstant) return ((PsiEnumConstant)ref).resolveMethodGenerics();
-    if (ref instanceof PsiCallExpression) {
-      return ((PsiCallExpression)ref).resolveMethodGenerics();
+  private static PsiCall getCall(final PsiElement ref) {
+    if (ref instanceof PsiCall call) {
+      return call;
     }
     PsiElement parent = ref.getParent();
-    if (parent instanceof PsiCall) {
-      return ((PsiCall)parent).resolveMethodGenerics();
+    if (parent instanceof PsiCall call) {
+      return call;
     }
     else if (parent instanceof PsiAnonymousClass) {
-      return ((PsiNewExpression)parent.getParent()).resolveMethodGenerics();
+      return (PsiCall) parent.getParent();
     }
-    LOG.error("Unknown reference");
-
     return null;
   }
 

java/java-impl-refactorings/src/com/intellij/refactoring/changeSignature/MethodReferenceUsageInfo.java

@@ -1,18 +1,4 @@
-/*
- * Copyright 2000-2016 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.refactoring.changeSignature;
 
 import com.intellij.psi.PsiCallExpression;
@@ -28,7 +14,7 @@ class MethodReferenceUsageInfo extends UsageInfo {
   
   private PsiCallExpression myCallExpression;
 
-  MethodReferenceUsageInfo(PsiElement element, PsiMethod method, boolean isToModifyArgs, boolean isToCatchExceptions) {
+  MethodReferenceUsageInfo(PsiElement element, boolean isToModifyArgs, boolean isToCatchExceptions) {
     super(element);
     myIsToModifyArgs = isToModifyArgs;
     myIsToCatchExceptions = isToCatchExceptions;

java/java-impl/src/com/intellij/refactoring/changeSignature/JavaChangeInfoImpl.java

@@ -158,6 +158,11 @@ public class JavaChangeInfoImpl extends UserDataHolderBase implements JavaChange
       isParameterSetOrOrderChanged = true;
     }
     else {
+      boolean isVararg = newParms.length > 0 && newParms[newParms.length - 1].isVarargType();
+      if (wasVararg != isVararg) {
+        // Need to trigger possible argument changes
+        isParameterSetOrOrderChanged = true;
+      }
       for(int i = 0; i < newParms.length; i++){
         ParameterInfoImpl parmInfo = newParms[i];
 

java/java-tests/testData/refactoring/changeSignature/ArrayToVararg.java

@@ -0,0 +1,8 @@
+public class X
+{
+  void doSo<caret>mething(int x, String[] args) { /* ... */ }
+  
+  void use() {
+    doSomething(0, new String[]{"one", "two"});
+  }
+}

java/java-tests/testData/refactoring/changeSignature/ArrayToVararg_after.java

@@ -0,0 +1,8 @@
+public class X
+{
+  void doSomething(int x, String... args) { /* ... */ }
+  
+  void use() {
+    doSomething(0, "one", "two");
+  }
+}

java/java-tests/testData/refactoring/changeSignature/DefCtorVarargToArray.java

@@ -0,0 +1,8 @@
+class X {
+  <caret>X(String... args) {}
+  
+  X(int x) {
+    this();
+  }
+}
+class Y extends X {}

java/java-tests/testData/refactoring/changeSignature/DefCtorVarargToArray_after.java

@@ -0,0 +1,12 @@
+class X {
+  X(String[] args) {}
+  
+  X(int x) {
+    this(new String[]{});
+  }
+}
+class Y extends X {
+    Y() {
+        super(new String[]{});
+    }
+}

java/java-tests/testData/refactoring/changeSignature/EnumVarargToArray.java

@@ -0,0 +1,7 @@
+public enum X
+{
+  A(0, "foo", "bar"),
+  B(0, new String[]{"one", "two"});
+  
+  <caret>X(int x, String... args) { /* ... */ }
+}

java/java-tests/testData/refactoring/changeSignature/EnumVarargToArray_after.java

@@ -0,0 +1,7 @@
+public enum X
+{
+  A(0, new String[]{"foo", "bar"}),
+  B(0, new String[]{"one", "two"});
+  
+  X(int x, String[] args) { /* ... */ }
+}

java/java-tests/testData/refactoring/changeSignature/VarargToArray.java

@@ -0,0 +1,9 @@
+public class X
+{
+  void doSo<caret>mething(int x, String... args) { /* ... */ }
+  
+  void use() {
+    doSomething(0, "foo", "bar");
+    doSomething(0, new String[]{"one", "two"});
+  }
+}

java/java-tests/testData/refactoring/changeSignature/VarargToArrayReorder.java

@@ -0,0 +1,9 @@
+public class X
+{
+  void doSome<caret>thing(int x, String... args) { /* ... */ }
+  
+  void use() {
+    doSomething(0, "foo", "bar");
+    doSomething(0, new String[]{"one", "two"});
+  }
+}

java/java-tests/testData/refactoring/changeSignature/VarargToArrayReorder_after.java

@@ -0,0 +1,9 @@
+public class X
+{
+  void doSomething(String[] args, int x) { /* ... */ }
+  
+  void use() {
+    doSomething(new String[]{"foo", "bar"}, 0);
+    doSomething(new String[]{"one", "two"}, 0);
+  }
+}

java/java-tests/testData/refactoring/changeSignature/VarargToArray_after.java

@@ -0,0 +1,9 @@
+public class X
+{
+  void doSomething(int x, String[] args) { /* ... */ }
+  
+  void use() {
+    doSomething(0, new String[]{"foo", "bar"});
+    doSomething(0, new String[]{"one", "two"});
+  }
+}

java/java-tests/testSrc/com/intellij/java/refactoring/ChangeSignatureTest.java

@@ -1,4 +1,4 @@
-// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.java.refactoring;
 
 import com.intellij.codeInsight.TargetElementUtil;
@@ -672,6 +672,40 @@ public class ChangeSignatureTest extends ChangeSignatureBaseTest {
       ParameterInfoImpl.createNew().withType(PsiTypes.shortType()).withName("d"),
     }, false);
   }
+  
+  public void testVarargToArray() { // IDEA-318626
+    doTest(null, null, null, method -> new ParameterInfoImpl[] {
+      ParameterInfoImpl.create(0).withName("x"),
+      ParameterInfoImpl.create(1).withName("args").withType(method.getParameterList().getParameter(1).getType().getDeepComponentType().createArrayType())
+    }, false);
+  }
+
+  public void testEnumVarargToArray() {
+    doTest(null, null, null, method -> new ParameterInfoImpl[] {
+      ParameterInfoImpl.create(0).withName("x"),
+      ParameterInfoImpl.create(1).withName("args").withType(method.getParameterList().getParameter(1).getType().getDeepComponentType().createArrayType())
+    }, false);
+  }
+
+  public void testDefCtorVarargToArray() {
+    doTest(null, null, null, method -> new ParameterInfoImpl[] {
+      ParameterInfoImpl.create(0).withName("args").withType(method.getParameterList().getParameter(0).getType().getDeepComponentType().createArrayType())
+    }, false);
+  }
+
+  public void testArrayToVararg() { // IDEA-318626
+    doTest(null, null, null, method -> new ParameterInfoImpl[] {
+      ParameterInfoImpl.create(0).withName("x"),
+      ParameterInfoImpl.create(1).withName("args").withType(new PsiEllipsisType(method.getParameterList().getParameter(1).getType().getDeepComponentType()))
+    }, false);
+  }
+  
+  public void testVarargToArrayReorder() { // IDEA-318626
+    doTest(null, null, null, method -> new ParameterInfoImpl[] {
+      ParameterInfoImpl.create(1).withName("args").withType(method.getParameterList().getParameter(1).getType().getDeepComponentType().createArrayType()),
+      ParameterInfoImpl.create(0).withName("x")
+    }, false);
+  }
 
   /* workers */
 }