[codeInsight] IDEA-219640 Inspection to detect some pointless String.substring

IDEA didn't use to report pointless String.substring invocations for
cases when only one character in the string is extracted. In such cases
it makes more sense to use the String#charAt method because it is
allocation free.

IDEA didn't use to suggest to refactor for the code when String.substring
selects a single character and is followed by the equals method(e.g.
args[0].substring(0, 1).equals("_")) to more readable version e.g.
args[0].charAt(0) == '_'.

This patch adds such refactorings to both of the cases. It also adds
the possiblity to handle expressions like "i+1" and "i+2" to
com.siyeh.ig.psiutils.ExpressionUtils#isDifference.

Signed-off-by: Nikita Eshkeev <nikita.eshkeev@jetbrains.com>

GitOrigin-RevId: 6d179c7c9c605f08b4d4468712f8468fd68045a1

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

@@ -2,6 +2,6 @@
 class Foo {
   public static void main(String[] args) {
     StringBuilder sb = new StringBuilder();
-    sb.append(args[0], 3, 4);
+    sb.append(args[0], 2, 4);
   }
 }

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

@@ -0,0 +1,37 @@
+// "Fix all 'Redundant String operation' problems in file" "true"
+class Foo {
+  public static void main(String[] args) {
+    StringBuilder sb = new StringBuilder();
+
+    int i = Integer.parseInt(args[4]);
+
+    sb.append(args[0].charAt(0));
+    sb.append(args[0].charAt(2));
+    sb.append(args[0].charAt(i));
+
+    sb.append(args[0].charAt(i + 2));
+    sb.append(args[0].charAt(2 + i));
+    sb.append(args[0].charAt(i + 2));
+    sb.append(args[0].charAt(2 + i));
+
+    sb.append(args[0].substring(2 + i, 4 + i));
+    sb.append(args[0].substring(i + 2, i + 4));
+
+    String s1 = "xxx" + args[0].substring(3, 5);
+    String s2 = "xxx" + args[0].charAt(3);
+    String s3 = args[0].charAt(2) + "xxx";
+
+    boolean value = args[0].charAt(4) == '_';
+
+    if(args[0].charAt(4) == '_') { }
+    if(args[0].charAt(4) != '_') { }
+    if(args[0].charAt(4) == '_') { }
+    if(args[0].charAt(4) != '_') { }
+
+    System.out.print(args[0].charAt(2));
+    System.out.println(args[0].charAt(2));
+
+    System.out.print(args[0].substring(2, 4));
+    System.out.println(args[0].substring(2, 4));
+  }
+}

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

@@ -2,6 +2,6 @@
 class Foo {
   public static void main(String[] args) {
     StringBuilder sb = new StringBuilder();
-    sb.append(args[0].subs<caret>tring(3, 4));
+    sb.append(args[0].subs<caret>tring(2, 4));
   }
 }

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

@@ -0,0 +1,37 @@
+// "Fix all 'Redundant String operation' problems in file" "true"
+class Foo {
+  public static void main(String[] args) {
+    StringBuilder sb = new StringBuilder();
+
+    int i = Integer.parseInt(args[4]);
+
+    sb.append(args[0].sub<caret>string(0, 1));
+    sb.append(args[0].substring(2, 3));
+    sb.append(args[0].substring(i, i + 1));
+
+    sb.append(args[0].substring(i + 2, i + 3));
+    sb.append(args[0].substring(2 + i, i + 3));
+    sb.append(args[0].substring(i + 2, 3 + i));
+    sb.append(args[0].substring(2 + i, 3 + i));
+
+    sb.append(args[0].substring(2 + i, 4 + i));
+    sb.append(args[0].substring(i + 2, i + 4));
+
+    String s1 = "xxx" + args[0].substring(3, 5);
+    String s2 = "xxx" + args[0].substring(3, 4);
+    String s3 = args[0].substring(2, 3) + "xxx";
+
+    boolean value = args[0].substring(4, 5).equals("_");
+
+    if(args[0].substring(4, 5).equals("_")) { }
+    if(!args[0].substring(4, 5).equals("_")) { }
+    if(!!args[0].substring(4, 5).equals("_")) { }
+    if(!!!!!args[0].substring(4, 5).equals("_")) { }
+
+    System.out.print(args[0].substring(2, 3));
+    System.out.println(args[0].substring(2, 3));
+
+    System.out.print(args[0].substring(2, 4));
+    System.out.println(args[0].substring(2, 4));
+  }
+}

plugins/InspectionGadgets/InspectionGadgetsAnalysis/resources/messages/InspectionGadgetsBundle.properties

@@ -609,7 +609,6 @@ continue.statement.with.label.display.name='continue' statement with label
 class.loader.instantiation.display.name=ClassLoader instantiation
 return.from.finally.block.display.name='return' inside 'finally' block
 unnecessary.boxing.display.name=Unnecessary boxing
-annotation.naming.convention.display.name=Annotation naming convention
 annotation.naming.convention.element.description=Annotation
 checked.exception.class.display.name=Checked exception class
 switch.statement.with.confusing.declaration.display.name=Local variable used and declared in different 'switch' branches
@@ -618,7 +617,6 @@ manual.array.copy.display.name=Manual array copy
 manual.array.to.collection.copy.display.name=Manual array to collection copy
 long.literals.ending.with.lowercase.l.display.name='long' literal ending with 'l' instead of 'L'
 overly.complex.arithmetic.expression.display.name=Overly complex arithmetic expression
-junit.abstract.test.class.naming.convention.display.name=JUnit abstract test class naming convention
 junit.abstract.test.class.naming.convention.element.description=Abstract test
 unnecessary.parentheses.display.name=Unnecessary parentheses
 test.case.in.product.code.display.name=JUnit TestCase in product source
@@ -2039,7 +2037,6 @@ equals.with.itself.display.name='equals()' called on itself
 equals.with.itself.problem.descriptor=<code>#ref()</code> called on itself
 junit4.method.naming.convention.display.name=JUnit 4+ test method naming convention
 junit4.method.naming.convention.element.description=JUnit 4+ test
-junit3.method.naming.convention.display.name=JUnit 3 test method naming convention
 junit3.method.naming.convention.element.description=JUnit 3 test
 introduce.holder.class.quickfix=Introduce holder class
 double.brace.initialization.display.name=Double brace initialization
@@ -2370,6 +2367,7 @@ create.missing.switch.branches.fix.family.name=Create enum switch branches
 unnecessary.fully.qualified.name.fix.family.name=Replace fully qualified name
 return.of.collection.field.fix.family.name=Make return collection 'unmodifiable'
 remove.redundant.substring.fix.family.name=Remove redundant 'substring()' call
+remove.redundant.substring.to.char.at.fix.family.name=Replace 'substring()' to 'charAt()'
 remove.redundant.substring.fix.text=Use ''{0}'' and remove redundant ''substring()'' call
 make.class.final.fix.family.name=Make class final
 side.effects.method.ref.to.lambda.fix.family.name={0} (side effects)

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java

@@ -1039,7 +1039,7 @@ public class ExpressionUtils {
     return expression;
   }
 
-  @Contract(value = "null -> null")
+  @Contract("null -> null")
   @Nullable
   public static PsiLocalVariable resolveLocalVariable(@Nullable PsiExpression expression) {
     expression = ParenthesesUtils.stripParentheses(expression);
@@ -1171,6 +1171,12 @@ public class ExpressionUtils {
         return true;
       }
     }
+    if (to instanceof PsiBinaryExpression && from instanceof PsiBinaryExpression) {
+      final BinaryOpExtractor extractor = new BinaryOpExtractor((PsiBinaryExpression)from, (PsiBinaryExpression)to);
+      final BinaryOpExtractor.Result result = extractor.extract();
+      from = result.myFrom;
+      to = result.myTo;
+    }
     Integer fromConstant = tryCast(computeConstantExpression(from), Integer.class);
     if (fromConstant == null) return false;
     Integer toConstant = tryCast(computeConstantExpression(to), Integer.class);
@@ -1180,6 +1186,56 @@ public class ExpressionUtils {
     return diffConstant == toConstant - fromConstant;
   }
 
+  private static final class BinaryOpExtractor {
+    @NotNull static final EquivalenceChecker eq = EquivalenceChecker.getCanonicalPsiEquivalence();
+    @NotNull private final PsiBinaryExpression myFrom;
+    @NotNull private final PsiBinaryExpression myTo;
+
+    private BinaryOpExtractor(@NotNull final PsiBinaryExpression from,
+                              @NotNull final PsiBinaryExpression to) {
+      myFrom = from;
+      myTo = to;
+    }
+
+    @NotNull
+    private Result extract() {
+      final IElementType opTo = myTo.getOperationTokenType();
+      final IElementType opFrom = myFrom.getOperationTokenType();
+
+      if ((opTo == JavaTokenType.PLUS || opTo == JavaTokenType.MINUS) && opTo == opFrom) {
+        @NotNull final PsiExpression toLeft = myTo.getLOperand();
+        @NotNull final PsiExpression toRight = myTo.getROperand() != null ? myTo.getROperand() : myTo;
+
+        @NotNull final PsiExpression fromLeft = myFrom.getLOperand();
+        @NotNull final PsiExpression fromRight = myFrom.getROperand() != null ? myFrom.getROperand() : myFrom;
+
+        if (eq.expressionsAreEquivalent(fromLeft, toLeft)) {
+          return new Result(fromRight, toRight);
+        }
+        else if (eq.expressionsAreEquivalent(fromLeft, toRight)) {
+          return new Result(fromRight, toLeft);
+        }
+        else if (eq.expressionsAreEquivalent(fromRight, toLeft)) {
+          return new Result(fromLeft, toRight);
+        }
+        else if (eq.expressionsAreEquivalent(fromRight, toRight)) {
+          return new Result(fromLeft, toLeft);
+        }
+      }
+      return new Result(myFrom, myTo);
+    }
+
+    private static final class Result {
+      @NotNull private final PsiExpression myFrom;
+      @NotNull private final PsiExpression myTo;
+
+      private Result(@NotNull final PsiExpression from, @NotNull final PsiExpression to) {
+        myFrom = from;
+        myTo = to;
+      }
+    }
+  }
+
   /**
    * Returns an expression which represents an array element with given index if array is known to be never modified
    * after initialization.

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

@@ -6,7 +6,9 @@ import com.intellij.codeInsight.daemon.impl.quickfix.DeleteElementFix;
 import com.intellij.codeInspection.*;
 import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.NlsContexts;
 import com.intellij.openapi.util.TextRange;
+import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.psi.*;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
@@ -16,10 +18,7 @@ import com.siyeh.ig.PsiReplacementUtil;
 import com.siyeh.ig.callMatcher.CallMapper;
 import com.siyeh.ig.callMatcher.CallMatcher;
 import com.siyeh.ig.psiutils.*;
-import org.jetbrains.annotations.Nls;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
-import org.jetbrains.annotations.PropertyKey;
+import org.jetbrains.annotations.*;
 
 import javax.swing.*;
 import java.util.Collections;
@@ -165,13 +164,33 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
         if (lengthMatches) {
           PsiElement anchor = qualifierCall.getMethodExpression().getReferenceNameElement();
           if (anchor != null) {
+            final String equalToValue = tryCast(ExpressionUtils.computeConstantExpression(equalTo), String.class);
+            if (StringUtil.length(equalToValue) == 1) {
+              final PsiElement outermostEqualsExpr = getOutermostEqualsExpr(call);
+              final String eqSign = getEqualsSing(outermostEqualsExpr);
+
+              final String converted = String.format("%s.charAt(%s) %s '%s'",
+                                                     receiver.getText(),
+                                                     args[0].getText(),
+                                                     eqSign,
+                                                     equalToValue
+              );
+
+              final SubstringToCharAtQuickFix fix = new SubstringToCharAtQuickFix(outermostEqualsExpr.getText(),
+                                                                                  converted);
+              return myManager.createProblemDescriptor(outermostEqualsExpr,
+                                                       CommonQuickFixBundle
+                                                         .message("fix.replace.x.with.y", outermostEqualsExpr.getText(), converted),
+                                                       fix,
+                                                       ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly);
+            }
             return myManager.createProblemDescriptor(anchor, (TextRange)null,
                                                      InspectionGadgetsBundle.message("inspection.redundant.string.call.message"),
                                                      ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly,
                                                      new RemoveRedundantSubstringFix("startsWith"));
           }
         }
-      } 
+      }
       if (STRING_SUBSTRING_ONE_ARG.test(qualifierCall)) {
         PsiExpression equalTo = call.getArgumentList().getExpressions()[0];
         PsiExpression from = qualifierCall.getArgumentList().getExpressions()[0];
@@ -190,6 +209,35 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
       return null;
     }
 
+    @NotNull
+    @Contract(value = "_ -> !null", pure = true)
+    private static String getEqualsSing(@NotNull final PsiElement ancestor) {
+      final int negations = StringUtil.countChars(ancestor.getText(), '!');
+      final int i = negations & 1;
+
+      return i == 0 ? "==" : "!=";
+    }
+
+    @Contract(value = "null -> null; !null -> !null", pure = true)
+    @Nullable
+    private static PsiElement getOutermostEqualsExpr(@Nullable PsiElement ancestor) {
+      if (ancestor == null) return null;
+      PsiElement parent;
+      while (true) {
+        parent = PsiUtil.skipParenthesizedExprUp(ancestor.getParent());
+        if (parent instanceof PsiPrefixExpression) {
+          final PsiPrefixExpression parentPrefixExpr = (PsiPrefixExpression)parent;
+          if (JavaTokenType.EXCL != parentPrefixExpr.getOperationTokenType()) break;
+
+          ancestor = parent;
+        }
+        else {
+          break;
+        }
+      }
+      return ancestor;
+    }
+
     private boolean lengthMatches(PsiExpression equalTo, PsiExpression from, PsiExpression to) {
       String str = tryCast(ExpressionUtils.computeConstantExpression(equalTo), String.class);
       PsiElementFactory factory = JavaPsiFacade.getElementFactory(myHolder.getProject());
@@ -292,6 +340,24 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
                                                  InspectionGadgetsBundle.message("inspection.redundant.string.call.message"),
                                                  fix, ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly);
       }
+
+      boolean betterWithCharAt = isBetterWithCharAt(call);
+      if (betterWithCharAt) {
+        final PsiElement substring = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement());
+
+        final String converted = String.format("%s.charAt(%s)",
+                                               Objects.requireNonNull(stringExpression).getText(),
+                                               args[0].getText());
+
+        final SubstringToCharAtQuickFix fix = new SubstringToCharAtQuickFix(call.getText(), converted);
+
+        final TextRange textRange = new TextRange(substring.getStartOffsetInParent(),
+                                                  substring.getStartOffsetInParent() + substring.getTextLength());
+        return myManager.createProblemDescriptor(call, textRange,
+                                                 CommonQuickFixBundle.message("fix.replace.x.with.y", call.getText(), converted),
+                                                 ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly,
+                                                 fix);
+      }
       PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent());
       if (parent instanceof PsiExpressionList && ((PsiExpressionList)parent).getExpressionCount() == 1) {
         PsiMethodCallExpression parentCall = tryCast(parent.getParent(), PsiMethodCallExpression.class);
@@ -307,6 +373,22 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
       return null;
     }
 
+    private static boolean isBetterWithCharAt(@NotNull final PsiMethodCallExpression call) {
+      final PsiExpression[] args = call.getArgumentList().getExpressions();
+      final PsiElementFactory factory = JavaPsiFacade.getElementFactory(call.getProject());
+
+      final boolean diffByOne = ExpressionUtils.isDifference(args[0],
+                                                             args[1],
+                                                             factory.createExpressionFromText("1", null));
+      if (!diffByOne) return false;
+
+      final PsiReferenceExpression methodExpression = call.getMethodExpression();
+      final PsiExpression qualifier = ExpressionUtils.getEffectiveQualifier(methodExpression);
+      final boolean throwable = TypeUtils.expressionHasTypeOrSubtype(qualifier, "java.lang.Throwable");
+
+      return !ExpressionUtils.isConversionToStringNecessary(call, throwable);
+    }
+
     private static boolean isLengthOf(PsiExpression stringLengthCandidate, PsiExpression stringExpression) {
       PsiMethodCallExpression argCall = tryCast(PsiUtil.skipParenthesizedExprDown(stringLengthCandidate), PsiMethodCallExpression.class);
       return STRING_LENGTH.test(argCall) &&
@@ -332,6 +414,38 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
       }
       return call.getTextRange();
     }
+
+    private static class SubstringToCharAtQuickFix implements LocalQuickFix {
+      @NotNull private final String myText;
+      @NotNull private final String myConverted;
+
+      SubstringToCharAtQuickFix(@NotNull final String text,
+                                @NotNull final String converted) {
+        myText = text;
+        myConverted = converted;
+      }
+
+      @Override
+      public @NlsContexts.ListItem @NotNull String getName() {
+        return CommonQuickFixBundle.message("fix.replace.x.with.y", myText, myConverted);
+      }
+
+      @Override
+      public @NlsContexts.ListItem @NotNull String getFamilyName() {
+        return InspectionGadgetsBundle.message("remove.redundant.substring.to.char.at.fix.family.name");
+      }
+
+      @Override
+      public void applyFix(@NotNull final Project project, @NotNull final ProblemDescriptor descriptor) {
+        final PsiElement element = descriptor.getPsiElement();
+        if (element == null) return;
+
+        final PsiExpression text = JavaPsiFacade.getElementFactory(project).createExpressionFromText(myConverted, null);
+
+        final CommentTracker ct = new CommentTracker();
+        ct.replaceAndRestoreComments(element, text);
+      }
+    }
   }
 
   private static class RemoveRedundantSubstringFix implements LocalQuickFix {