Fix another polyadic expression victim ("Join Concatenated String Literals" intention)

plugins/IntentionPowerPak/src/com/siyeh/ipp/concatenation/JoinConcatenatedStringLiteralsIntention.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2009 Dave Griffith, Bas Leijdekkers
+ * Copyright 2003-2012 Dave Griffith, Bas Leijdekkers
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,14 +16,10 @@
 package com.siyeh.ipp.concatenation;
 
 import com.intellij.openapi.util.text.StringUtil;
-import com.intellij.psi.PsiBinaryExpression;
-import com.intellij.psi.PsiElement;
-import com.intellij.psi.PsiExpression;
-import com.intellij.psi.PsiJavaToken;
+import com.intellij.psi.*;
 import com.intellij.util.IncorrectOperationException;
 import com.siyeh.ipp.base.Intention;
 import com.siyeh.ipp.base.PsiElementPredicate;
-import com.siyeh.ipp.psiutils.ExpressionUtils;
 import org.jetbrains.annotations.NotNull;
 
 public class JoinConcatenatedStringLiteralsIntention extends Intention {
@@ -35,65 +31,44 @@ public class JoinConcatenatedStringLiteralsIntention extends Intention {
   }
 
   @Override
-  public void processIntention(PsiElement element)
-    throws IncorrectOperationException {
-    final PsiBinaryExpression binaryExpression =
-      (PsiBinaryExpression)element.getParent();
-    assert binaryExpression != null;
-    final PsiBinaryExpression copy =
-      (PsiBinaryExpression)binaryExpression.copy();
-    final PsiExpression lhs = copy.getLOperand();
-    String newExpression = "";
-    if (lhs instanceof PsiBinaryExpression) {
-      final PsiBinaryExpression lhsBinaryExpression =
-        (PsiBinaryExpression)lhs;
-      newExpression += getLeftSideText(lhsBinaryExpression);
-      final PsiExpression rightSide = lhsBinaryExpression.getROperand();
-      assert rightSide != null;
-      lhs.replace(rightSide);
+  public void processIntention(PsiElement element) throws IncorrectOperationException {
+    if (element instanceof PsiWhiteSpace) {
+      element = element.getPrevSibling();
     }
-    newExpression += '"' + computeConstantStringExpression(copy) + '"';
-    replaceExpression(newExpression, binaryExpression);
-  }
-
-  /**
-   * handles the specified expression as if it was part of a string expression
-   * (even if it's of another type) and computes a constant string expression
-   * from it.
-   */
-  private static String computeConstantStringExpression(
-    PsiBinaryExpression expression) {
-    final PsiExpression lhs = expression.getLOperand();
-    final Object lhsConstant =
-      ExpressionUtils.computeConstantExpression(lhs);
-    final String lhsText;
-    if (lhsConstant == null) {
-      lhsText = "";
+    if (!(element instanceof PsiJavaToken)) {
+      return;
     }
-    else {
-      lhsText = lhsConstant.toString();
+    final PsiJavaToken token = (PsiJavaToken)element;
+    final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)element.getParent();
+    final PsiExpression[] operands = polyadicExpression.getOperands();
+    StringBuilder newExpression = new StringBuilder();
+    PsiExpression previous = null;
+    for (PsiExpression operand : operands) {
+      if (newExpression.length() != 0 && previous != null) {
+        newExpression.append('+');
+      }
+      final PsiJavaToken currentToken = polyadicExpression.getTokenBeforeOperand(operand);
+      if (token == currentToken) {
+        final PsiLiteralExpression literal1 = (PsiLiteralExpression)previous;
+        assert literal1 != null;
+        final PsiLiteralExpression literal2 = (PsiLiteralExpression)operand;
+        final Object value1 = literal1.getValue();
+        final Object value2 = literal2.getValue();
+        assert value1 != null && value2 != null;
+        final String text1 = StringUtil.escapeStringCharacters(value1.toString());
+        final String text2 = StringUtil.escapeStringCharacters(value2.toString());
+        newExpression.append('"').append(text1).append(text2).append('"');
+        previous = null;
+      } else {
+        if (previous != null) {
+          newExpression.append(previous.getText());
+        }
+        previous = operand;
+      }
     }
-    String result;
-    if (lhsText.length() == 0) {
-      result = "";
+    if (previous != null) {
+      newExpression.append('+').append(previous.getText());
     }
-    else {
-      result = lhsText;
-    }
-    final PsiExpression rhs = expression.getROperand();
-    final Object rhsConstant =
-      ExpressionUtils.computeConstantExpression(rhs);
-    if (rhsConstant != null) {
-      result += rhsConstant.toString();
-    }
-    result = StringUtil.escapeStringCharacters(result);
-    return result;
-  }
-
-  private static String getLeftSideText(
-    PsiBinaryExpression binaryExpression) {
-    final PsiExpression lhs = binaryExpression.getLOperand();
-    final PsiJavaToken sign = binaryExpression.getOperationSign();
-    return lhs.getText() + sign.getText();
+    replaceExpression(newExpression.toString(), polyadicExpression);
   }
 }

plugins/IntentionPowerPak/src/com/siyeh/ipp/concatenation/StringConcatPredicate.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers
+ * Copyright 2003-2012 Dave Griffith, Bas Leijdekkers
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,63 +17,49 @@ package com.siyeh.ipp.concatenation;
 
 import com.intellij.psi.*;
 import com.intellij.psi.tree.IElementType;
-import com.intellij.psi.util.PsiUtil;
 import com.siyeh.ipp.base.PsiElementPredicate;
-import org.jetbrains.annotations.Nullable;
 
 class StringConcatPredicate implements PsiElementPredicate {
 
   public boolean satisfiedBy(PsiElement element) {
-    if (element instanceof PsiJavaToken) {
-      final PsiJavaToken token = (PsiJavaToken)element;
-      final IElementType tokenType = token.getTokenType();
-      if (!tokenType.equals(JavaTokenType.PLUS)) {
-        return false;
-      }
+    if (element instanceof PsiWhiteSpace) {
+      element = element.getPrevSibling();
     }
-    else if (!(element instanceof PsiWhiteSpace)) {
+    if (!(element instanceof PsiJavaToken)) {
       return false;
     }
-    final PsiElement parent = element.getParent();
-    if (!(parent instanceof PsiBinaryExpression)) {
-      return false;
-    }
-    final PsiBinaryExpression binaryExpression =
-      (PsiBinaryExpression)parent;
-    final IElementType tokenType = binaryExpression.getOperationTokenType();
+    final PsiJavaToken token = (PsiJavaToken)element;
+    final IElementType tokenType = token.getTokenType();
     if (!tokenType.equals(JavaTokenType.PLUS)) {
       return false;
     }
-    final PsiType type = binaryExpression.getType();
-    if (type == null || !type.equalsToText("java.lang.String")) {
+    final PsiElement parent = element.getParent();
+    if (!(parent instanceof PsiPolyadicExpression)) {
       return false;
     }
-    final PsiExpression rhs = binaryExpression.getROperand();
-    if (rhs == null) {
+    final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)parent;
+    final PsiType type = polyadicExpression.getType();
+    if (type == null || !type.equalsToText(CommonClassNames.JAVA_LANG_STRING)) {
       return false;
     }
-    final PsiExpression lhs = binaryExpression.getLOperand();
-    final PsiExpression rightMostExpression = getRightmostExpression(lhs);
-    if (rightMostExpression instanceof PsiPrefixExpression) {
-      final PsiType prefixExpressionType = rightMostExpression.getType();
-      if (prefixExpressionType == null ||
-          prefixExpressionType.equalsToText("java.lang.String")) {
-        return false;
+    final PsiExpression[] operands = polyadicExpression.getOperands();
+    PsiExpression previous = null;
+    boolean stringTypeSeen = false;
+    for (int i = 0, length = operands.length; i < length; i++) {
+      final PsiExpression operand = operands[i];
+      final PsiType operandType = operand.getType();
+      final PsiJavaToken currentToken = polyadicExpression.getTokenBeforeOperand(operand);
+      if (token == currentToken) {
+        if (!(previous instanceof PsiLiteralExpression) || !(operand instanceof PsiLiteralExpression)) {
+          return false;
+        }
+        return stringTypeSeen || (i == 1 && operandType != null && operandType.equalsToText(CommonClassNames.JAVA_LANG_STRING));
+      }
+      previous = operand;
+      if (!stringTypeSeen) {
+        stringTypeSeen = operandType != null && operandType.equalsToText(CommonClassNames.JAVA_LANG_STRING);
       }
     }
-    return PsiUtil.isConstantExpression(rhs) &&
-           PsiUtil.isConstantExpression(rightMostExpression);
-  }
-
-  @Nullable
-  private static PsiExpression getRightmostExpression(
-    PsiExpression expression) {
-    if (expression instanceof PsiBinaryExpression) {
-      final PsiBinaryExpression binaryExpression =
-        (PsiBinaryExpression)expression;
-      final PsiExpression rhs = binaryExpression.getROperand();
-      return getRightmostExpression(rhs);
-    }
-    return expression;
+    return false;
   }
 }

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/NonString.java

@@ -0,0 +1,3 @@
+class NonString {
+  String t = 9 <caret>+ "asdf" + "lkjh";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/NonString2.java

@@ -0,0 +1,3 @@
+class NonString2 {
+  String t = "asdf" + "lkjh" + 7 <caret>+ 9 + "z";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/NonString2_after.java

@@ -0,0 +1,3 @@
+class NonString2 {
+  String t = "asdf" + "lkjh" + "79" + "z";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/NonString_after.java

@@ -0,0 +1,3 @@
+class NonString {
+  String t = "9asdf" + "lkjh";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/NotAvailable.java

@@ -0,0 +1,3 @@
+class NotAvailable {
+  String t = 8 +<caret> 9 + "asdf" + "lkjh";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/Polyadic.java

@@ -0,0 +1,3 @@
+class Polyadic {
+  String s = "a" + "b" +<caret> "c" + "d";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/Polyadic_after.java

@@ -0,0 +1,3 @@
+class Polyadic {
+  String s = "a" + "bc" + "d";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/Simple.java

@@ -0,0 +1,3 @@
+class Simple {
+    String a = "asdf" +<caret> "b";
+}

plugins/IntentionPowerPak/test/com/siyeh/ipp/concatenation/join_concat/Simple_after.java

@@ -0,0 +1,3 @@
+class Simple {
+    String a = "asdfb";
+}

plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/concatenation/JoinConcatenatedStringLiteralsIntentionTest.java

@@ -0,0 +1,23 @@
+package com.siyeh.ipp.concatenation;
+
+import com.siyeh.IntentionPowerPackBundle;
+import com.siyeh.ipp.IPPTestCase;
+
+public class JoinConcatenatedStringLiteralsIntentionTest extends IPPTestCase {
+    public void testSimple() { doTest(); }
+    public void testPolyadic() { doTest(); }
+    public void testNonString() { doTest(); }
+    public void testNonString2() { doTest(); }
+    public void testNotAvailable() { assertIntentionNotAvailable(); }
+
+    @Override
+    protected String getIntentionName() {
+        return IntentionPowerPackBundle.message(
+                "join.concatenated.string.literals.intention.name");
+    }
+
+    @Override
+    protected String getRelativePath() {
+        return "concatenation/join_concat";
+    }
+}