[codeInsight] IDEA-113640 Provide intention to combine System.out.println(String.format(...)) into System.out.printf

This patch fixes the notes from the code review, it includes:

- Renaming the testData files so their titles are more informative
- Moving `PsiLiteralUtil#append` to `RedundantStringFormatCallInspection` and renaming it to `joinWithNewlineToken` so it does not have to deal with the escape characters' problems
- `RedundantStringFormatCallInspection` only highlights the "`format`" word in `String.format` in order to reduce the warnings area in code visually

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

GitOrigin-RevId: 07b3b3b2d24e500774928d406e274dd4cb20bd5d
This commit is contained in:
Nikita Eshkeev
2020-05-04 17:48:23 +03:00
committed by intellij-monorepo-bot
parent 05dc20b48b
commit fcd7fac054
11 changed files with 53 additions and 55 deletions

View File

@@ -639,44 +639,4 @@ public class PsiLiteralUtil {
return lineBreakIdx + 1;
}
}
/**
* This method appends a suffix to a {@link PsiLiteralExpression} and returns a new
* {@link PsiLiteralExpression} that contains the resulting content leaving the original
* {@link PsiLiteralExpression} unchanged.
*
* @param expression the expression to append a string to
* @param suffix the suffix to add to the expression
* @return a new instance of {@link PsiLiteralExpression} that contains the concatenated
* value of the original {@link PsiLiteralExpression} and the suffix.
*/
@Nullable
@Contract(value = "null, _ -> null; _, null -> param1; !null, _ -> !null", pure = true)
public static PsiLiteralExpression append(@Nullable final PsiLiteralExpression expression, @Nullable final String suffix) {
if (expression == null) return null;
if (StringUtil.isEmpty(suffix)) return expression;
final Object value = expression.getValue();
if (value == null) return expression;
final StringBuilder newExpression = new StringBuilder();
final String leftText = value.toString();
if (expression.isTextBlock()) {
final String indent = StringUtil.repeat(" ", getTextBlockIndent(expression));
newExpression.append("\"\"\"").append('\n').append(indent);
newExpression.append(leftText.replaceAll("\n", "\n" + indent));
newExpression.append(StringUtil.escapeStringCharacters(suffix));
newExpression.append("\"\"\"");
}
else {
newExpression.append('"');
newExpression.append(StringUtil.escapeStringCharacters(leftText));
newExpression.append(StringUtil.escapeStringCharacters(suffix));
newExpression.append('"');
}
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(expression.getProject());
return (PsiLiteralExpression)factory.createExpressionFromText(newExpression.toString(), null);
}
}

View File

@@ -5,6 +5,7 @@ import com.intellij.codeInspection.*;
import com.intellij.codeInspection.util.IntentionFamilyName;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiLiteralUtil;
@@ -72,11 +73,8 @@ public final class RedundantStringFormatCallInspection extends LocalInspectionTo
final PsiExpression formatValue = args.getExpressions()[0];
if (containsNewlineToken(formatValue)) return null;
final PsiElement method = call.getMethodExpression().getReferenceNameElement();
if (method == null) return null;
final TextRange textRange = new TextRange(method.getStartOffsetInParent(),
method.getStartOffsetInParent() + method.getTextLength());
final TextRange textRange = getMethodNameRange(call);
if (textRange == null) return null;
return myManager.createProblemDescriptor(call, textRange,
InspectionGadgetsBundle.message("redundant.call.problem.descriptor"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
@@ -86,8 +84,10 @@ public final class RedundantStringFormatCallInspection extends LocalInspectionTo
@Nullable
private ProblemDescriptor getRedundantStringFormatProblem(@NotNull final PsiMethodCallExpression call) {
if (isStringFormatCallRedundant(call)) {
return myManager.createProblemDescriptor(call, (TextRange)null,
InspectionGadgetsBundle.message("redundant.string.format.call.display.name"),
final TextRange textRange = getMethodNameRange(call);
if (textRange == null) return null;
return myManager.createProblemDescriptor(call, textRange,
InspectionGadgetsBundle.message("redundant.call.problem.descriptor"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
new RemoveRedundantStringFormatFix());
}
@@ -97,12 +97,21 @@ public final class RedundantStringFormatCallInspection extends LocalInspectionTo
if (!PRINTSTREAM_PRINT.test(printlnCall)) return null;
}
return myManager.createProblemDescriptor(call, (TextRange)null,
InspectionGadgetsBundle.message("redundant.string.format.call.display.name"),
final TextRange textRange = getMethodNameRange(call);
if (textRange == null) return null;
return myManager.createProblemDescriptor(call, textRange,
InspectionGadgetsBundle.message("redundant.call.problem.descriptor"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
new StringFormatToPrintfQuickFix(isPrintlnCall));
}
@Nullable
private static TextRange getMethodNameRange(@NotNull final PsiMethodCallExpression call) {
final PsiElement method = call.getMethodExpression().getReferenceNameElement();
if (method == null) return null;
return new TextRange(method.getStartOffsetInParent(), method.getStartOffsetInParent() + method.getTextLength());
}
@Contract(pure = true)
private static boolean isStringFormatCallRedundant(@NotNull final PsiMethodCallExpression call) {
final PsiExpressionList params = call.getArgumentList();
@@ -259,13 +268,13 @@ public final class RedundantStringFormatCallInspection extends LocalInspectionTo
final String newLineToken = "%n";
if (formatArg instanceof PsiLiteralExpression) {
final PsiLiteralExpression replacement = PsiLiteralUtil.append((PsiLiteralExpression)formatArg, newLineToken);
final PsiLiteralExpression replacement = joinWithNewlineToken((PsiLiteralExpression)formatArg);
formatArg.replace(replacement);
}
else if (formatArg instanceof PsiPolyadicExpression){
final PsiElement lastChild = formatArg.getLastChild();
if (lastChild instanceof PsiLiteralExpression) {
final PsiLiteralExpression replacement = PsiLiteralUtil.append((PsiLiteralExpression)lastChild, newLineToken);
final PsiLiteralExpression replacement = joinWithNewlineToken((PsiLiteralExpression)lastChild);
lastChild.replace(replacement);
}
else {
@@ -283,4 +292,33 @@ public final class RedundantStringFormatCallInspection extends LocalInspectionTo
}
}
@Contract(value = "null -> null; !null -> !null", pure = true)
private static PsiLiteralExpression joinWithNewlineToken(@Nullable final PsiLiteralExpression expression) {
if (expression == null) return null;
if (StringUtil.isEmpty("%n")) return expression;
final Object value = expression.getValue();
if (value == null) return expression;
final StringBuilder newExpression = new StringBuilder();
final String leftText = value.toString();
if (expression.isTextBlock()) {
final String indent = StringUtil.repeat(" ", PsiLiteralUtil.getTextBlockIndent(expression));
newExpression.append("\"\"\"").append('\n').append(indent);
newExpression.append(leftText.replaceAll("\n", "\n" + indent));
newExpression.append("%n");
newExpression.append("\"\"\"");
}
else {
newExpression.append('"');
newExpression.append(StringUtil.escapeStringCharacters(leftText));
newExpression.append("%n");
newExpression.append('"');
}
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(expression.getProject());
return (PsiLiteralExpression)factory.createExpressionFromText(newExpression.toString(), null);
}
}

View File

@@ -7,15 +7,15 @@ import static java.lang.String.format;
public class RedundantStringFormatCall {
public static final String A = String.format("%n");
String b = <warning descr="Redundant call to 'String.format()'">String.format("no parameters")</warning>;
String b = String.<warning descr="Redundant call to 'format()'">format</warning>("no parameters");
String c = String.format("asdf%n" +
"asdf%n");
String d = String.format("asdf" + "asdf" + "asdf%n");
String e = <warning descr="Redundant call to 'String.format()'">format("test")</warning>;
String e = <warning descr="Redundant call to 'format()'">format</warning>("test");
void m() {
System.out.println(<warning descr="Redundant call to 'String.format()'">String.format("string contains %%n%n")</warning>); // ok
System.out.println(<warning descr="Redundant call to 'String.format()'">String.format(Locale.ENGLISH, "string contains %%n%n")</warning>);
System.out.println(String.<warning descr="Redundant call to 'format()'">format</warning>("string contains %%n%n")); // ok
System.out.println(String.<warning descr="Redundant call to 'format()'">format</warning>(Locale.ENGLISH, "string contains %%n%n"));
System.out.<warning descr="Redundant call to 'printf()'">printf</warning>("empty battery");
}
}