IDEADEV-37189: Intentions: wrong suggested "Convert parameter to map entry" for Map...
authorunknown <Maxim.Medvedev@.Labs.IntelliJ.Net>
Thu, 1 Oct 2009 15:10:33 +0000 (19:10 +0400)
committerMaxim Medvedev <maxim.medvedev@jetbrains.com>
Thu, 1 Oct 2009 15:15:46 +0000 (19:15 +0400)
plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/style/parameterToEntry/ConvertParameterToMapEntryIntention.java
plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyValidationUtil.java

index ccd9bba45499eaefb4e56f22bba98eab52f12433..a132911ac9fff7b100a0753fee0cc16c39506739 100644 (file)
@@ -20,6 +20,7 @@ import com.intellij.util.Function;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.Query;
 import com.intellij.util.containers.ContainerUtil;
+import com.intellij.util.containers.hash.HashMap;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -47,14 +48,15 @@ import org.jetbrains.plugins.groovy.refactoring.GroovyValidationUtil;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 
 /**
  * @author ilyas
  */
 public class ConvertParameterToMapEntryIntention extends Intention {
 
-  private static final Logger LOG = Logger.getInstance("#org.jetbrains.plugins.groovy.intentions.style.ConvertParameterToMapEntryIntention")
-    ;
+  private static final Logger LOG =
+    Logger.getInstance("#org.jetbrains.plugins.groovy.intentions.style.ConvertParameterToMapEntryIntention");
   @NonNls private static final String CLOSURE_CAPTION = "closure";
   @NonNls private static final String CLOSURE_CAPTION_CAP = "Closure";
   @NonNls private static final String METHOD_CAPTION = "method";
@@ -104,7 +106,7 @@ public class ConvertParameterToMapEntryIntention extends Intention {
             @Override
             protected void doOKAction() {
               String name = getEnteredName();
-              ArrayList<String> conflicts = new ArrayList<String>();
+              Map<PsiElement, String> conflicts = new HashMap<PsiElement, String>();
               GroovyValidationUtil.validateNewParameterName(firstParam, conflicts, name);
               if (reportConflicts(conflicts, project)) {
                 performRefactoring(element, owner, occurrences, createNewFirst(), name, specifyTypeExplicitly());
@@ -118,7 +120,8 @@ public class ConvertParameterToMapEntryIntention extends Intention {
               dialog.show();
             }
           });
-        } else {
+        }
+        else {
           //todo add statictics manager
           performRefactoring(element, owner, occurrences, true,
                              (new GroovyValidationUtil.ParameterNameSuggester("attrs", firstParam)).generateName(), false);
@@ -144,7 +147,6 @@ public class ConvertParameterToMapEntryIntention extends Intention {
                                          final boolean specifyMapType) {
     final GrParameter param = getAppropriateParameter(element);
     assert param != null;
-    // todo check for parameter invocations as map property
     final String paramName = param.getName();
     final String mapName = createNewFirstParam ? mapParamName : getFirstParameter(owner).getName();
 
@@ -152,13 +154,12 @@ public class ConvertParameterToMapEntryIntention extends Intention {
     final Project project = element.getProject();
     final Runnable runnable = new Runnable() {
       public void run() {
-        boolean doCreate = createNewFirstParam;
         final GroovyPsiElementFactory factory = GroovyPsiElementFactory.getInstance(project);
         final GrParameterList list = owner.getParameterList();
         assert list != null;
         final int index = list.getParameterNumber(param);
         final int newIndex = createNewFirstParam ? index : index - 1;
-        if (!doCreate && index <= 0) { // bad undo
+        if (!createNewFirstParam && index <= 0) { // bad undo
           return;
         }
 
@@ -174,7 +175,7 @@ public class ConvertParameterToMapEntryIntention extends Intention {
         }
 
         //Add new map parameter to closure/method if it's necessary
-        if (doCreate) {
+        if (createNewFirstParam) {
           try {
             final GrParameter newParam = factory.createParameter(mapName, specifyMapType ? MAP_TYPE_TEXT : "", null);
             list.addParameterToHead(newParam);
@@ -372,24 +373,34 @@ public class ConvertParameterToMapEntryIntention extends Intention {
 
   private static class MyPsiElementPredicate implements PsiElementPredicate {
     public boolean satisfiedBy(final PsiElement element) {
+      GrParametersOwner owner = null;
       if (element instanceof GrParameter) {
-        final GrParametersOwner owner = PsiTreeUtil.getParentOfType(element, GrParametersOwner.class);
-        if (owner instanceof GrClosableBlock || owner instanceof GrMethodImpl) return true;
-        return false;
+        owner = PsiTreeUtil.getParentOfType(element, GrParametersOwner.class);
       }
-
-      if (element instanceof GrReferenceExpression) {
+      else if (element instanceof GrReferenceExpression) {
         GrReferenceExpression expr = (GrReferenceExpression)element;
         if (expr.getQualifierExpression() != null) return false;
         final PsiElement resolved = expr.resolve();
         if (!(resolved instanceof GrParameter)) return false;
-        final GrParametersOwner owner = PsiTreeUtil.getParentOfType(resolved, GrParametersOwner.class);
-        if (owner instanceof GrClosableBlock || owner instanceof GrMethodImpl) return true;
+        owner = PsiTreeUtil.getParentOfType(resolved, GrParametersOwner.class);
       }
-      return false;
+      if (!(owner instanceof GrClosableBlock || owner instanceof GrMethodImpl)) return false;
+      return checkForMapParameters(owner);
     }
   }
 
+  private static boolean checkForMapParameters(GrParametersOwner owner) {
+    final GrParameter[] parameters = owner.getParameters();
+    if (parameters.length != 1) return true;
+
+    final GrParameter parameter = parameters[0];
+    final PsiType type = parameter.getTypeGroovy();
+    if (!(type instanceof PsiClassType)) return true;
+
+    final PsiClass psiClass = ((PsiClassType)type).resolve();
+    return psiClass == null || !CommonClassNames.JAVA_UTIL_MAP.equals(psiClass.getQualifiedName());
+  }
+
   private static void showErrorMessage(String message, final Project project) {
     CommonRefactoringUtil.showErrorMessage(REFACTORING_NAME, message, null, project);
   }
@@ -399,7 +410,7 @@ public class ConvertParameterToMapEntryIntention extends Intention {
     CodeStyleManager.getInstance(owner.getProject()).reformat(owner);
   }
 
-  private static boolean reportConflicts(final ArrayList<String> conflicts, final Project project) {
+  private static boolean reportConflicts(final Map<PsiElement, String> conflicts, final Project project) {
     if (conflicts.size() == 0) return true;
     ConflictsDialog conflictsDialog = new ConflictsDialog(project, conflicts);
     conflictsDialog.show();
index 0a840a9bc31a108262fa400ae6110b6364880f1b..f18c8b32b90a80e3f10bb1e7cf88980aa041e8b4 100644 (file)
@@ -5,22 +5,23 @@ import com.intellij.psi.PsiFile;
 import com.intellij.psi.PsiNamedElement;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.refactoring.util.CommonRefactoringUtil;
+import com.intellij.util.containers.HashMap;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.plugins.groovy.lang.psi.GroovyFile;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.*;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrForClause;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameter;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameterList;
-import org.jetbrains.plugins.groovy.lang.psi.GroovyFile;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition;
 
-import java.util.ArrayList;
+import java.util.Map;
 
 /**
  * @author ilyas
  */
 public class GroovyValidationUtil {
 
-  public static boolean validateNewParameterName(GrParameter variable, ArrayList<String> conflicts, @NotNull String varName) {
+  public static boolean validateNewParameterName(GrParameter variable, Map<PsiElement, String> conflicts, @NotNull String varName) {
     GrParameterList list = PsiTreeUtil.getParentOfType(variable, GrParameterList.class);
     GrParametersOwner owner = PsiTreeUtil.getParentOfType(variable, GrParametersOwner.class);
     assert owner != null;
@@ -36,7 +37,7 @@ public class GroovyValidationUtil {
 
   private static void validateVariableOccurrencesUp(PsiElement parent,
                                                     PsiElement lastParent,
-                                                    ArrayList<String> conflicts,
+                                                    Map<PsiElement, String> conflicts,
                                                     @NotNull String varName,
                                                     final boolean containerIsFile) {
     if (!containerIsFile && (parent instanceof PsiFile) || parent == null) return;
@@ -77,7 +78,7 @@ public class GroovyValidationUtil {
 
   private static void validateVariableOccurrencesDown(PsiElement parent,
                                                       PsiElement startChild,
-                                                      ArrayList<String> conflicts,
+                                                      Map<PsiElement, String> conflicts,
                                                       @NotNull String varName) {
     PsiElement child = parent.getLastChild();
     while (child != null && child != startChild && !(child instanceof GrTypeDefinition)) {
@@ -86,7 +87,7 @@ public class GroovyValidationUtil {
     }
   }
 
-  private static void validateVariableOccurrencesDownImpl(final PsiElement child, final ArrayList<String> conflicts, final String varName) {
+  private static void validateVariableOccurrencesDownImpl(final PsiElement child, final Map<PsiElement, String> conflicts, final String varName) {
     if (child instanceof PsiNamedElement) {
       PsiNamedElement element = (PsiNamedElement)child;
       if (varName.equals(element.getName())) {
@@ -101,13 +102,13 @@ public class GroovyValidationUtil {
     }
   }
 
-  private static void addConflict(final String varName, final PsiNamedElement element, final ArrayList<String> conflicts) {
+  private static void addConflict(final String varName, final PsiNamedElement element, final Map<PsiElement, String> conflicts) {
     if (element instanceof GrParameter) {
-      conflicts.add(GroovyRefactoringBundle.message("variable.conflicts.with.parameter.0", CommonRefactoringUtil.htmlEmphasize(varName)));
+      conflicts.put(element, GroovyRefactoringBundle.message("variable.conflicts.with.parameter.0", CommonRefactoringUtil.htmlEmphasize(varName)));
     } else if (element instanceof GrField) {
-      conflicts.add(GroovyRefactoringBundle.message("variable.conflicts.with.field.0", CommonRefactoringUtil.htmlEmphasize(varName)));
+      conflicts.put(element, GroovyRefactoringBundle.message("variable.conflicts.with.field.0", CommonRefactoringUtil.htmlEmphasize(varName)));
     } else {
-      conflicts.add(GroovyRefactoringBundle.message("variable.conflicts.with.variable.0", CommonRefactoringUtil.htmlEmphasize(varName)));
+      conflicts.put(element, GroovyRefactoringBundle.message("variable.conflicts.with.variable.0", CommonRefactoringUtil.htmlEmphasize(varName)));
     }
   }
 
@@ -123,11 +124,11 @@ public class GroovyValidationUtil {
     public String generateName() {
       String name = myName;
       int i = 1;
-      ArrayList<String> confl = new ArrayList<String>();
+      Map<PsiElement, String> confl = new HashMap<PsiElement, String>();
       while (!validateNewParameterName(myParameter, confl, name)) {
         name = myName + i;
         i++;
-        confl = new ArrayList<String>();
+        confl = new HashMap<PsiElement, String>();
       }
       return name;
     }