RedundantStringOperation: append("") (IDEA-180178), intern()

ConstantStringInternInspection removed

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringBuilderAppendCleanUp.java

@@ -0,0 +1,8 @@
+// "Fix all 'Redundant String operation' problems in file" "true"
+class Foo {
+  public static void main(String[] args) {
+    StringBuilder sb = new StringBuilder();
+      sb = new StringBuilder();
+      sb.append("x");
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringBuilderAppendEmpty.java

@@ -0,0 +1,7 @@
+// "Remove redundant 'append' call" "true"
+class Foo {
+  public static void main(String[] args) {
+    StringBuilder sb = new StringBuilder();
+    sb.append("foo").append("bar");
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringIntern.java

@@ -0,0 +1,4 @@
+// "Remove redundant 'intern' call" "true"
+class Foo {
+  private static final String x = ("Hello "+"World"+'!');
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringToString.java

@@ -1,4 +1,4 @@
-// "Remove 'toString'" "true"
+// "Remove redundant 'toString' call" "true"
 class Foo {
   public static void main(String[] args) {
       /*valuable comment!!!*/

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeObjectToString.java

@@ -1,4 +1,4 @@
-// "Remove 'toString'" "false"
+// "Remove redundant 'toString' call" "false"
 class Foo {
   public static void main(String[] args) {
     Object[] a = args;

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringBuilderAppendCleanUp.java

@@ -0,0 +1,9 @@
+// "Fix all 'Redundant String operation' problems in file" "true"
+class Foo {
+  public static void main(String[] args) {
+    StringBuilder sb = new StringBuilder();
+    sb.appe<caret>nd("");
+    (sb = new StringBuilder()).append("");
+    (sb.append("x")).append("");
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringBuilderAppendEmpty.java

@@ -0,0 +1,7 @@
+// "Remove redundant 'append' call" "true"
+class Foo {
+  public static void main(String[] args) {
+    StringBuilder sb = new StringBuilder();
+    sb.append("foo").ap<caret>pend(("")).append("bar");
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringIntern.java

@@ -0,0 +1,4 @@
+// "Remove redundant 'intern' call" "true"
+class Foo {
+  private static final String x = ("Hello "+"World"+'!').inte<caret>rn();
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringInternNonConst.java

@@ -0,0 +1,4 @@
+// "Remove redundant 'intern' call" "false"
+class Foo {
+  private static final String x = ("Hello "+"World".trim()+'!').inte<caret>rn();
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringToString.java

@@ -1,4 +1,4 @@
-// "Remove 'toString'" "true"
+// "Remove redundant 'toString' call" "true"
 class Foo {
   public static void main(String[] args) {
     String s = args[0].toString<caret>(/*valuable comment!!!*/);

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml

@@ -1929,9 +1929,6 @@
                      bundle="com.siyeh.InspectionGadgetsBundle" key="collections.must.have.initial.capacity.display.name"
                      groupBundle="messages.InspectionsBundle" groupKey="group.names.performance.issues" enabledByDefault="false"
                      level="WARNING" implementationClass="com.siyeh.ig.performance.CollectionsMustHaveInitialCapacityInspection"/>
-    <localInspection groupPath="Java" language="JAVA" shortName="ConstantStringIntern" bundle="com.siyeh.InspectionGadgetsBundle" key="constant.string.intern.display.name"
-                     groupBundle="messages.InspectionsBundle" groupKey="group.names.performance.issues" enabledByDefault="true"
-                     level="WARNING" implementationClass="com.siyeh.ig.performance.ConstantStringInternInspection"/>
     <localInspection groupPath="Java" language="JAVA" shortName="DynamicRegexReplaceableByCompiledPattern" bundle="com.siyeh.InspectionGadgetsBundle"
                      key="dynamic.regex.replaceable.by.compiled.pattern.display.name" groupBundle="messages.InspectionsBundle"
                      groupKey="group.names.performance.issues" enabledByDefault="false" level="WARNING"

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.java

@@ -34,7 +34,7 @@ public class InspectionGadgetsBundle {
   }
 
   private static Reference<ResourceBundle> ourBundle;
-  @NonNls private static final String BUNDLE = "com.siyeh.InspectionGadgetsBundle";
+  @NonNls public static final String BUNDLE = "com.siyeh.InspectionGadgetsBundle";
 
   private InspectionGadgetsBundle() {
   }

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties

@@ -1515,9 +1515,6 @@ access.to.static.field.locked.on.instance.problem.descriptor=Access to static fi
 make.method.ctr.quickfix=Make method constructor
 replace.all.dot.display.name=Call to String.replaceAll(".", ...)
 replace.all.dot.problem.descriptor=Call to <code>String.#ref(".", ...)</code> #loc
-constant.string.intern.display.name=Call to 'intern()' on String constant
-constant.string.intern.problem.descriptor=<code>.#ref()</code> on compile-time constant is unnecessary #loc
-constant.string.intern.quickfix=Remove 'intern()' call
 class.extends.utility.class.display.name=Class extends utility class
 class.extends.utility.class.problem.descriptor=Class <code>#ref</code> extends utility class ''{0}'' #loc
 class.extends.utility.class.ignore.utility.class.option=Ignore if overriding class is a utility class
@@ -2230,6 +2227,7 @@ return.of.field.with.mutable.type.problem.descriptor=Return of {0} field <code>{
 ignore.private.methods.option=Ignore assignments in and returns from private methods
 
 inspection.redundant.string.operation.display.name=Redundant String operation
-inspection.redundant.string.remove.fix.name=Remove ''{0}''
+inspection.redundant.string.remove.fix.name=Remove redundant ''{0}'' call
 inspection.redundant.string.replace.with.qualifier.fix.family.name=Replace with qualifier
 inspection.redundant.string.message=<code>#ref</code> is redundant #loc
+inspection.redundant.string.intern.on.constant.message=<code>.#ref()</code> on compile-time constant is unnecessary #loc

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ConstantStringInternInspection.java

@@ -1,131 +0,0 @@
-/*
- * Copyright 2006-2010 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.
- * 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.
- */
-package com.siyeh.ig.performance;
-
-import com.intellij.codeInspection.ProblemDescriptor;
-import com.intellij.openapi.project.Project;
-import com.intellij.psi.*;
-import com.intellij.psi.util.PsiUtil;
-import com.intellij.util.IncorrectOperationException;
-import com.siyeh.InspectionGadgetsBundle;
-import com.siyeh.ig.BaseInspection;
-import com.siyeh.ig.BaseInspectionVisitor;
-import com.siyeh.ig.InspectionGadgetsFix;
-import com.siyeh.ig.PsiReplacementUtil;
-import org.jetbrains.annotations.NonNls;
-import org.jetbrains.annotations.NotNull;
-
-public class ConstantStringInternInspection extends BaseInspection {
-
-  @Override
-  @NotNull
-  public String getDisplayName() {
-    return InspectionGadgetsBundle.message(
-      "constant.string.intern.display.name");
-  }
-
-  @Override
-  @NotNull
-  protected String buildErrorString(Object... infos) {
-    return InspectionGadgetsBundle.message(
-      "constant.string.intern.problem.descriptor");
-  }
-
-  @Override
-  public InspectionGadgetsFix buildFix(Object... infos) {
-    return new ConstantStringInternFix();
-  }
-
-  @Override
-  public BaseInspectionVisitor buildVisitor() {
-    return new ConstantStringInternVisitor();
-  }
-
-  @Override
-  public boolean isEnabledByDefault() {
-    return true;
-  }
-
-  private static class ConstantStringInternFix extends InspectionGadgetsFix {
-
-    @Override
-    @NotNull
-    public String getFamilyName() {
-      return InspectionGadgetsBundle.message(
-        "constant.string.intern.quickfix");
-    }
-
-    @Override
-    public void doFix(Project project, ProblemDescriptor descriptor)
-      throws IncorrectOperationException {
-      final PsiElement element = descriptor.getPsiElement();
-      final PsiElement parent = element.getParent();
-      final PsiMethodCallExpression call =
-        (PsiMethodCallExpression)parent.getParent();
-      final PsiReferenceExpression expression =
-        call.getMethodExpression();
-      final PsiExpression qualifier = expression.getQualifierExpression();
-      if (qualifier == null) {
-        return;
-      }
-      final String qualifierText = qualifier.getText();
-      PsiReplacementUtil.replaceExpression(call, qualifierText);
-    }
-  }
-
-  private static class ConstantStringInternVisitor
-    extends BaseInspectionVisitor {
-
-    @Override
-    public void visitMethodCallExpression(
-      @NotNull PsiMethodCallExpression expression) {
-      super.visitMethodCallExpression(expression);
-      final PsiReferenceExpression methodExpression =
-        expression.getMethodExpression();
-      @NonNls final String methodName =
-        methodExpression.getReferenceName();
-      if (!"intern".equals(methodName)) {
-        return;
-      }
-      final PsiExpressionList argList = expression.getArgumentList();
-      final PsiExpression[] args = argList.getExpressions();
-      if (args.length != 0) {
-        return;
-      }
-      final PsiExpression qualifier =
-        methodExpression.getQualifierExpression();
-      if (qualifier == null) {
-        return;
-      }
-      if (!PsiUtil.isConstantExpression(qualifier)) {
-        return;
-      }
-      final PsiMethod method = expression.resolveMethod();
-      if (method == null) {
-        return;
-      }
-      final PsiClass aClass = method.getContainingClass();
-      if (aClass == null) {
-        return;
-      }
-      final String className = aClass.getQualifiedName();
-      if (!CommonClassNames.JAVA_LANG_STRING.equals(className)) {
-        return;
-      }
-      registerMethodCallError(expression);
-    }
-  }
-}

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/redundancy/RedundantStringOperationInspection.java

@@ -10,21 +10,28 @@ import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.ObjectUtils;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.callMatcher.CallMatcher;
-import com.siyeh.ig.psiutils.CommentTracker;
-import com.siyeh.ig.psiutils.EquivalenceChecker;
-import com.siyeh.ig.psiutils.ExpressionUtils;
+import com.siyeh.ig.psiutils.*;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.PropertyKey;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+import static com.intellij.psi.CommonClassNames.JAVA_LANG_STRING;
+import static com.siyeh.InspectionGadgetsBundle.BUNDLE;
 
 public class RedundantStringOperationInspection extends AbstractBaseJavaLocalInspectionTool implements CleanupLocalInspectionTool {
-  private static final CallMatcher STRING_TO_STRING =
-    CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "toString").parameterCount(0);
-  private static final CallMatcher STRING_LENGTH =
-    CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "length").parameterCount(0);
-  private static final CallMatcher STRING_SUBSTRING =
-    CallMatcher.anyOf(
-      CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "substring").parameterTypes("int"),
-      CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "substring").parameterTypes("int", "int"));
+  private static final CallMatcher STRING_TO_STRING = CallMatcher.instanceCall(JAVA_LANG_STRING, "toString").parameterCount(0);
+  private static final CallMatcher STRING_INTERN = CallMatcher.instanceCall(JAVA_LANG_STRING, "intern").parameterCount(0);
+  private static final CallMatcher STRING_LENGTH = CallMatcher.instanceCall(JAVA_LANG_STRING, "length").parameterCount(0);
+  private static final CallMatcher STRING_SUBSTRING = CallMatcher.anyOf(
+      CallMatcher.instanceCall(JAVA_LANG_STRING, "substring").parameterTypes("int"),
+      CallMatcher.instanceCall(JAVA_LANG_STRING, "substring").parameterTypes("int", "int"));
+  private static final CallMatcher STRING_BUILDER_APPEND =
+    CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_ABSTRACT_STRING_BUILDER, "append")
+      .parameterTypes(JAVA_LANG_STRING);
 
   @NotNull
   @Override
@@ -35,11 +42,20 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
         PsiExpression qualifier = call.getMethodExpression().getQualifierExpression();
         if (qualifier != null) {
           if (STRING_TO_STRING.test(call)) {
-            registerProblem(call, "toString");
+            registerProblem(call, "inspection.redundant.string.message");
           }
           else if (STRING_SUBSTRING.test(call)) {
             processSubstring(call);
           }
+          else if (STRING_BUILDER_APPEND.test(call)) {
+            PsiExpression arg = call.getArgumentList().getExpressions()[0];
+            if (ExpressionUtils.isLiteral(PsiUtil.skipParenthesizedExprDown(arg), "")) {
+              registerProblem(call, "inspection.redundant.string.message");
+            }
+          }
+          else if (STRING_INTERN.test(call) && PsiUtil.isConstantExpression(qualifier)) {
+            registerProblem(call, "inspection.redundant.string.intern.on.constant.message");
+          }
         }
       }
 
@@ -56,12 +72,13 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
             return;
           }
         }
-        registerProblem(call, "substring");
+        registerProblem(call, "inspection.redundant.string.message");
       }
 
-      private void registerProblem(PsiMethodCallExpression call, String toRemove) {
-        holder.registerProblem(call, InspectionGadgetsBundle.message("inspection.redundant.string.message"),
-                               ProblemHighlightType.LIKE_UNUSED_SYMBOL, getRange(call), new ReplaceWithQualifierFix(toRemove));
+      private void registerProblem(PsiMethodCallExpression call, @NotNull @PropertyKey(resourceBundle = BUNDLE) String key) {
+        String name = call.getMethodExpression().getReferenceName();
+        holder.registerProblem(call, InspectionGadgetsBundle.message(key),
+                               ProblemHighlightType.LIKE_UNUSED_SYMBOL, getRange(call), new ReplaceWithQualifierFix(name));
       }
     };
   }
@@ -104,7 +121,22 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
       if (call == null) return;
       PsiExpression qualifier = ExpressionUtils.getQualifierOrThis(call.getMethodExpression());
       CommentTracker ct = new CommentTracker();
-      ct.replaceAndRestoreComments(call, ct.markUnchanged(qualifier));
+      PsiExpression result = (PsiExpression)ct.replaceAndRestoreComments(call, ct.markUnchanged(qualifier));
+      if (result.getParent() instanceof PsiExpressionStatement) {
+        extractSideEffects(result, (PsiExpressionStatement)result.getParent());
+      }
+    }
+
+    private static void extractSideEffects(PsiExpression result, PsiStatement statement) {
+      List<PsiExpression> sideEffects = SideEffectChecker.extractSideEffectExpressions(result);
+      if (Collections.singletonList(result).equals(sideEffects)) return;
+
+      PsiStatement[] statements = StatementExtractor.generateStatements(sideEffects, result);
+      if (statements.length > 0) {
+        PsiStatement lastAdded = BlockUtils.addBefore(statement, statements);
+        statement = Objects.requireNonNull(PsiTreeUtil.getNextSiblingOfType(lastAdded, PsiStatement.class));
+      }
+      statement.delete();
     }
   }
 }

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/redundancy/RedundantStringOperationMerger.java

@@ -16,7 +16,7 @@ public class RedundantStringOperationMerger extends InspectionElementsMerger {
   public String[] getSourceToolNames() {
     return new String[] {
       "StringToString", "RedundantStringToString",
-      "SubstringZero"
+      "SubstringZero", "ConstantStringIntern"
     };
   }
 }

plugins/InspectionGadgets/src/inspectionDescriptions/ConstantStringIntern.html

@@ -1,10 +0,0 @@
-<html>
-<body>
-Reports on any call to <b>String.intern()</b> on a compile-time constant
-string. Per the Java Language Specification, compile-time constant strings are automatically interned, making the call
-to <b>String.intern()</b> redundant.
-<!-- tooltip end -->
-<p>
-
-</body>
-</html>

plugins/InspectionGadgets/test/com/siyeh/igtest/performance/ConstantStringInternInspection.java

@@ -1,5 +0,0 @@
-package com.siyeh.igtest.performance;
-
-public class ConstantStringInternInspection {
-    private String foo = "foo".intern();
-}