IG: better messages and fixed highlighting for "Redundant 'String' operation" inspection

GitOrigin-RevId: f328b47c3c64b6f078f89fa04f0353ec34d57f24
This commit is contained in:
Bas Leijdekkers
2022-08-29 17:45:32 +02:00
committed by intellij-monorepo-bot
parent e7c55493a1
commit 3d5589bde2
12 changed files with 67 additions and 77 deletions

View File

@@ -23,6 +23,6 @@ class Main {
String s4 = new String((out4.toByteArray()));
String s5 = new String(foo().toByteArray());
String s6 = new String((foo().toByteArray()<caret>));
String s6 = new String<caret>((foo().toByteArray()));
}
}

View File

@@ -27,6 +27,6 @@ class Main {
String s4 = new String((out4.toByteArray()), (charset));
String s5 = new String(foo().toByteArray(), charset);
String s6 = new String((foo().toByteArray()), (charset<caret>));
String s6 = new <caret>String((foo().toByteArray()), (charset));
}
}

View File

@@ -26,6 +26,6 @@ class Main {
String s4 = new String((out4.toByteArray()), (csn));
String s5 = new String(foo().toByteArray(), csn);
String s6 = new String((foo().toByteArray()), (csn<caret>));
String s6 = new <caret>String((foo().toByteArray()), (csn));
}
}

View File

@@ -2,7 +2,7 @@
class Test {
String foo1(char c) {
return new String(new char[] { c })<caret>;
return new <caret>String(new char[] { c });
}
String foo2(Character character) {

View File

@@ -1,7 +1,7 @@
// "Fix all 'Redundant 'String' operation' problems in file" "true"
class Foo {
void test(String s) {
String s1 = s.substring(s.leng<caret>th());
String s1 = s.substring<caret>(s.length());
String s2 = s.substring(s1.length());
}
}

View File

@@ -2,7 +2,7 @@
class Test {
String foo1(char c) {
return String.valueOf(new char[]<caret> { c });
return String.valueOf(new <caret>char[] { c });
}
String foo2(Character character) {

View File

@@ -5,7 +5,7 @@ import java.nio.charset.Charset;
class Main {
void string() {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
String s = <warning descr="Inefficient conversion from ByteArrayOutputStream">new String(baos.toByteArray())</warning>;
String s = new <warning descr="Inefficient conversion from ByteArrayOutputStream">String</warning>(baos.toByteArray());
}
void charset() {
@@ -19,7 +19,7 @@ class Main {
String csn = "ISO-8859-1";
ByteArrayOutputStream baos = new ByteArrayOutputStream();
String s = <warning descr="Inefficient conversion from ByteArrayOutputStream">new String(baos.toByteArray(), csn)</warning>;
String s = new <warning descr="Inefficient conversion from ByteArrayOutputStream">String</warning>(baos.toByteArray(), csn);
}

View File

@@ -0,0 +1,17 @@
import java.io.ByteArrayOutputStream;
class Substring {
void m(String message) {
boolean underline = message.<warning descr="'substring()' call can be replaced with 'charAt()'">substring</warning>(1, 2).equals("_");
String s1 = message.<warning descr="Call to 'substring()' is redundant">substring</warning>(message.length());
StringBuilder sb = new StringBuilder();
sb.append(message.<warning descr="'substring()' call can be replaced with 'charAt()'">substring</warning>(2, 3));
if (!!!!!message.<warning descr="'substring()' call can be replaced with 'charAt()'">substring</warning>(4, 5).equals("_")) { }
String s = new <warning descr="'new String()' is redundant">String</warning>("foo");
String.valueOf(<warning descr="'new char[]' is redundant">new char[] </warning>{ 'c' });
new <warning descr="Can be replaced with 'String.valueOf()'">String</warning>(new char[] { 'c' });
String s2 = new <warning descr="'new String()' is redundant">String</warning>();
}
}

View File

@@ -28,6 +28,7 @@ public class RedundantStringOperationInspectionTest extends LightJavaInspectionT
public void testBAOStoString() {doTest();}
public void testNewStringNewChar() {doTest();}
public void testStringValueOfNewChar() {doTest();}
public void testRedundantStringOperation() {doTest();}
@Override
protected String getBasePath() {

View File

@@ -2174,11 +2174,12 @@ inspection.redundant.string.length.argument.message=Unnecessary string length ar
inspection.redundant.zero.argument.message=Unnecessary zero argument
inspection.redundant.string.remove.argument.fix.name=Remove argument
inspection.redundant.string.intern.on.constant.message=Call to <code>#ref()</code> on compile-time constant is unnecessary #loc
inspection.redundant.string.constructor.message=<code>#ref</code> is redundant #loc
inspection.redundant.string.constructor.message=<code>new #ref()</code> is redundant #loc
inspection.redundant.string.new.array.message=<code>#ref</code> is redundant #loc
inspection.redundant.string.replace.with.arg.fix.name=Replace with argument
inspection.redundant.string.replace.with.empty.fix.name=Replace with empty string
inspection.redundant.string.option.do.not.report.string.constructors=Do not report String constructors
inspection.x.call.can.be.replaced.with.y=''{0}'' call can be replaced with ''{1}''
inspection.x.call.can.be.replaced.with.y=<code>#ref()</code> call can be replaced with ''{0}()''
inspection.type.may.be.weakened.display.name=Type may be weakened
inspection.type.may.be.weakened.problem.descriptor=Type of variable <code>#ref</code> may be weakened to {0} #loc

View File

@@ -2650,7 +2650,7 @@
<localInspection groupPath="Java" language="JAVA" shortName="UnnecessaryStringEscape" bundle="messages.InspectionGadgetsBundle"
key="unnecessary.string.escape.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.verbose.or.redundant.code.constructs"
enabledByDefault="true" level="WARNING" cleanupTool="true" implementationClass="com.siyeh.ig.redundancy.UnnecessaryStringEscapeInspection"/>
<localInspection groupPath="Java" language="JAVA" shortName="StringOperationCanBeSimplified" bundle="messages.InspectionGadgetsBundle" editorAttributes="NOT_USED_ELEMENT_ATTRIBUTES"
<localInspection groupPath="Java" language="JAVA" shortName="StringOperationCanBeSimplified" bundle="messages.InspectionGadgetsBundle"
key="inspection.redundant.string.operation.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.verbose.or.redundant.code.constructs"
enabledByDefault="true" level="WARNING" cleanupTool="true" implementationClass="com.siyeh.ig.redundancy.RedundantStringOperationInspection"/>
<localInspection groupPath="Java" language="JAVA" shortName="OnlyOneElementUsed" bundle="messages.InspectionGadgetsBundle"

View File

@@ -10,7 +10,6 @@ import com.intellij.codeInspection.util.IntentionFamilyName;
import com.intellij.codeInspection.util.IntentionName;
import com.intellij.java.analysis.JavaAnalysisBundle;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.NlsSafe;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.pom.java.LanguageLevel;
@@ -148,13 +147,15 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
private ProblemDescriptor getStringConstructorProblem(PsiNewExpression expression) {
PsiExpressionList args = expression.getArgumentList();
if (args == null) return null;
final PsiJavaCodeReferenceElement anchor = expression.getClassOrAnonymousClassReference();
if (anchor == null) return null;
if (args.isEmpty()) {
LocalQuickFix[] fixes = {
new StringConstructorFix(true),
new SetInspectionOptionFix(
myInspection, "ignoreStringConstructor",
InspectionGadgetsBundle.message("inspection.redundant.string.option.do.not.report.string.constructors"), true)};
return myManager.createProblemDescriptor(expression, (TextRange)null,
return myManager.createProblemDescriptor(anchor, (TextRange)null,
InspectionGadgetsBundle.message("inspection.redundant.string.constructor.message"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly, fixes);
}
@@ -168,11 +169,11 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
if (qualifier == null) return null;
String newExpressionText = qualifier.getText() + ".toString(" + (params.length == 2 ? params[1].getText() : "") + ")";
final LocalQuickFix fix = new ByteArrayOutputStreamToStringFix(newExpressionText);
return myManager.createProblemDescriptor(expression, (TextRange)null,
return myManager.createProblemDescriptor(anchor, (TextRange)null,
InspectionGadgetsBundle.message("inspection.byte.array.output.stream.to.string.message"),
ProblemHighlightType.WARNING, myIsOnTheFly, fix);
ProblemHighlightType.WARNING, myIsOnTheFly,
new ByteArrayOutputStreamToStringFix(newExpressionText));
}
}
if (args.getExpressionCount() == 1) {
@@ -183,20 +184,19 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
new SetInspectionOptionFix(
myInspection, "ignoreStringConstructor",
InspectionGadgetsBundle.message("inspection.redundant.string.option.do.not.report.string.constructors"), true)};
return myManager.createProblemDescriptor(expression, (TextRange)null,
return myManager.createProblemDescriptor(anchor, (TextRange)null,
JavaAnalysisBundle.message("inspection.can.be.replaced.with.message", "String.valueOf()"),
ProblemHighlightType.WARNING, myIsOnTheFly, fixes);
}
PsiExpression arg = args.getExpressions()[0];
if (TypeUtils.isJavaLangString(arg.getType()) &&
(PsiUtil.isLanguageLevel7OrHigher(expression) || !STRING_SUBSTRING.matches(arg))) {
TextRange range = new TextRange(0, args.getStartOffsetInParent());
LocalQuickFix[] fixes = {
new StringConstructorFix(false),
new SetInspectionOptionFix(
myInspection, "ignoreStringConstructor",
InspectionGadgetsBundle.message("inspection.redundant.string.option.do.not.report.string.constructors"), true)};
return myManager.createProblemDescriptor(expression, range,
return myManager.createProblemDescriptor(anchor, (TextRange)null,
InspectionGadgetsBundle.message("inspection.redundant.string.constructor.message"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly, fixes);
}
@@ -269,7 +269,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
}
return myManager.createProblemDescriptor(anchor, (TextRange)null,
InspectionGadgetsBundle.message("inspection.x.call.can.be.replaced.with.y",
nameMethod, EQUALS_IGNORE_CASE),
EQUALS_IGNORE_CASE),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
new RemoveRedundantChangeCaseFix(nameMethod, type));
}
@@ -310,7 +310,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
if (equalTo instanceof PsiLiteralExpression) {
final Object equalsValue = ((PsiLiteralExpression)equalTo).getValue();
if (equalsValue instanceof String && StringUtil.length((String)equalsValue) == 1) {
return createSubstringToCharAtProblemDescriptor(call);
return createSubstringToCharAtProblemDescriptor(call, anchor);
}
}
RemoveRedundantSubstringFix fix = new RemoveRedundantSubstringFix(isLengthOf(args[1], receiver) ? "endsWith" : "startsWith");
@@ -358,17 +358,13 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
* @return generated instance of {@link ProblemDescriptor}
*/
@NotNull
private ProblemDescriptor createSubstringToCharAtProblemDescriptor(@NotNull final PsiMethodCallExpression call) {
private ProblemDescriptor createSubstringToCharAtProblemDescriptor(@NotNull PsiMethodCallExpression call, @NotNull PsiElement anchor) {
final String converted = SubstringToCharAtQuickFix.getTargetString(call, PsiElement::getText);
assert converted != null : "Message cannot be null";
final PsiElement outermostEqualsExpr = getOutermostEquals(call);
final SubstringToCharAtQuickFix fix = new SubstringToCharAtQuickFix(outermostEqualsExpr.getText(), converted, true);
final @NlsSafe String message =
InspectionGadgetsBundle.message("inspection.x.call.can.be.replaced.with.y", "substring()", "charAt()");
return myManager.createProblemDescriptor(outermostEqualsExpr,
message,
fix,
return myManager.createProblemDescriptor(anchor,
InspectionGadgetsBundle.message("inspection.x.call.can.be.replaced.with.y", "charAt"),
new SubstringToCharAtQuickFix(getOutermostEquals(call).getText(), converted, true),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly);
}
@@ -416,7 +412,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
if (!STRIP.test(qualifierCall)) return null;
PsiElement anchor = qualifierCall.getMethodExpression().getReferenceNameElement();
if (anchor == null) return null;
String message = InspectionGadgetsBundle.message("inspection.x.call.can.be.replaced.with.y", "strip", "isBlank");
String message = InspectionGadgetsBundle.message("inspection.x.call.can.be.replaced.with.y", "isBlank");
return myManager.createProblemDescriptor(anchor, (TextRange)null, message, ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
new StripIsEmptyToIsBlankFix());
}
@@ -500,11 +496,10 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
if (ExpressionUtils.isZero(args[0])) {
return getProblem(call, "inspection.redundant.string.call.message");
} else if (isLengthOf(args[0], stringExpression)) {
SubstringToEmptyStringFix fix = new SubstringToEmptyStringFix();
return myManager.createProblemDescriptor(call,
InspectionGadgetsBundle.message("inspection.redundant.string.constructor.message"),
fix,
PsiElement anchor = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement());
return myManager.createProblemDescriptor(anchor,
InspectionGadgetsBundle.message("inspection.redundant.string.call.message"),
new SubstringToEmptyStringFix(),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly);
}
return null;
@@ -516,28 +511,18 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
}
DeleteElementFix fix =
new DeleteElementFix(args[1], InspectionGadgetsBundle.message("inspection.redundant.string.remove.argument.fix.name"));
final String message = InspectionGadgetsBundle.message("inspection.redundant.string.length.argument.message");
return myManager.createProblemDescriptor(args[1],
message,
InspectionGadgetsBundle.message("inspection.redundant.string.length.argument.message"),
fix, ProblemHighlightType.GENERIC_ERROR_OR_WARNING, 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, false);
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),
if (isBetterWithCharAt(call)) {
final String converted = String.format("%s.charAt(%s)", Objects.requireNonNull(stringExpression).getText(), args[0].getText());
PsiElement anchor = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement());
return myManager.createProblemDescriptor(anchor, (TextRange)null,
InspectionGadgetsBundle.message("inspection.x.call.can.be.replaced.with.y", "charAt"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
fix);
new SubstringToCharAtQuickFix(call.getText(), converted, false));
}
PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent());
if (parent instanceof PsiExpressionList && ((PsiExpressionList)parent).getExpressionCount() == 1) {
@@ -596,13 +581,10 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
private ProblemDescriptor getProblem(PsiMethodCallExpression call, @NotNull @PropertyKey(resourceBundle = BUNDLE) String key) {
PsiElement anchor = call.getMethodExpression().getReferenceNameElement();
if (anchor == null) {
return null;
}
String name = call.getMethodExpression().getReferenceName();
if (anchor == null) return null;
return myManager.createProblemDescriptor(anchor, (TextRange)null, InspectionGadgetsBundle.message(key),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
new RemoveRedundantStringCallFix(name, FixType.REPLACE_WITH_QUALIFIER));
new RemoveRedundantStringCallFix(anchor.getText(), FixType.REPLACE_WITH_QUALIFIER));
}
@Nullable
@@ -611,7 +593,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
if (charArrayCreationArgument == null) return null;
return myManager.createProblemDescriptor(charArrayCreationArgument.newExpression,
new TextRange(0, charArrayCreationArgument.arrayInitializer.getStartOffsetInParent()),
InspectionGadgetsBundle.message("inspection.redundant.string.constructor.message"),
InspectionGadgetsBundle.message("inspection.redundant.string.new.array.message"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
myIsOnTheFly,
new UnwrapArrayInitializerFix(charArrayCreationArgument.initializer.getText()));
@@ -649,7 +631,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
applyEqualityFix(descriptor);
}
else {
PsiMethodCallExpression call = tryCast(descriptor.getPsiElement(), PsiMethodCallExpression.class);
PsiMethodCallExpression call = tryCast(descriptor.getPsiElement().getParent().getParent(), PsiMethodCallExpression.class);
if (call == null) return;
PsiExpression[] args = call.getArgumentList().getExpressions();
if (args.length != 2) return;
@@ -659,26 +641,15 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
}
private static void applyEqualityFix(@NotNull final ProblemDescriptor descriptor) {
final PsiElement element = descriptor.getPsiElement();
if (element == null) return;
final PsiMethodCallExpression call;
if (element instanceof PsiMethodCallExpression) {
call = (PsiMethodCallExpression)element;
}
else {
// Strip PsiPrefixExpression
call = PsiTreeUtil.findChildOfType(element, PsiMethodCallExpression.class);
}
final PsiElement element = descriptor.getPsiElement().getParent().getParent();
final PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class);
if (call == null) return;
final CommentTracker ct = new CommentTracker();
final String convertTo = getTargetString(call, ct::text);
if (convertTo == null) return;
ct.replaceAndRestoreComments(element, convertTo);
ct.replaceAndRestoreComments(getOutermostEquals(call), convertTo);
}
private static @NonNls @Nullable String getTargetString(@NotNull final PsiMethodCallExpression call,
@@ -929,7 +900,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
@Override
public void doFix(Project project, ProblemDescriptor descriptor) {
final PsiNewExpression expression = tryCast(descriptor.getPsiElement(), PsiNewExpression.class);
final PsiNewExpression expression = tryCast(descriptor.getPsiElement().getParent(), PsiNewExpression.class);
if (expression == null) return;
final PsiExpressionList argList = expression.getArgumentList();
if (argList == null) return;
@@ -968,7 +939,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
@Override
public void doFix(Project project, ProblemDescriptor descriptor) {
final PsiNewExpression expression = tryCast(descriptor.getPsiElement(), PsiNewExpression.class);
final PsiNewExpression expression = tryCast(descriptor.getPsiElement().getParent(), PsiNewExpression.class);
if (expression == null) return;
final PsiExpressionList args = expression.getArgumentList();
@@ -1009,7 +980,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
@Override
public void doFix(Project project, ProblemDescriptor descriptor) {
final PsiMethodCallExpression expression = tryCast(descriptor.getPsiElement(), PsiMethodCallExpression.class);
final PsiMethodCallExpression expression = tryCast(descriptor.getPsiElement().getParent().getParent(), PsiMethodCallExpression.class);
if (expression == null) return;
new CommentTracker().replaceAndRestoreComments(expression, "\"\"");
}
@@ -1031,7 +1002,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
@Override
public void doFix(Project project, ProblemDescriptor descriptor) {
final PsiNewExpression expression = tryCast(descriptor.getPsiElement(), PsiNewExpression.class);
final PsiNewExpression expression = tryCast(descriptor.getPsiElement().getParent(), PsiNewExpression.class);
if (expression == null) return;
final CharArrayCreationArgument charArrayCreationArgument = CharArrayCreationArgument.from(expression.getArgumentList());
if (charArrayCreationArgument == null) return;