diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/StringRepeatCanBeUsedInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/StringRepeatCanBeUsedInspection.java index 897eef2dc6eb..027cfc0005a9 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/StringRepeatCanBeUsedInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/StringRepeatCanBeUsedInspection.java @@ -14,6 +14,7 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.text.StringUtil; import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.*; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.util.PsiLiteralUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; @@ -24,12 +25,13 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import static com.intellij.codeInspection.options.OptPane.checkbox; -import static com.intellij.codeInspection.options.OptPane.pane; +import static com.intellij.codeInspection.options.OptPane.*; +import static com.intellij.psi.CommonClassNames.*; import static com.intellij.util.ObjectUtils.tryCast; public final class StringRepeatCanBeUsedInspection extends AbstractBaseJavaLocalInspectionTool { - private static final CallMatcher APPEND = CallMatcher.instanceCall("java.lang.AbstractStringBuilder", "append").parameterCount(1); + private static final CallMatcher APPEND = CallMatcher.instanceCall(JAVA_LANG_ABSTRACT_STRING_BUILDER, "append").parameterCount(1); + private static final CallMatcher REPEAT = CallMatcher.instanceCall(JAVA_LANG_STRING, "repeat").parameterCount(1); public boolean ADD_MATH_MAX = true; @@ -63,15 +65,46 @@ public final class StringRepeatCanBeUsedInspection extends AbstractBaseJavaLocal if (type == null) return; String builderClassName = type.getPresentableText(); holder.registerProblem(statement.getFirstChild(), - JavaBundle.message("inspection.message.can.be.replaced.with.builder.repeat", - builderClassName + ".repeat()"), - new AbstractStringBuilderRepeatCanBeUsedFix(ADD_MATH_MAX, builderClassName)); + messageForCanBeReplacedWithBuilderRepeat(builderClassName), + new ConvertForLoopToStringBuilderRepeatFix(ADD_MATH_MAX, builderClassName)); } else { holder.registerProblem(statement.getFirstChild(), JavaBundle.message("inspection.message.can.be.replaced.with.string.repeat"), - new StringRepeatCanBeUsedFix(ADD_MATH_MAX)); + new ConvertForLoopToStringRepeatFix(ADD_MATH_MAX)); } } + + /** + * Detects AbstractStringBuilder.append() call with String.repeat() as an argument, for example + *
+       *   StringBuilder sb = new StringBuilder();
+       *   sb.append("*".repeat(10))
+       * 
+ */ + @Override + public void visitMethodCallExpression(@NotNull PsiMethodCallExpression call) { + if (!languageLevel.isAtLeast(LanguageLevel.JDK_21)) return; + if (!APPEND.test(call)) return; + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return; + PsiExpression[] args = call.getArgumentList().getExpressions(); + if (args.length != 1) return; + PsiMethodCallExpression repeatCall = tryCast(PsiUtil.skipParenthesizedExprDown(args[0]), PsiMethodCallExpression.class); + if (repeatCall == null) return; + if (!REPEAT.test(repeatCall)) return; + if (ErrorUtil.containsDeepError(call)) return; + PsiType type = qualifier.getType(); + if (type == null) return; + String builderClassName = type.getPresentableText(); + PsiElement reference = call.getMethodExpression().getReferenceNameElement(); + if (reference == null) return; + holder.registerProblem(reference, messageForCanBeReplacedWithBuilderRepeat(builderClassName), + new ConvertStringRepeatToStringBuilderRepeatFix(builderClassName)); + } + + private static @Nls @NotNull String messageForCanBeReplacedWithBuilderRepeat(String builderClassName) { + return JavaBundle.message("inspection.message.can.be.replaced.with.builder.repeat", builderClassName + ".repeat()"); + } }; } @@ -83,10 +116,24 @@ public final class StringRepeatCanBeUsedInspection extends AbstractBaseJavaLocal return call; } - private static final class AbstractStringBuilderRepeatCanBeUsedFix extends RepeatCanBeUsedFix { + /** + * Replaces a for loop containing a call to AbstractStringBuilder.append(s), for example + *
+   * StringBuilder sb = new StringBuilder();
+   * for(int i=0; i<100; i++) {
+   *   sb.append(" ");
+   * }
+   * 
+ * with single call to AbstractStringBuilder.repeat() + *
+   * StringBuilder sb = new StringBuilder();
+   * sb.repeat(" ", 100);
+   * 
+ */ + private static final class ConvertForLoopToStringBuilderRepeatFix extends ConvertToRepeatFix { private final String builderClassShortName; - private AbstractStringBuilderRepeatCanBeUsedFix(boolean addMathMax, String builderClassShortName) { + private ConvertForLoopToStringBuilderRepeatFix(boolean addMathMax, String builderClassShortName) { super(addMathMax); this.builderClassShortName = builderClassShortName; } @@ -114,8 +161,22 @@ public final class StringRepeatCanBeUsedInspection extends AbstractBaseJavaLocal } } - private static final class StringRepeatCanBeUsedFix extends RepeatCanBeUsedFix { - private StringRepeatCanBeUsedFix(boolean addMathMax) { + /** + * Replaces a for loop containing a call to AbstractStringBuilder.append(s), for example + *
+   * StringBuilder sb = new StringBuilder();
+   * for(int i=0; i<100; i++) {
+   *   sb.append(" ");
+   * }
+   * 
+ * with single call to AbstractStringBuilder.append() that uses String.repeat() + *
+   * StringBuilder sb = new StringBuilder();
+   * sb.append(" ".repeat(100));
+   * 
+ */ + private static final class ConvertForLoopToStringRepeatFix extends ConvertToRepeatFix { + private ConvertForLoopToStringRepeatFix(boolean addMathMax) { super(addMathMax); } @@ -144,10 +205,10 @@ public final class StringRepeatCanBeUsedInspection extends AbstractBaseJavaLocal } } - private static abstract class RepeatCanBeUsedFix extends PsiUpdateModCommandQuickFix { + private static abstract class ConvertToRepeatFix extends PsiUpdateModCommandQuickFix { protected final boolean myAddMathMax; - private RepeatCanBeUsedFix(boolean addMathMax) { + private ConvertToRepeatFix(boolean addMathMax) { myAddMathMax = addMathMax; } @@ -233,7 +294,66 @@ public final class StringRepeatCanBeUsedInspection extends AbstractBaseJavaLocal if (isStringType && NullabilityUtil.getExpressionNullability(arg, true) == Nullability.NOT_NULL) { return ct.text(arg, ParenthesesUtils.METHOD_CALL_PRECEDENCE); } - return CommonClassNames.JAVA_LANG_STRING + ".valueOf(" + ct.text(arg) + ")"; + return JAVA_LANG_STRING + ".valueOf(" + ct.text(arg) + ")"; + } + } + + /** + * Replaces AbstractStringBuilder.append() containing String.repeat(...) argument + * with AbstractStringBuilder.repeat(), for example + *
+   * StringBuilder sb = new StringBuilder();
+   * sb.append(" ".repeat(100));
+   * 
+ * with call to AbstractStringBuilder.repeat() + *
+   * StringBuilder sb = new StringBuilder();
+   * sb.repeat(" ", 100));
+   * 
+ */ + private static final class ConvertStringRepeatToStringBuilderRepeatFix extends PsiUpdateModCommandQuickFix { + private final String builderClassShortName; + + private ConvertStringRepeatToStringBuilderRepeatFix(String builderClassShortName) { + this.builderClassShortName = builderClassShortName; + } + + @Override + public @Nls(capitalization = Nls.Capitalization.Sentence) @NotNull String getFamilyName() { + return CommonQuickFixBundle.message("fix.replace.with.x", builderClassShortName + ".repeat()"); + } + + @Override + protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) { + PsiMethodCallExpression appendCall = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class); + if (appendCall == null || !APPEND.test(appendCall)) return; + if (ErrorUtil.containsDeepError(appendCall)) return; + + PsiExpression qualifierExpression = appendCall.getMethodExpression().getQualifierExpression(); + if (qualifierExpression == null) return; + + PsiExpression[] args = appendCall.getArgumentList().getExpressions(); + if (args.length != 1) return; + PsiMethodCallExpression repeatCall = tryCast(PsiUtil.skipParenthesizedExprDown(args[0]), PsiMethodCallExpression.class); + if (repeatCall == null) return; + PsiExpression[] repeatArgs = repeatCall.getArgumentList().getExpressions(); + if (repeatArgs.length != 1) return; + PsiExpression count = repeatArgs[0]; + + PsiExpression stringExpression = PsiUtil.skipParenthesizedExprDown(repeatCall.getMethodExpression().getQualifierExpression()); + if (stringExpression == null) return; + CommentTracker ct = new CommentTracker(); + String replacement = + ct.text(qualifierExpression) + ".repeat(" + sanitizedStringExpression(stringExpression, ct) + ", " + ct.text(count) + ")"; + var replaced = ct.replaceAndRestoreComments(appendCall, replacement); + JavaCodeStyleManager.getInstance(project).shortenClassReferences(replaced); + } + + private static String sanitizedStringExpression(PsiExpression stringExpression, CommentTracker ct) { + if (NullabilityUtil.getExpressionNullability(stringExpression, true) == Nullability.NOT_NULL) { + return ct.text(stringExpression); + } + return JAVA_UTIL_OBJECTS + ".requireNonNull(" + ct.text(stringExpression) + ")"; } } } diff --git a/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppend.java b/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppend.java new file mode 100644 index 000000000000..ed80971bc225 --- /dev/null +++ b/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppend.java @@ -0,0 +1,8 @@ +// "Replace with 'StringBuilder.repeat()'" "true" +class Test { + String hundredSpaces() { + StringBuilder sb = new StringBuilder(); + sb.repeat(" ", 100); + return sb.toString(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppendNotNull.java b/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppendNotNull.java new file mode 100644 index 000000000000..7ced5adb1284 --- /dev/null +++ b/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppendNotNull.java @@ -0,0 +1,9 @@ +// "Replace with 'StringBuilder.repeat()'" "true" +class Test { + String hundredSpaces() { + String s = "*"; + StringBuilder sb = new StringBuilder(); + sb.repeat(s, 10); + return sb.toString(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppendNullable.java b/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppendNullable.java new file mode 100644 index 000000000000..2d137453312d --- /dev/null +++ b/java/java-tests/testData/inspection/stringBuilderRepeat/afterStringRepeatInsideBuilderAppendNullable.java @@ -0,0 +1,10 @@ +import java.util.Objects; + +// "Replace with 'StringBuilder.repeat()'" "true" +class Test { + String hundredSpaces(String s, int i) { + StringBuilder sb = new StringBuilder(); + sb.repeat(Objects.requireNonNull(s), i); + return sb.toString(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppend.java b/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppend.java new file mode 100644 index 000000000000..63aed2cc7aca --- /dev/null +++ b/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppend.java @@ -0,0 +1,8 @@ +// "Replace with 'StringBuilder.repeat()'" "true" +class Test { + String hundredSpaces() { + StringBuilder sb = new StringBuilder(); + sb.append(" ".repeat(100)); + return sb.toString(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppendNotNull.java b/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppendNotNull.java new file mode 100644 index 000000000000..27473a5dd423 --- /dev/null +++ b/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppendNotNull.java @@ -0,0 +1,9 @@ +// "Replace with 'StringBuilder.repeat()'" "true" +class Test { + String hundredSpaces() { + String s = "*"; + StringBuilder sb = new StringBuilder(); + sb.append(s.repeat(10)); + return sb.toString(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppendNullable.java b/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppendNullable.java new file mode 100644 index 000000000000..c3d3d8b0adb1 --- /dev/null +++ b/java/java-tests/testData/inspection/stringBuilderRepeat/beforeStringRepeatInsideBuilderAppendNullable.java @@ -0,0 +1,8 @@ +// "Replace with 'StringBuilder.repeat()'" "true" +class Test { + String hundredSpaces(String s, int i) { + StringBuilder sb = new StringBuilder(); + sb.append(s.repeat(i)); + return sb.toString(); + } +} \ No newline at end of file