[groovy] refactor 'change to operator' inspection
authorDaniil Ovchinnikov <daniil.ovchinnikov@jetbrains.com>
Tue, 15 Nov 2016 14:23:13 +0000 (17:23 +0300)
committerDaniil Ovchinnikov <daniil.ovchinnikov@jetbrains.com>
Tue, 15 Nov 2016 19:32:19 +0000 (22:32 +0300)
- use strings instead of IElementTypes
- get rid of call transformation
- simplify weird builder
- highlight only method reference instead of whole method call
- @NotNull

15 files changed:
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/ChangeToOperatorInspection.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/data/ReplacementData.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/AsBooleanTransformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/BinaryTransformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/CompareToTransformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/EqualsTransformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/GetAtTransformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/IsCaseTransformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/PutAtTransformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/SimpleBinaryTransformation.java [moved from plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/CallTransformation.java with 63% similarity]
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/Transformation.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/Transformations.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/changeToOperator/transformations/UnaryTransformation.java
plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrChangeToOperatorTest.groovy

index 90f9afefe55804174bb8f9221467cfce94bc1ac6..9ad98f402eb7dd6e768fa90e887de5198a0b149f 100644 (file)
@@ -42,10 +42,10 @@ parameter.can.be.final.tooltip=Parameter ''{0}'' can be final
 groovy.probable.bugs=Probable bugs
 equals.between.inconvertible.types='equals()' between objects of inconvertible types
 equals.between.inconvertible.types.tooltip=<code>{0}</code> between objects of inconvertible types ''{1}'' and ''{2}''
-change.to.operator.message=''{0}'' can be changed to operator
-change.to.operator.fix=Change ''{0}'' to operator
-change.to.operator.double.negation.option=Use double negation (i.e. !!)
-change.to.operator.compareTo.equality.option=Change compareTo equality to equals (i.e. ==)
+replace.with.operator.message=''{0}'' can be replaced with operator
+replace.with.operator.fix=Replace ''{0}'' with operator
+replace.with.operator.double.negation.option=Use double negation (i.e. !!)
+replace.with.operator.compareTo.equality.option=Replace 'compareTo' equality to equals (i.e. ==)
 unassigned.access=Variable Not Assigned
 unassigned.access.short.name=VariableNotAssigned
 unassigned.access.tooltip=Variable ''{0}'' might not be assigned
index 9f9d98fd147ba690957fc4c4c9bbc8b75b0c7db7..64b082171ecfd9468c0bda81397742e79be5bb19 100644 (file)
@@ -19,9 +19,9 @@ import com.intellij.codeInspection.LocalQuickFix;
 import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiMethod;
 import com.intellij.psi.PsiModifier;
-import com.intellij.psi.PsiModifierListOwner;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.plugins.groovy.codeInspection.BaseInspection;
 import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor;
@@ -29,19 +29,17 @@ import org.jetbrains.plugins.groovy.codeInspection.GroovyFix;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.ReplacementData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations.Transformation;
-import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations.Transformations;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression;
 
 import javax.swing.*;
-import java.util.Map;
 
 import static com.intellij.codeInspection.ProblemHighlightType.GENERIC_ERROR_OR_WARNING;
 import static org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle.message;
+import static org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations.Transformations.TRANSFORMATIONS;
 
 public class ChangeToOperatorInspection extends BaseInspection {
-  private static final Map<String, Transformation> TRANSFORMATIONS = Transformations.get();
-
   public boolean useDoubleNegation = true;
   public boolean shouldChangeCompareToEqualityToEquals = true;
 
@@ -51,44 +49,45 @@ public class ChangeToOperatorInspection extends BaseInspection {
     return new BaseInspectionVisitor() {
       @Override
       public void visitMethodCallExpression(@NotNull GrMethodCallExpression methodCallExpression) {
-        super.visitMethodCallExpression(methodCallExpression);
-        processMethodCall(methodCallExpression);
-      }
+        GrExpression invokedExpression = methodCallExpression.getInvokedExpression();
+        if (!(invokedExpression instanceof GrReferenceExpression)) return;
+
+        PsiElement element = ((GrReferenceExpression)invokedExpression).getReferenceNameElement();
+        if (element == null) return;
 
-      private void processMethodCall(GrMethodCallExpression methodCallExpression) {
         PsiMethod method = methodCallExpression.resolveMethod();
-        if (!isValid(method)) return;
+        if (method == null || method.hasModifierProperty(PsiModifier.STATIC)) return;
 
         String methodName = method.getName();
         Transformation transformation = TRANSFORMATIONS.get(methodName);
         if (transformation == null) return;
 
-        // TODO apply transformation recursively
         OptionsData optionsData = new OptionsData(useDoubleNegation, shouldChangeCompareToEqualityToEquals);
         ReplacementData replacement = transformation.transform(methodCallExpression, optionsData);
         if (replacement == null) return;
 
-        GrExpression expression = replacement.getExpression();
-        String text = expression.getText();
-        GroovyFix fix = getFix(message("change.to.operator.fix", text), replacement.getReplacement());
-        registerError(expression, message("change.to.operator.message", text), new LocalQuickFix[]{fix}, GENERIC_ERROR_OR_WARNING);
-      }
-
-      private boolean isValid(PsiModifierListOwner method) {
-        return ((method != null) && !method.hasModifierProperty(PsiModifier.STATIC));
+        GroovyFix fix = getFix(message("replace.with.operator.fix", methodName), replacement);
+        registerError(element, message("replace.with.operator.message", methodName), new LocalQuickFix[]{fix}, GENERIC_ERROR_OR_WARNING);
       }
 
-      private GroovyFix getFix(@NotNull final String message, final String replacement) {
+      private GroovyFix getFix(@NotNull final String title, final ReplacementData replacement) {
         return new GroovyFix() {
+
           @NotNull
           @Override
-          public String getName() {
-            return message;
+          public String getFamilyName() {
+            return title;
           }
 
           @Override
           protected void doFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
-            replaceExpression((GrExpression)descriptor.getPsiElement(), replacement);
+            PsiElement element = descriptor.getPsiElement().getParent();
+            if (!(element instanceof GrReferenceExpression)) return;
+
+            PsiElement call = element.getParent();
+            if (!(call instanceof GrMethodCallExpression)) return;
+
+            replaceExpression(replacement.getElementToReplace((GrMethodCallExpression)call), replacement.getReplacement());
           }
         };
       }
@@ -98,8 +97,8 @@ public class ChangeToOperatorInspection extends BaseInspection {
   @Override
   public JComponent createOptionsPanel() {
     MultipleCheckboxOptionsPanel optionsPanel = new MultipleCheckboxOptionsPanel(this);
-    optionsPanel.addCheckbox(message("change.to.operator.double.negation.option"), "useDoubleNegation");
-    optionsPanel.addCheckbox(message("change.to.operator.compareTo.equality.option"), "shouldChangeCompareToEqualityToEquals");
+    optionsPanel.addCheckbox(message("replace.with.operator.double.negation.option"), "useDoubleNegation");
+    optionsPanel.addCheckbox(message("replace.with.operator.compareTo.equality.option"), "shouldChangeCompareToEqualityToEquals");
     return optionsPanel;
   }
 }
\ No newline at end of file
index 28ccbd207a66906520f450ef38ffd4931b5517ec..ae2bf323dfe7304f16721427963f28541aa65916 100644 (file)
  */
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data;
 
+import com.intellij.util.NotNullFunction;
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression;
 
 public final class ReplacementData {
-  private final GrExpression element;
-  private final String replacement;
 
-  public ReplacementData(@NotNull GrExpression element, @NotNull String replacement) {
-    this.element = element;
-    this.replacement = replacement;
-  }
+  private final String myReplacement;
+  private final NotNullFunction<GrMethodCallExpression, GrExpression> myElementToReplace;
 
-  public GrExpression getExpression() {
-    return element;
+  public ReplacementData(@NotNull String replacement, @Nullable NotNullFunction<GrMethodCallExpression, GrExpression> elementToReplace) {
+    this.myReplacement = replacement;
+    this.myElementToReplace = elementToReplace;
   }
 
+  @NotNull
+  @Contract(pure = true)
   public String getReplacement() {
-    return replacement;
+    return myReplacement;
+  }
+
+  @NotNull
+  public GrExpression getElementToReplace(@NotNull GrMethodCallExpression call) {
+    return myElementToReplace == null ? call : myElementToReplace.fun(call);
   }
 }
index fcae291e5955fa03576c3f90fc26420f1feaf1f5..1d26a707c54a4d37abc47f6d8c013419dbe082b8 100644 (file)
@@ -16,6 +16,7 @@
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations;
 
 import com.intellij.psi.PsiElement;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
@@ -30,12 +31,9 @@ class AsBooleanTransformation extends UnaryTransformation {
   public static final String NEGATION = mLNOT.toString();
   public static final String DOUBLE_NEGATION = NEGATION + NEGATION;
 
-  public AsBooleanTransformation() {
-    super(null);
-  }
-
+  @NotNull
   @Override
-  protected GrExpression getExpandedElement(GrMethodCallExpression callExpression) {
+  protected GrExpression getExpandedElement(@NotNull GrMethodCallExpression callExpression) {
     PsiElement parent = callExpression.getParent();
     if (isNegation(parent)) {
       return (GrExpression)parent;
@@ -46,7 +44,7 @@ class AsBooleanTransformation extends UnaryTransformation {
   }
 
   @Override
-  @Nullable 
+  @Nullable
   protected String getPrefix(MethodCallData methodInfo, OptionsData optionsData) {
     if (methodInfo.isNegated()) {
       return NEGATION;
@@ -64,5 +62,5 @@ class AsBooleanTransformation extends UnaryTransformation {
 
   private static boolean isImplicitlyBoolean(MethodCallData methodInfo) {
     return methodInfo.getBackingElement().getParent() instanceof GrIfStatement;
-  } 
+  }
 }
index 555932e6a1a21ab7709136b7b3b8910ea2c2a936..462dc017c9677c9269f95d3a8084e21dd2d1acd1 100644 (file)
@@ -15,7 +15,6 @@
  */
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations;
 
-import com.intellij.psi.tree.IElementType;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
@@ -27,16 +26,13 @@ import static java.lang.String.format;
  * a.equals(b)  → (a == b)
  * !a.equals(b) → (a != b)
  */
-public class BinaryTransformation extends Transformation {
-  public BinaryTransformation(@Nullable IElementType operator) {
-    super(operator);
-  }
+abstract class BinaryTransformation extends Transformation {
 
   @Override
   @Nullable
   public String getReplacement(MethodCallData methodInfo, OptionsData optionsData) {
     String lhs = getLhs(methodInfo);
-    IElementType operator = getOperator(methodInfo, optionsData);
+    String operator = getOperator(methodInfo, optionsData);
     String rhs = getRhs(methodInfo);
     if (lhs == null || operator == null || rhs == null) return null;
 
@@ -49,9 +45,7 @@ public class BinaryTransformation extends Transformation {
   }
 
   @Nullable
-  protected IElementType getOperator(MethodCallData methodInfo, OptionsData optionsData) {
-    return operator;
-  }
+  protected abstract String getOperator(MethodCallData methodInfo, OptionsData optionsData);
 
   @Nullable
   protected String getRhs(MethodCallData methodInfo) {
index acd951758e00245a5ce5144d13763fc17e90e6b0..ed02135ef1c9dd0d5b92631c1274312bf2e36c97 100644 (file)
@@ -17,6 +17,7 @@ package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformat
 
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.tree.IElementType;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
@@ -27,22 +28,19 @@ import static org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes.*;
 import static org.jetbrains.plugins.groovy.lang.psi.impl.utils.ComparisonUtils.isComparison;
 
 class CompareToTransformation extends BinaryTransformation {
-  public CompareToTransformation() {
-    super(null);
-  }
 
+  @NotNull
   @Override
-  protected GrExpression getExpandedElement(GrMethodCallExpression callExpression) {
+  protected GrExpression getExpandedElement(@NotNull GrMethodCallExpression callExpression) {
     PsiElement parent = callExpression.getParent();
-    return isComparison(parent) ? (GrExpression)parent 
-                                : super.getExpandedElement(callExpression);
+    return isComparison(parent) ? (GrExpression)parent : super.getExpandedElement(callExpression);
   }
 
   @Override
-  protected IElementType getOperator(MethodCallData methodInfo, OptionsData optionsData) {
+  protected String getOperator(MethodCallData methodInfo, OptionsData optionsData) {
     IElementType comparison = methodInfo.getComparison();
     if (shouldChangeCompareToEqualityToEquals(optionsData, comparison)) {
-      return firstNonNull(comparison, mCOMPARE_TO);
+      return firstNonNull(comparison, mCOMPARE_TO).toString();
     }
     else {
       return null;
index 0ddb90e629edda7d87974b9a4179fc56abe0cc43..40b001d4bf8ef3b700225fe7c7aeae899c9e6106 100644 (file)
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations;
 
 import com.intellij.psi.PsiElement;
-import com.intellij.psi.tree.IElementType;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression;
 
-import static org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes.mEQUAL;
-import static org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes.mNOT_EQUAL;
 import static org.jetbrains.plugins.groovy.lang.psi.impl.utils.BoolUtils.isNegation;
 
 class EqualsTransformation extends BinaryTransformation {
-  public EqualsTransformation() {
-    super(null);
-  }
 
+  @NotNull
   @Override
-  protected GrExpression getExpandedElement(GrMethodCallExpression callExpression) {
+  protected GrExpression getExpandedElement(@NotNull GrMethodCallExpression callExpression) {
     PsiElement parent = callExpression.getParent();
-    return isNegation(parent) ? (GrExpression)parent
-                              : super.getExpandedElement(callExpression);
+    return isNegation(parent) ? (GrExpression)parent : super.getExpandedElement(callExpression);
   }
 
   @Override
   @Nullable
-  protected IElementType getOperator(MethodCallData methodInfo, OptionsData optionsData) {
-    return methodInfo.isNegated() ? mNOT_EQUAL
-                                  : mEQUAL;
+  protected String getOperator(MethodCallData methodInfo, OptionsData optionsData) {
+    return methodInfo.isNegated() ? "!=" : "==";
   }
 }
index b42d8aa83da3389bf74193c5833295d25d980faa..75a88636722436c85b9f84e988dbdfe3d7ec568d 100644 (file)
@@ -22,9 +22,6 @@ import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.Options
 import static java.lang.String.format;
 
 class GetAtTransformation extends Transformation {
-  public GetAtTransformation() {
-    super(null);
-  }
 
   @Override
   @Nullable
@@ -32,7 +29,6 @@ class GetAtTransformation extends Transformation {
     String argument = methodInfo.getArgument(0);
     if (argument == null) return null;
 
-    return format("%s[%s]",
-                  methodInfo.getBase(), argument);
+    return format("%s[%s]", methodInfo.getBase(), argument);
   }
 }
index 86208aa5633b2712f55b795a6f172fade4e70aea..e4d1ea3479a7a3ad110e398d996e4174e338411b 100644 (file)
@@ -17,11 +17,11 @@ package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformat
 
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
-import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes;
 
-class IsCaseTransformation extends BinaryTransformation {
+class IsCaseTransformation extends SimpleBinaryTransformation {
+
   public IsCaseTransformation() {
-    super(GroovyTokenTypes.kIN);
+    super("in");
   }
 
   @Override
index 0a18a41dbf75113770f4e7ab677926b845661d31..191508c91a43afe688b97251f8bf9da3e2da31c0 100644 (file)
@@ -25,7 +25,6 @@ class PutAtTransformation extends Transformation {
   private final Transformation getAtTransformation;
 
   public PutAtTransformation(Transformation getAtTransformation) {
-    super(null);
     this.getAtTransformation = getAtTransformation;
   }
 
  */
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations;
 
+import com.intellij.psi.tree.IElementType;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
 
-import static org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes.mLPAREN;
-import static org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes.mRPAREN;
+public class SimpleBinaryTransformation extends BinaryTransformation {
 
-class CallTransformation extends UnaryTransformation {
-  public static final String EMPTY_PARENS = mLPAREN.toString() + mRPAREN;
+  private final String myOperator;
 
-  public CallTransformation() {
-    super(null);
+  public SimpleBinaryTransformation(@NotNull IElementType operatorType) {
+    this(operatorType.toString());
   }
 
+  public SimpleBinaryTransformation(@NotNull String operator) {
+    myOperator = operator;
+  }
+
+  @NotNull
   @Override
-  public String getReplacement(MethodCallData call, OptionsData options) {
-    return super.getReplacement(call, options) + EMPTY_PARENS;
+  protected String getOperator(MethodCallData methodInfo, OptionsData optionsData) {
+    return myOperator;
   }
 }
index 50afdcebf1d906b93c63b05d747ee80ebeeacfdf..2ffb3468eb2ae2edb632f7289222c657d532468f 100644 (file)
@@ -15,7 +15,7 @@
  */
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations;
 
-import com.intellij.psi.tree.IElementType;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
@@ -24,12 +24,6 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpres
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression;
 
 public abstract class Transformation {
-  @Nullable
-  protected final IElementType operator;
-
-  public Transformation(@Nullable IElementType operator) {
-    this.operator = operator;
-  }
 
   @Nullable
   public ReplacementData transform(GrMethodCallExpression callExpression, OptionsData optionsData) {
@@ -40,10 +34,11 @@ public abstract class Transformation {
     String replacement = getReplacement(methodInfo, optionsData);
     if (replacement == null) return null;
 
-    return new ReplacementData(element, replacement);
+    return new ReplacementData(replacement, this::getExpandedElement);
   }
 
-  protected GrExpression getExpandedElement(GrMethodCallExpression callExpression) {
+  @NotNull
+  protected GrExpression getExpandedElement(@NotNull GrMethodCallExpression callExpression) {
     return callExpression;
   }
 
index c02886b9f5776c6f49047ff86ba7e896724c7cc1..daf73a68cc0479dada74f07665df1409ba0e1722 100644 (file)
  */
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations;
 
-import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.ContainerUtil.ImmutableMapBuilder;
-import org.jetbrains.plugins.groovy.lang.lexer.GroovyElementType;
 
 import java.util.Map;
 
 import static org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes.*;
 import static org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.HardcodedGroovyMethodConstants.*;
 
-public class Transformations {
-  private final ImmutableMapBuilder<String, Transformation> result = ContainerUtil.immutableMapBuilder();
-
-  public static Map<String, Transformation> get() {
-    Transformations builder = new Transformations();
-    return builder.build();
-  }
-
-  private Transformations() {
-    addUnary();
-    addBinary();
-    addCustom();
-  }
-
-  private void addUnary() {
-    result.put(BITWISE_NEGATE, new UnaryTransformation(mBNOT));
-    result.put(NEGATIVE, new UnaryTransformation(mMINUS));
-    result.put(POSITIVE, new UnaryTransformation(mPLUS));
-    result.put(NEXT, new UnaryTransformation(mINC));
-    result.put(PREVIOUS, new UnaryTransformation(mDEC));
-
-    result.put(AS_BOOLEAN, new AsBooleanTransformation());
-    //result.put(CALL, new CallTransformation());
-  }
-
-  private void addBinary() {
-    result.put(PLUS, new BinaryTransformation(mPLUS));
-    result.put(MINUS, new BinaryTransformation(mMINUS));
-    result.put(MULTIPLY, new BinaryTransformation(mSTAR));
-    result.put(POWER, new BinaryTransformation(mSTAR_STAR));
-    result.put(DIV, new BinaryTransformation(mDIV));
-    result.put(MOD, new BinaryTransformation(mMOD));
-    result.put(OR, new BinaryTransformation(mBOR));
-    result.put(AND, new BinaryTransformation(mBAND));
-    result.put(XOR, new BinaryTransformation(mBXOR));
-    result.put(LEFT_SHIFT, new BinaryTransformation(new GroovyElementType("<<")));
-    result.put(RIGHT_SHIFT, new BinaryTransformation(new GroovyElementType(">>")));
-    result.put(RIGHT_SHIFT_UNSIGNED, new BinaryTransformation(new GroovyElementType(">>>")));
-
-    result.put(AS_TYPE, new BinaryTransformation(kAS));
-
-    result.put(IS_CASE, new IsCaseTransformation());
-    result.put(EQUALS, new EqualsTransformation());
-    result.put(COMPARE_TO, new CompareToTransformation());
-  }
-
-  private void addCustom() {
-    Transformation getAtTransformation = new GetAtTransformation();
-    result.put(GET_AT, getAtTransformation);
-    result.put(PUT_AT, new PutAtTransformation(getAtTransformation));
-  }
-
-  private Map<String, Transformation> build() {
-    return result.build();
-  }
+public interface Transformations {
+
+  Map<String, Transformation> TRANSFORMATIONS = new ImmutableMapBuilder<String, Transformation>()
+
+    // unary
+    .put(BITWISE_NEGATE, new UnaryTransformation(mBNOT))
+    .put(NEGATIVE, new UnaryTransformation(mMINUS))
+    .put(POSITIVE, new UnaryTransformation(mPLUS))
+    .put(NEXT, new UnaryTransformation(mINC))
+    .put(PREVIOUS, new UnaryTransformation(mDEC))
+    .put(AS_BOOLEAN, new AsBooleanTransformation())
+
+    // binary
+    .put(PLUS, new SimpleBinaryTransformation(mPLUS))
+    .put(MINUS, new SimpleBinaryTransformation(mMINUS))
+    .put(MULTIPLY, new SimpleBinaryTransformation(mSTAR))
+    .put(POWER, new SimpleBinaryTransformation(mSTAR_STAR))
+    .put(DIV, new SimpleBinaryTransformation(mDIV))
+    .put(MOD, new SimpleBinaryTransformation(mMOD))
+    .put(OR, new SimpleBinaryTransformation(mBOR))
+    .put(AND, new SimpleBinaryTransformation(mBAND))
+    .put(XOR, new SimpleBinaryTransformation(mBXOR))
+    .put(LEFT_SHIFT, new SimpleBinaryTransformation("<<"))
+    .put(RIGHT_SHIFT, new SimpleBinaryTransformation(">>"))
+    .put(RIGHT_SHIFT_UNSIGNED, new SimpleBinaryTransformation(">>>"))
+    .put(AS_TYPE, new SimpleBinaryTransformation(kAS))
+    .put(IS_CASE, new IsCaseTransformation())
+    .put(EQUALS, new EqualsTransformation())
+    .put(COMPARE_TO, new CompareToTransformation())
+
+    // custom
+    .put(GET_AT, new GetAtTransformation())
+    .put(PUT_AT, new PutAtTransformation(new GetAtTransformation()))
+
+    .build();
 }
index db44889e19cd7657e6e6e34318611748892f79a3..6ae72cd354d8ecc35632807b343c0ed06a7c7fd8 100644 (file)
 package org.jetbrains.plugins.groovy.codeInspection.changeToOperator.transformations;
 
 import com.intellij.psi.tree.IElementType;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.MethodCallData;
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.data.OptionsData;
 
-import static com.google.common.base.MoreObjects.firstNonNull;
-
 /**
  * e.g.
  * !a.asBoolean()    → !a
@@ -29,8 +28,19 @@ import static com.google.common.base.MoreObjects.firstNonNull;
  * if(a.asBoolean()) → if(a)
  */
 public class UnaryTransformation extends Transformation {
-  public UnaryTransformation(@Nullable IElementType operator) {
-    super(operator);
+
+  private final @NotNull String myOperator;
+
+  public UnaryTransformation(@NotNull IElementType operatorType) {
+    this(operatorType.toString());
+  }
+
+  public UnaryTransformation(@NotNull String operator) {
+    myOperator = operator;
+  }
+
+  public UnaryTransformation() {
+    this("");
   }
 
   @Override
@@ -38,13 +48,13 @@ public class UnaryTransformation extends Transformation {
   public String getReplacement(MethodCallData call, OptionsData options) {
     String prefix = getPrefix(call, options);
     String base = call.getBase();
-    if ((prefix == null) || (base == null)) return null;
+    if (prefix == null) return null;
 
     return prefix + base;
   }
 
   @Nullable
   protected String getPrefix(MethodCallData call, OptionsData optionsData) {
-    return firstNonNull(operator, "").toString();
+    return myOperator;
   }
 }
\ No newline at end of file
index a5f5cf6a6dc1ec76d39a27461f365422c3f35667..9e2dfbbc8dafe9120e524666ee3f1acbc4be2411 100644 (file)
 package org.jetbrains.plugins.groovy.inspections
 
 import com.intellij.testFramework.LightProjectDescriptor
-import org.intellij.lang.annotations.Language
+import groovy.transform.CompileStatic
 import org.jetbrains.plugins.groovy.GroovyLightProjectDescriptor
 import org.jetbrains.plugins.groovy.LightGroovyTestCase
 import org.jetbrains.plugins.groovy.codeInspection.changeToOperator.ChangeToOperatorInspection
+import org.jetbrains.plugins.groovy.lang.psi.GroovyFile
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrBinaryExpression
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrMethodCall
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrUnaryExpression
 
-import java.util.regex.Pattern
-
-import static org.jetbrains.plugins.groovy.GroovyFileType.GROOVY_FILE_TYPE
-import static org.jetbrains.plugins.groovy.util.TestUtils.CARET_MARKER
-
+@CompileStatic
 class GrChangeToOperatorTest extends LightGroovyTestCase {
 
   final LightProjectDescriptor projectDescriptor = GroovyLightProjectDescriptor.GROOVY_LATEST
 
-  def inspection = new ChangeToOperatorInspection()
+  final ChangeToOperatorInspection inspection = new ChangeToOperatorInspection()
+
+  @Override
+  void setUp() throws Exception {
+    super.setUp()
+    fixture.with {
+      addFileToProject 'Operators.groovy', '''\
+class Operators {
+  def bitwiseNegate() { null }
+  def negative() { null }
+  def positive() { null }
+  def call() { null }
+  def next() { null }
+  def previous() { null }
+  def plus(b) { null }
+  def minus(b) { null }
+  def multiply(b) { null }
+  def power(b) { null }
+  def div(b) { null }
+  def mod(b) { null }
+  def or(b) { null }
+  def and(b) { null }
+  def xor(b) { null }
+  def leftShift(b) { null }
+  def rightShift(b) { null }
+  def rightShiftUnsigned(b) { null }
+  def asType(b) { null }
+  def getAt(b) { null }
+  def putAt(b, c) { null }
+  boolean asBoolean() { true }
+  boolean isCase(b) { true }
+  boolean equals(b) { true }
+  int compareTo(b) { 0 }
+}
+'''
+      enableInspections inspection
+    }
+  }
 
   void testSimpleUnaryExpression() {
-    assertValid(/a.bitwiseNegate()/, /~a/)
-    assertValid(/a.negative()/, /-a/)
-    assertValid(/a.positive()/, /+a/)
-//    assertValid(/a.call()/, /a()/)
-    assertValid(/a.next()/, /++a/)
-    assertValid(/a.previous()/, /--a/)
+    doTest "a.bitwiseNegate()", "~a"
+    doTest "a.negative()", "-a"
+    doTest "a.positive()", "+a"
+    doTest "a.next()", "++a"
+    doTest "a.previous()", "--a"
   }
 
   void testNegatableUnaryExpression() {
-    assertValid(/a.asBoolean()/, /!!a/)
-    assertValid(/!a.asBoolean()/, /!a/)
-
-    assertValid(/if (${_}a.asBoolean()${_});/, /a/)
-    assertValid(/if (${_}!a.asBoolean()${_});/, /!a/)
-  }
-
-  void testComplexNegatableUnaryExpression() {
-    assertValid(/if (${_}'a'.intern().asBoolean()${_});/, /'a'.intern()/)
+    doTest "a.asBoolean()", "!!a"
+    doTest "!a.asBoolean()", "!a"
+    doTest "if (a.asBoo<caret>lean());", "if (a);"
+    doTest "if (!a.asBo<caret>olean());", "if (!a);"
+    doTest "if ('a'.intern().as<caret>Boolean());", "if ('a'.intern());"
   }
 
   void testNegatedOption() {
     inspection.useDoubleNegation = false
-
-    assertValid(/a.asBoolean()/)
-    assertValid(/!a.asBoolean()/, /!a/)
-
-    assertValid(/if (${_}a.asBoolean()${_});/, /a/)
-    assertValid(/if (${_}!a.asBoolean()${_});/, /!a/)
-  }
-
-  void testComplexNegatedOption() {
-    inspection.useDoubleNegation = false
-
-    assertValid(/if (${_}'a'.intern().asBoolean()${_});/, /'a'.intern()/)
+    doTest "a.asBoolean()"
+    doTest "!a.asBoolean()", "!a"
+    doTest "if (a.as<caret>Boolean());", "if (a);"
+    doTest "if (!a.asB<caret>oolean());", "if (!a);"
+    doTest "if ('a'.intern().asBool<caret>ean());", "if ('a'.intern());"
   }
 
   void testSimpleBinaryExpression() {
-    assertValid(/a.minus(b)/, /a - b/)
-    assertValid(/a.plus(b)/, /a + b/)
-    assertValid(/a.power(b)/, /a**b/)
-    assertValid(/a.div(b)/, $/a / b/$)
-    assertValid(/a.mod(b)/, /a % b/)
-    assertValid(/a.or(b)/, /a | b/)
-    assertValid(/a.and(b)/, /a & b/)
-    assertValid(/a.xor(b)/, /a ^ b/)
-    assertValid(/a.leftShift(b)/, /a << b/)
-    assertValid(/a.rightShift(b)/, /a >> b/)
-    assertValid(/a.rightShiftUnsigned(b)/, /a >>> b/)
-
-    assertValid(/a.asType(String)/, /a as String/)
-    assertValid(/a.multiply(b)/, /a * b/)
-
-    assertValid(/(a.toString() as Operators).minus(b.hashCode())/, /(a.toString() as Operators) - b.hashCode()/)
-
-    assertValid(/!${_}a.asType(String)${_}/, /(a as String)/)
-
-    assertValid(/${_}a.xor((a.b+1) == b)${_} == a/, /(a ^ ((a.b + 1) == b))/)
+    [
+      "a.plus(b)"              : "a + b",
+      "a.minus(b)"             : "a - b",
+      "a.multiply(b)"          : "a * b",
+      "a.div(b)"               : "a / b",
+      "a.power(b)"             : "a**b",
+      "a.mod(b)"               : "a % b",
+      "a.or(b)"                : "a | b",
+      "a.and(b)"               : "a & b",
+      "a.xor(b)"               : "a ^ b",
+      "a.leftShift(b)"         : "a << b",
+      "a.rightShift(b)"        : "a >> b",
+      "a.rightShiftUnsigned(b)": "a >>> b",
+      "a.asType(String)"       : "a as String",
+      "!a.asType(String)"      : "!(a as String)",
+    ].each {
+      doTest it.key, it.value
+    }
   }
 
   void testComplexBinaryExpression() {
-    assertValid(/b.isCase(a)/, /a in b/)
-    assertValid(/if (${_}[1, 2, 3].isCase(2-1)${_});/, /(2 - 1) in [1, 2, 3]/)
-    assertValid(/def x = ${_}"1".plus(1)${_}/, /"1" + 1/)
-    assertValid(/("1" + 1).plus(1)/, /("1" + 1) + 1/)
-    assertValid(/!a.toString().asBoolean()/, /!a.toString()/)
+    [
+      "(a.toString() as Operators).minus(b.hashCode())": "(a.toString() as Operators) - b.hashCode()",
+      "b.isCase(a)"                                    : "a in b",
+      "if ([1, 2, 3].is<caret>Case(2-1));"             : "if ((2 - 1) in [1, 2, 3]);",
+      'def x = "1".p<caret>lus(1)'                     : 'def x = "1" + 1',
+      '("1" + 1).plus(1)'                              : '("1" + 1) + 1',
+      '!a.toString().asBoolean()'                      : '!a.toString()',
+      "a.xor((a.b + 1) == b) == a"                     : "(a ^ ((a.b + 1) == b)) == a",
+    ].each {
+      doTest it.key, it.value
+    }
   }
 
   void testNegatableBinaryExpression() {
-    assertValid(/a.equals(b)/, /a == b/)
-    assertValid(/!a.equals(b)/, /a != b/)
+    doTest "a.equals(b)", "a == b"
+    doTest "!a.equals(b)", "a != b"
   }
 
   void testComplexNegatableBinaryExpression() {
-    assertValid(/!(1.toString().replace('1', '2')+"").equals(2.toString())/, /(1.toString().replace('1', '2') + "") != 2.toString()/)
+    doTest(/!(1.toString().replace('1', '2')+"").equals(2.toString())/, /(1.toString().replace('1', '2') + "") != 2.toString()/)
   }
 
   void testCompareTo() {
-    assertValid(/a.compareTo(b)/, /a <=> b/)
-    assertValid(/a.compareTo(b) < 0/, /a < b/)
-    assertValid(/a.compareTo(b) <= 0/, /a <= b/)
-    assertValid(/a.compareTo(b) == 0/, /a == b/)
-    assertValid(/a.compareTo(b) != 0/, /a != b/)
-    assertValid(/a.compareTo(b) >= 0/, /a >= b/)
-    assertValid(/a.compareTo(b) > 0/, /a > b/)
-
-    assertValid(/if (${_}(2-1).compareTo(3) > 0${_});/, /(2 - 1) > 3/)
+    doTest "a.compareTo(b)", "a <=> b"
+    doTest "a.compareTo(b) < 0", "a < b"
+    doTest "a.compareTo(b) <= 0", "a <= b"
+    doTest "a.compareTo(b) == 0", "a == b"
+    doTest "a.compareTo(b) != 0", "a != b"
+    doTest "a.compareTo(b) >= 0", "a >= b"
+    doTest "a.compareTo(b) > 0", "a > b"
+    doTest "if ((2-1).compa<caret>reTo(3) > 0);", /if ((2 - 1) > 3);/
   }
 
   void testCompareToOption() {
     inspection.shouldChangeCompareToEqualityToEquals = false
-    assertValid(/a.compareTo(b) == 0/)
-    assertValid(/a.compareTo(b) != 0/)
+    doTest "a.compareTo(b) == 0"
+    doTest "a.compareTo(b) != 0"
   }
 
   void testGetAndPut() {
-    assertValid(/a.getAt(b)/, /a[b]/)
-    assertValid(/a.putAt(b, 'c')/, /a[b] = 'c'/)
-
-    assertValid(/a.putAt(b, 'c'*2)/, /a[b] = ('c' * 2)/)
-  }
-
-  final _ = '/*placeholder*/'
-  @Language('Groovy') final DECLARATIONS = '''
-    @SuppressWarnings("GrMethodMayBeStatic")
-    class Operators {
-      def bitwiseNegate() { null }
-      def negative() { null }
-      def positive() { null }
-      def call() { null }
-      def next() { null }
-      def previous() { null }
-      def plus(b) { null }
-      def minus(b) { null }
-      def multiply(b) { null }
-      def power(b) { null }
-      def div(b) { null }
-      def mod(b) { null }
-      def or(b) { null }
-      def and(b) { null }
-      def xor(b) { null }
-      def leftShift(b) { null }
-      def rightShift(b) { null }
-      def rightShiftUnsigned(b) { null }
-      def asType(b) { null }
-      def getAt(b) { null }
-      def putAt(b, c) { null }
-
-      boolean asBoolean() { true }
-      boolean isCase(b) { true }
-      boolean equals(b) { true }
-      int compareTo(b) { 0 }
+    doTest "a.getAt(b)", "a[b]"
+    doTest "a.putAt(b, 'c')", "a[b] = 'c'"
+    doTest "a.putAt(b, 'c'*2)", "a[b] = ('c' * 2)"
+  }
+
+  final String DECLARATIONS = 'def (Operators a, Operators b) = [null, null]\n'
+
+  private void doTest(String before, String after = null) {
+    fixture.with {
+      configureByText '_.groovy', "$DECLARATIONS$before"
+      moveCaret()
+      def intentions = filterAvailableIntentions('Replace ')
+      if (after) {
+        assert intentions
+        launchAction intentions.first()
+        checkResult "$DECLARATIONS$after"
+      }
+      else {
+        assert !intentions
+      }
     }
-
-    def (Operators a, Operators b) = [null, null]
-  '''
-
-  private void assertValid(@Language('Groovy') String text, @Language('Groovy') String methodReplacement) {
-    def (prefix, method, suffix) = getMessage(text)
-    configure("${prefix}${CARET_MARKER}${method}${suffix}")
-    myFixture.launchAction(myFixture.findSingleIntention("Change '"))
-    myFixture.checkResult("${DECLARATIONS}  ${prefix}${methodReplacement}${suffix}")
   }
 
-  private void assertValid(@Language('Groovy') String text) {
-    configure(text)
-    assertEmpty myFixture.getAvailableIntentions()
-  }
+  private void moveCaret() {
+    def statement = (fixture.file as GroovyFile).statements.last()
+    GrMethodCall call = null
 
-  private configure(@Language('Groovy') String text) {
-    myFixture.enableInspections(inspection)
-    myFixture.configureByText(GROOVY_FILE_TYPE, "${DECLARATIONS}  ${text}")
-  }
+    if (statement instanceof GrMethodCall) {
+      call = statement as GrMethodCall
+    }
+    else if (statement instanceof GrUnaryExpression) {
+      def operand = (statement as GrUnaryExpression).operand
+      if (operand instanceof GrMethodCall) {
+        call = operand as GrMethodCall
+      }
+    }
+    else if (statement instanceof GrBinaryExpression) {
+      def left = (statement as GrBinaryExpression).leftOperand
+      if (left instanceof GrMethodCall) {
+        call = left as GrMethodCall
+      }
+    }
 
-  private getMessage(String text) {
-    def _ = Pattern.quote(_)
-    def (__, prefix, method, suffix) = (text =~ /^(?:(.*?)${_})?(.+?)(?:${_}(.*?))?$/)[0]
-    [prefix ?: '', method, suffix ?: '']
+    if (call) {
+      def invoked = call.invokedExpression as GrReferenceExpression
+      fixture.editor.caretModel.moveToOffset(invoked.referenceNameElement.textRange.startOffset)
+    }
   }
 }
\ No newline at end of file