extract method: allow to choose return type from hierarchy (IDEA-79995)

2026-09-27 10:03:11 +07:00 · 2014-12-02 19:34:40 +01:00
parent 72515b5403
commit c6110f2cfa
13 changed files with 273 additions and 21 deletions
@@ -21,6 +21,7 @@
 package com.intellij.refactoring.extractMethod;

 import com.intellij.psi.PsiModifier;
+import com.intellij.psi.PsiType;
 import com.intellij.refactoring.util.VariableData;

 public interface AbstractExtractDialog {
@@ -31,6 +32,7 @@ public interface AbstractExtractDialog {
  String getVisibility();
  boolean isMakeStatic();
  boolean isChainedConstructor();
+  PsiType getReturnType();

  void show();
  boolean isOK();
@@ -32,10 +32,7 @@ import com.intellij.psi.util.PsiFormatUtil;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.refactoring.RefactoringBundle;
-import com.intellij.refactoring.ui.ComboBoxVisibilityPanel;
-import com.intellij.refactoring.ui.ConflictsDialog;
-import com.intellij.refactoring.ui.JavaComboBoxVisibilityPanel;
-import com.intellij.refactoring.ui.MethodSignatureComponent;
+import com.intellij.refactoring.ui.*;
 import com.intellij.refactoring.util.ConflictsUtil;
 import com.intellij.refactoring.util.ParameterTablePanel;
 import com.intellij.refactoring.util.VariableData;
@@ -46,9 +43,11 @@ import com.intellij.ui.SeparatorFactory;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.VisibilityUtil;
 import com.intellij.util.containers.MultiMap;
+import com.intellij.util.ui.DialogUtil;
 import com.intellij.util.ui.UIUtil;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;

 import javax.swing.*;
 import javax.swing.border.Border;
@@ -61,7 +60,6 @@ import java.awt.event.*;
 /**
 * @author Konstantin Bulenkov
 */
-@SuppressWarnings("MethodMayBeStatic")
 public class ExtractMethodDialog extends DialogWrapper implements AbstractExtractDialog {
  private static final String EXTRACT_METHOD_DEFAULT_VISIBILITY = "extract.method.default.visibility";
  public static final String EXTRACT_METHOD_GENERATE_ANNOTATIONS = "extractMethod.generateAnnotations";
@@ -93,6 +91,7 @@ public class ExtractMethodDialog extends DialogWrapper implements AbstractExtrac
  public JPanel myCenterPanel;
  public JPanel myParamTable;
  private VariableData[] myInputVariables;
+  private TypeSelector mySelector;

  public ExtractMethodDialog(Project project,
                             PsiClass targetClass, final InputVariables inputVariables, PsiType returnType,
@@ -120,14 +119,8 @@ public class ExtractMethodDialog extends DialogWrapper implements AbstractExtrac
    mySignature.setMinimumSize(new Dimension(500, 100));
    setTitle(title);

-    // Create UI components
-
    myNameField = createNameField(initialMethodName);
-
-    int height = myVariableData.getInputVariables().size() + 2;
-    if (myExceptions.length > 0) {
-      height += myExceptions.length + 1;
-    }
+    
    myMakeStatic = new NonFocusableCheckBox();
    myMakeStatic.setText(RefactoringBundle.message("declare.static.checkbox"));
    if (canBeChainedConstructor) {
@@ -226,8 +219,15 @@ public class ExtractMethodDialog extends DialogWrapper implements AbstractExtrac
    });

    myVisibilityPanel = createVisibilityPanel();
+    final JPanel visibilityAndReturnType = new JPanel(new BorderLayout(2, 0));
+    visibilityAndReturnType.add(myVisibilityPanel, BorderLayout.WEST);
+    final JPanel returnTypePanel = createReturnTypePanel();
+    if (returnTypePanel != null) {
+      visibilityAndReturnType.add(returnTypePanel, BorderLayout.EAST);
+    }
+
    final JPanel visibilityAndName = new JPanel(new BorderLayout(2, 0));
-    visibilityAndName.add(myVisibilityPanel, BorderLayout.WEST);
+    visibilityAndName.add(visibilityAndReturnType, BorderLayout.WEST);
    visibilityAndName.add(namePanel, BorderLayout.CENTER);
    main.add(visibilityAndName, BorderLayout.CENTER);
    setOKActionEnabled(false);
@@ -239,6 +239,31 @@ public class ExtractMethodDialog extends DialogWrapper implements AbstractExtrac
    return main;
  }

+  @Nullable
+  private JPanel createReturnTypePanel() {
+    mySelector = new TypeSelectorManagerImpl(myProject, myReturnType, findOccurrences(), areTypesDirected()).getTypeSelector();
+    final JComponent component = mySelector.getComponent();
+    if (component instanceof JComboBox) {
+      final JPanel returnTypePanel = new JPanel(new BorderLayout(2, 0));
+      final JLabel label = new JLabel(RefactoringBundle.message("changeSignature.return.type.prompt"));
+      returnTypePanel.add(label, BorderLayout.NORTH);
+      returnTypePanel.add(component, BorderLayout.SOUTH);
+      DialogUtil.registerMnemonic(label, component);
+      ((JComboBox)component).addActionListener(new ActionListener() {
+        @Override
+        public void actionPerformed(ActionEvent e) {
+          updateSignature();
+        }
+      });
+      return returnTypePanel;
+    }
+    return null;
+  }
+
+  protected PsiExpression[] findOccurrences() {
+    return PsiExpression.EMPTY_ARRAY;
+  }
+
  protected JPanel createOptionsPanel() {
    final JPanel optionsPanel = new JPanel(new FlowLayout(FlowLayout.LEFT, 0, 5));

@@ -490,7 +515,7 @@ public class ExtractMethodDialog extends DialogWrapper implements AbstractExtrac
      buffer.append(myTargetClass.getName());
    }
    else {
-      buffer.append(PsiFormatUtil.formatType(myReturnType, 0, PsiSubstitutor.EMPTY));
+      buffer.append(PsiFormatUtil.formatType(mySelector.getSelectedType(), 0, PsiSubstitutor.EMPTY));
      buffer.append(" ");
      buffer.append(myNameField.getText());
    }
@@ -558,4 +583,8 @@ public class ExtractMethodDialog extends DialogWrapper implements AbstractExtrac

    ConflictsUtil.checkMethodConflicts(myTargetClass, null, prototype, conflicts);
  }
+
+  public PsiType getReturnType() {
+    return mySelector.getSelectedType();
+  }
 }
@@ -16,6 +16,7 @@
 package com.intellij.refactoring.extractMethod;

 import com.intellij.codeInsight.ChangeContextUtil;
+import com.intellij.codeInsight.CodeInsightUtil;
 import com.intellij.codeInsight.ExceptionUtil;
 import com.intellij.codeInsight.NullableNotNullManager;
 import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil;
@@ -486,6 +487,11 @@ public class ExtractMethodProcessor implements MatchProvider {
    myStatic = isStatic() | dialog.isMakeStatic();
    myIsChainedConstructor = dialog.isChainedConstructor();
    myMethodVisibility = dialog.getVisibility();
+
+    final PsiType returnType = dialog.getReturnType();
+    if (returnType != null) {
+      myReturnType = returnType;
+    }
  }

  protected AbstractExtractDialog createExtractMethodDialog(final boolean direct) {
@@ -500,6 +506,11 @@ public class ExtractMethodProcessor implements MatchProvider {
        return direct;
      }

+      @Override
+      protected PsiExpression[] findOccurrences() {
+        return ExtractMethodProcessor.this.findOccurrences();
+      }
+
      @Override
      protected boolean isOutputVariable(PsiVariable var) {
        return ExtractMethodProcessor.this.isOutputVariable(var);
@@ -533,6 +544,19 @@ public class ExtractMethodProcessor implements MatchProvider {
    };
  }

+  public PsiExpression[] findOccurrences() {
+    if (myExpression != null) {
+      return new PsiExpression[] {myExpression};
+    }
+    if (myOutputVariable != null) {
+      final PsiElement scope = myOutputVariable instanceof PsiLocalVariable 
+                               ? RefactoringUtil.getVariableScope((PsiLocalVariable)myOutputVariable) 
+                               : PsiTreeUtil.findCommonParent(myElements);
+      return CodeInsightUtil.findReferenceExpressions(scope, myOutputVariable);
+    }
+    return PsiExpression.EMPTY_ARRAY;
+  }
+
  private Nullness initNullness() {
    if (!PsiUtil.isLanguageLevel5OrHigher(myElements[0]) || PsiUtil.resolveClassInType(myReturnType) == null) return null;
    final PsiMethod emptyMethod = generateEmptyMethod(getThrownExceptions(), isStatic(), "name");
@@ -614,6 +638,14 @@ public class ExtractMethodProcessor implements MatchProvider {
    }
  }

+  @TestOnly
+  public void testPrepare(PsiType returnType) {
+    testPrepare();
+    if (returnType != null) {
+      myReturnType = returnType;
+    }
+  }
+
  @TestOnly
  public void doNotPassParameter(int i) {
    myVariableDatum[i].passAsParameter = false;
@@ -964,7 +996,7 @@ public class ExtractMethodProcessor implements MatchProvider {
  }

  private void declareVariableAtMethodCallLocation(String name) {
-    declareVariableAtMethodCallLocation(name, myOutputVariable.getType());
+    declareVariableAtMethodCallLocation(name, myReturnType);
  }

  private String declareVariableAtMethodCallLocation(String name, PsiType type) {
@@ -305,6 +305,11 @@ public class ExtractLightMethodObjectHandler {
      return false;
    }

+    @Override
+    public PsiType getReturnType() {
+      return null;
+    }
+
    @Override
    public void show() {}

@@ -130,6 +130,11 @@ public class ExtractMethodObjectDialog extends DialogWrapper implements Abstract
    return false;
  }

+  @Override
+  public PsiType getReturnType() {
+    return null;
+  }
+
  @NotNull
  protected Action[] createActions() {
    return new Action[]{getOKAction(), getCancelAction(), getHelpAction()};
@@ -197,7 +197,7 @@ public class TypeSelectorManagerImpl implements TypeSelectorManager {
    return result.toArray(PsiType.createArray(result.size()));
  }

-  private void collectAllSameShapedTypes(ExpectedTypeInfo[] expectedTypes, ArrayList<PsiType> allowedTypes) {
+  private static void collectAllSameShapedTypes(ExpectedTypeInfo[] expectedTypes, ArrayList<PsiType> allowedTypes) {
    for (ExpectedTypeInfo info : expectedTypes) {
      if (info.getKind() == ExpectedTypeInfo.TYPE_SAME_SHAPED) {
        allowedTypes.add(info.getDefaultType());
@@ -0,0 +1,13 @@
+class X {
+  void foo(java.util.List l) {
+    for (Object o : l) {
+      <selection>if (o == null) continue;
+      String x = bar(o);</selection>
+      System.out.println(x);
+    }
+  }
+
+  private String bar(Object o) {
+    return "";
+  }
+}
@@ -0,0 +1,22 @@
+import org.jetbrains.annotations.Nullable;
+
+class X {
+  void foo(java.util.List l) {
+    for (Object o : l) {
+        Object x = newMethod(o);
+        if (x == null) continue;
+      System.out.println(x);
+    }
+  }
+
+    @Nullable
+    private Object newMethod(Object o) {
+        if (o == null) return null;
+        String x = bar(o);
+        return x;
+    }
+
+    private String bar(Object o) {
+    return "";
+  }
+}
@@ -0,0 +1,6 @@
+class Test {
+  void foo() {
+    String x = <selection>""</selection>;
+    System.out.println(x);
+  }
+}
@@ -0,0 +1,7 @@
+class Test {
+  void foo() {
+    <selection>String x = "";</selection>
+    int len = x.length();
+    System.out.println(x);
+  }
+}
@@ -0,0 +1,6 @@
+class Test {
+  void foo() {
+    <selection>String x = "";</selection>
+    System.out.println(x);
+  }
+}
@@ -19,12 +19,10 @@ import com.intellij.JavaTestUtil;
 import com.intellij.codeInsight.CodeInsightUtil;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.openapi.project.Project;
-import com.intellij.psi.PsiDocumentManager;
-import com.intellij.psi.PsiElement;
-import com.intellij.psi.PsiExpression;
-import com.intellij.psi.PsiFile;
+import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.CodeStyleSettings;
 import com.intellij.psi.codeStyle.CodeStyleSettingsManager;
+import com.intellij.psi.search.GlobalSearchScope;
 import com.intellij.refactoring.extractMethod.ExtractMethodHandler;
 import com.intellij.refactoring.extractMethod.ExtractMethodProcessor;
 import com.intellij.refactoring.extractMethod.PrepareFailedException;
@@ -623,6 +621,10 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
    doTest();
  }

+  public void testChangedReturnType() throws Exception {
+    doTestReturnTypeChanged(PsiType.getJavaLangObject(getPsiManager(), GlobalSearchScope.allScope(getProject())));
+  }
+
  private void doTestDisabledParam() throws PrepareFailedException {
    final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject());
    settings.ELSE_ON_NEW_LINE = true;
@@ -633,6 +635,16 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
    checkResultByFile(BASE_PATH + getTestName(false) + "_after.java");
  }

+  private void doTestReturnTypeChanged(PsiType type) throws PrepareFailedException {
+    final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject());
+    settings.ELSE_ON_NEW_LINE = true;
+    settings.CATCH_ON_NEW_LINE = myCatchOnNewLine;
+    configureByFile(BASE_PATH + getTestName(false) + ".java");
+    boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, type);
+    assertTrue(success);
+    checkResultByFile(BASE_PATH + getTestName(false) + "_after.java");
+  }
+
  private void doPrepareErrorTest(final String expectedMessage) throws Exception {
    String expectedError = null;
    try {
@@ -688,6 +700,18 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
                                             final boolean extractChainedConstructor,
                                             int... disabledParams)
    throws PrepareFailedException, IncorrectOperationException {
+    return performExtractMethod(doRefactor, replaceAllDuplicates, editor, file, project, extractChainedConstructor, null, disabledParams);
+  }
+
+  public static boolean performExtractMethod(boolean doRefactor,
+                                             boolean replaceAllDuplicates,
+                                             Editor editor,
+                                             PsiFile file,
+                                             Project project,
+                                             final boolean extractChainedConstructor,
+                                             PsiType returnType,
+                                             int... disabledParams)
+    throws PrepareFailedException, IncorrectOperationException {
    int startOffset = editor.getSelectionModel().getSelectionStart();
    int endOffset = editor.getSelectionModel().getSelectionEnd();

@@ -717,7 +741,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
    }

    if (doRefactor) {
-      processor.testPrepare();
+      processor.testPrepare(returnType);
      processor.testNullness();
      if (disabledParams != null) {
        for (int param : disabledParams) {
@@ -0,0 +1,101 @@
+/*
+ * Copyright 2000-2014 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.
+ */
+
+/*
+ * User: anna
+ * Date: 25-May-2010
+ */
+package com.intellij.refactoring;
+
+import com.intellij.JavaTestUtil;
+import com.intellij.codeInsight.CodeInsightUtil;
+import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.PsiExpression;
+import com.intellij.psi.PsiFile;
+import com.intellij.refactoring.extractMethod.ExtractMethodProcessor;
+import com.intellij.refactoring.ui.TypeSelectorManager;
+import com.intellij.refactoring.ui.TypeSelectorManagerImpl;
+import com.intellij.testFramework.LightCodeInsightTestCase;
+import org.jetbrains.annotations.NonNls;
+import org.jetbrains.annotations.NotNull;
+
+import javax.swing.*;
+
+public class SuggestedReturnTypesTest extends LightCodeInsightTestCase {
+  @NonNls private static final String BASE_PATH = "/refactoring/suggestedTypes/";
+
+  @NotNull
+  @Override
+  protected String getTestDataPath() {
+    return JavaTestUtil.getJavaTestDataPath();
+  }
+
+  public void testOutputUsedAsObject() throws Exception {
+    doTest("String", "Object", "Serializable", "Comparable<String>", "CharSequence");
+  }
+
+  public void testOutputUsedAsCharSequence() throws Exception {
+    doTest("String", "CharSequence");
+  }
+
+  public void testFromExpression() throws Exception {
+    doTest("String");
+  }
+
+  private void doTest(String... types) throws Exception {
+    configureByFile(BASE_PATH + getTestName(false) + ".java");
+
+    final Editor editor = getEditor();
+    final PsiFile file = getFile();
+    final Project project = getProject();
+
+    int startOffset = editor.getSelectionModel().getSelectionStart();
+    int endOffset = editor.getSelectionModel().getSelectionEnd();
+    PsiElement[] elements;
+    PsiExpression expr = CodeInsightUtil.findExpressionInRange(file, startOffset, endOffset);
+    if (expr != null) {
+      elements = new PsiElement[]{expr};
+    }
+    else {
+      elements = CodeInsightUtil.findStatementsInRange(file, startOffset, endOffset);
+    }
+    assertTrue(elements.length > 0);
+
+    final ExtractMethodProcessor processor =
+      new ExtractMethodProcessor(project, editor, elements, null, "Extract Method", "newMethod", null);
+
+    processor.prepare();
+
+    final PsiExpression[] occurrences = processor.findOccurrences();
+    final TypeSelectorManager manager = new TypeSelectorManagerImpl(project, processor.getReturnType(), occurrences, true);
+    final JComponent component = manager.getTypeSelector().getComponent();
+    if (types.length > 1) {
+      assertTrue("One type suggested", component instanceof JComboBox);
+      final DefaultComboBoxModel model = (DefaultComboBoxModel)((JComboBox)component).getModel();
+      assertEquals(types.length, model.getSize());
+      for (int i = 0, typesLength = types.length; i < typesLength; i++) {
+        String type = types[i];
+        assertEquals(type, model.getElementAt(i).toString());
+      }
+    }
+    else if (types.length == 1) {
+      assertTrue("Multiple types suggested", component instanceof JLabel);
+      assertEquals(types[0], ((JLabel)component).getText());
+    }
+  }
+}