diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringIndexOfZero.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringIndexOfZero.java new file mode 100644 index 000000000000..0091c270c465 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringIndexOfZero.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + int test(String foo) { + return foo.indexOf("bar"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringLastIndexOf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringLastIndexOf.java new file mode 100644 index 000000000000..4e67dda431e0 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringLastIndexOf.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + int test(String foo) { + return foo.lastIndexOf("bar"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringLastIndexOf2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringLastIndexOf2.java new file mode 100644 index 000000000000..4e67dda431e0 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringLastIndexOf2.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + int test(String foo) { + return foo.lastIndexOf("bar"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringStartsWithZero.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringStartsWithZero.java new file mode 100644 index 000000000000..57652a6a7255 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterStringStartsWithZero.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + boolean test(String foo) { + return foo.startsWith("bar"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringIndexOfZero.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringIndexOfZero.java new file mode 100644 index 000000000000..36e8292ddd1c --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringIndexOfZero.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + int test(String foo) { + return foo.indexOf("bar", 0); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringLastIndexOf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringLastIndexOf.java new file mode 100644 index 000000000000..6cec22e67839 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringLastIndexOf.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + int test(String foo) { + return foo.lastIndexOf("bar", foo.length()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringLastIndexOf2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringLastIndexOf2.java new file mode 100644 index 000000000000..fbd2fb5e7380 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringLastIndexOf2.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + int test(String foo) { + return foo.lastIndexOf("bar", ((foo.length())-1)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringStartsWithZero.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringStartsWithZero.java new file mode 100644 index 000000000000..247b595ba555 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeStringStartsWithZero.java @@ -0,0 +1,6 @@ +// "Remove argument" "true" +class Foo { + boolean test(String foo) { + return foo.startsWith("bar", (0)); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index 814c36076066..c10f0d0be6ea 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -2212,6 +2212,7 @@ inspection.redundant.string.remove.fix.name=Remove redundant ''{0}()'' call inspection.redundant.string.fix.family.name=Remove redundant call inspection.redundant.string.call.message=Call to #ref is redundant #loc inspection.redundant.string.argument.message=Unnecessary empty string argument +inspection.redundant.string.index.argument.message=Unnecessary zero index argument inspection.redundant.string.remove.argument.fix.name=Remove argument inspection.redundant.string.intern.on.constant.message=Call to #ref on compile-time constant is unnecessary #loc diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantStringOperationInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantStringOperationInspection.java index 405a8cf41e52..9dec2e113517 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantStringOperationInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantStringOperationInspection.java @@ -10,10 +10,12 @@ import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.siyeh.InspectionGadgetsBundle; +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 java.util.Collections; @@ -23,6 +25,8 @@ import java.util.Objects; import static com.intellij.psi.CommonClassNames.JAVA_LANG_STRING; import static com.intellij.util.ObjectUtils.tryCast; import static com.siyeh.InspectionGadgetsBundle.BUNDLE; +import static com.siyeh.ig.callMatcher.CallMatcher.anyOf; +import static com.siyeh.ig.callMatcher.CallMatcher.instanceCall; public class RedundantStringOperationInspection extends AbstractBaseJavaLocalInspectionTool implements CleanupLocalInspectionTool { enum FixType { @@ -30,131 +34,178 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns REPLACE_WITH_ARGUMENTS } - 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_TO_STRING = instanceCall(JAVA_LANG_STRING, "toString").parameterCount(0); + private static final CallMatcher STRING_INTERN = instanceCall(JAVA_LANG_STRING, "intern").parameterCount(0); + private static final CallMatcher STRING_LENGTH = instanceCall(JAVA_LANG_STRING, "length").parameterCount(0); + private static final CallMatcher STRING_SUBSTRING = anyOf( + instanceCall(JAVA_LANG_STRING, "substring").parameterTypes("int"), + 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); - private static final CallMatcher PRINTSTREAM_PRINTLN = CallMatcher.instanceCall("java.io.PrintStream", "println") + instanceCall(CommonClassNames.JAVA_LANG_ABSTRACT_STRING_BUILDER, "append").parameterTypes(JAVA_LANG_STRING); + private static final CallMatcher PRINTSTREAM_PRINTLN = instanceCall("java.io.PrintStream", "println") .parameterTypes(JAVA_LANG_STRING); + private static final CallMatcher METHOD_WITH_REDUNDANT_ZERO_AS_SECOND_PARAMETER = + instanceCall(JAVA_LANG_STRING, "indexOf", "startsWith").parameterCount(2); + private static final CallMatcher STRING_LAST_INDEX_OF = instanceCall(JAVA_LANG_STRING, "lastIndexOf").parameterCount(2); @NotNull @Override public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { - return new JavaElementVisitor() { - @Override - public void visitMethodCallExpression(PsiMethodCallExpression call) { - PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); - if (qualifier != null) { - if (STRING_TO_STRING.test(call)) { - registerProblem(call, "inspection.redundant.string.call.message"); - } - else if (STRING_SUBSTRING.test(call)) { - processSubstring(call); - } - else if (STRING_BUILDER_APPEND.test(call)) { - if (getSingleEmptyStringArgument(call) != null) { - registerProblem(call, "inspection.redundant.string.call.message"); - } - } - else if (STRING_INTERN.test(call) && PsiUtil.isConstantExpression(qualifier)) { - registerProblem(call, "inspection.redundant.string.intern.on.constant.message"); - } - else if (PRINTSTREAM_PRINTLN.test(call)) { - checkUnnecessaryEmptyStringArgument(call); - } - } - } - - @Override - public void visitNewExpression(PsiNewExpression expression) { - PsiJavaCodeReferenceElement classRef = expression.getClassReference(); - if (ConstructionUtils.isReferenceTo(classRef, CommonClassNames.JAVA_LANG_STRING_BUILDER, CommonClassNames.JAVA_LANG_STRING_BUFFER)) { - checkUnnecessaryEmptyStringArgument(expression); - } - } - - private void checkUnnecessaryEmptyStringArgument(PsiCall call) { - PsiExpression argument = getSingleEmptyStringArgument(call); - if (argument != null) { - LocalQuickFix fix = - new DeleteElementFix(argument, InspectionGadgetsBundle.message("inspection.redundant.string.remove.argument.fix.name")); - holder.registerProblem(argument, InspectionGadgetsBundle.message("inspection.redundant.string.argument.message"), - ProblemHighlightType.LIKE_UNUSED_SYMBOL, fix); - } - } - - private PsiExpression getSingleEmptyStringArgument(PsiCall call) { - PsiExpressionList argList = call.getArgumentList(); - if (argList == null) return null; - PsiExpression[] args = argList.getExpressions(); - if (args.length != 1) return null; - return ExpressionUtils.isLiteral(PsiUtil.skipParenthesizedExprDown(args[0]), "") ? args[0] : null; - } - - private void processSubstring(PsiMethodCallExpression call) { - PsiExpression[] args = call.getArgumentList().getExpressions(); - PsiExpression stringExpression = call.getMethodExpression().getQualifierExpression(); - switch (args.length) { - case 1: - if(ExpressionUtils.isZero(args[0])) { - registerProblem(call, "inspection.redundant.string.call.message"); - } - break; - case 2: - if (isLengthOf(args[1], stringExpression)) { - if (ExpressionUtils.isZero(args[0])) { - registerProblem(call, "inspection.redundant.string.call.message"); - } else { - DeleteElementFix fix = - new DeleteElementFix(args[1], InspectionGadgetsBundle.message("inspection.redundant.string.remove.argument.fix.name")); - holder.registerProblem(args[1], InspectionGadgetsBundle.message("inspection.redundant.string.call.message"), - ProblemHighlightType.LIKE_UNUSED_SYMBOL, fix); - } - } else { - PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent()); - if (parent instanceof PsiExpressionList && ((PsiExpressionList)parent).getExpressionCount() == 1) { - PsiMethodCallExpression parentCall = tryCast(parent.getParent(), PsiMethodCallExpression.class); - if (STRING_BUILDER_APPEND.test(parentCall)) { - PsiElement nameElement = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()); - holder.registerProblem(nameElement, InspectionGadgetsBundle.message("inspection.redundant.string.call.message"), - ProblemHighlightType.LIKE_UNUSED_SYMBOL, - new RemoveRedundantStringCallFix(nameElement.getText(), FixType.REPLACE_WITH_ARGUMENTS)); - } - } - } - break; - } - } - - private boolean isLengthOf(PsiExpression stringLengthCandidate, PsiExpression stringExpression) { - PsiMethodCallExpression argCall = tryCast(PsiUtil.skipParenthesizedExprDown(stringLengthCandidate), PsiMethodCallExpression.class); - return STRING_LENGTH.test(argCall) && - EquivalenceChecker.getCanonicalPsiEquivalence() - .expressionsAreEquivalent(stringExpression, argCall.getMethodExpression().getQualifierExpression()); - } - - 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 RemoveRedundantStringCallFix(name, FixType.REPLACE_WITH_QUALIFIER)); - } - }; + return new RedundantStringOperationVisitor(holder, isOnTheFly); } - @NotNull - private static TextRange getRange(PsiMethodCallExpression call) { - PsiElement nameElement = call.getMethodExpression().getReferenceNameElement(); - if (nameElement != null) { - TextRange callRange = call.getTextRange(); - return new TextRange(nameElement.getTextRange().getStartOffset(), callRange.getEndOffset()).shiftLeft( - callRange.getStartOffset()); + private static class RedundantStringOperationVisitor extends JavaElementVisitor { + private final CallMapper myProcessors = new CallMapper() + .register(STRING_TO_STRING, call -> getProblem(call, "inspection.redundant.string.call.message")) + .register(STRING_SUBSTRING, this::getSubstringProblem) + .register(STRING_BUILDER_APPEND, this::getAppendProblem) + .register(STRING_INTERN, this::getInternProblem) + .register(PRINTSTREAM_PRINTLN, call -> getRedundantArgumentProblem(getSingleEmptyStringArgument(call))) + .register(METHOD_WITH_REDUNDANT_ZERO_AS_SECOND_PARAMETER, this::getRedundantZeroAsSecondParameterProblem) + .register(STRING_LAST_INDEX_OF, this::getLastIndexOfProblem); + private final InspectionManager myManager; + private final ProblemsHolder myHolder; + private final boolean myIsOnTheFly; + + public RedundantStringOperationVisitor(ProblemsHolder holder, boolean isOnTheFly) { + myHolder = holder; + myIsOnTheFly = isOnTheFly; + myManager = myHolder.getManager(); + } + + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return; + myProcessors.mapAll(call).forEach(myHolder::registerProblem); + } + + @Override + public void visitNewExpression(PsiNewExpression expression) { + PsiJavaCodeReferenceElement classRef = expression.getClassReference(); + if (ConstructionUtils.isReferenceTo(classRef, CommonClassNames.JAVA_LANG_STRING_BUILDER, CommonClassNames.JAVA_LANG_STRING_BUFFER)) { + ProblemDescriptor descriptor = getRedundantArgumentProblem(getSingleEmptyStringArgument(expression)); + if (descriptor == null) return; + myHolder.registerProblem(descriptor); + } + } + + @Nullable + private ProblemDescriptor getAppendProblem(PsiMethodCallExpression call) { + return getSingleEmptyStringArgument(call) != null ? getProblem(call, "inspection.redundant.string.call.message") : null; + } + + @Nullable + private ProblemDescriptor getInternProblem(PsiMethodCallExpression call) { + return PsiUtil.isConstantExpression(call.getMethodExpression().getQualifierExpression()) + ? getProblem(call, "inspection.redundant.string.intern.on.constant.message") + : null; + } + + @Nullable + private ProblemDescriptor getLastIndexOfProblem(PsiMethodCallExpression call) { + PsiExpression secondArg = call.getArgumentList().getExpressions()[1]; + PsiExpression stripped = PsiUtil.skipParenthesizedExprDown(secondArg); + // s.lastIndexOf(..., s.length()) or s.lastIndexOf(..., s.length() - 1) + if (stripped instanceof PsiBinaryExpression) { + PsiBinaryExpression binOp = (PsiBinaryExpression)stripped; + if (binOp.getOperationTokenType().equals(JavaTokenType.MINUS) && + ExpressionUtils.isLiteral(PsiUtil.skipParenthesizedExprDown(binOp.getROperand()), 1)) { + stripped = binOp.getLOperand(); + } + } + return isLengthOf(stripped, call.getMethodExpression().getQualifierExpression()) ? getRedundantArgumentProblem(secondArg) : null; + } + + @Nullable + private ProblemDescriptor getRedundantZeroAsSecondParameterProblem(PsiMethodCallExpression call) { + PsiExpression secondArg = call.getArgumentList().getExpressions()[1]; + if (ExpressionUtils.isLiteral(PsiUtil.skipParenthesizedExprDown(secondArg), 0)) { + return getRedundantArgumentProblem(secondArg); + } + return null; + } + + @Nullable + private ProblemDescriptor getRedundantArgumentProblem(@Nullable PsiExpression argument) { + if (argument == null) return null; + LocalQuickFix fix = + new DeleteElementFix(argument, InspectionGadgetsBundle.message("inspection.redundant.string.remove.argument.fix.name")); + return myManager.createProblemDescriptor(argument, + InspectionGadgetsBundle.message( + "inspection.redundant.string.argument.message"), + myIsOnTheFly, + new LocalQuickFix[]{fix}, + ProblemHighlightType.LIKE_UNUSED_SYMBOL); + } + + @Nullable + private static PsiExpression getSingleEmptyStringArgument(PsiCall call) { + PsiExpressionList argList = call.getArgumentList(); + if (argList == null) return null; + PsiExpression[] args = argList.getExpressions(); + if (args.length != 1) return null; + return ExpressionUtils.isLiteral(PsiUtil.skipParenthesizedExprDown(args[0]), "") ? args[0] : null; + } + + @Nullable + private ProblemDescriptor getSubstringProblem(PsiMethodCallExpression call) { + PsiExpression[] args = call.getArgumentList().getExpressions(); + PsiExpression stringExpression = call.getMethodExpression().getQualifierExpression(); + if (args.length == 1) { + return ExpressionUtils.isZero(args[0]) ? getProblem(call, "inspection.redundant.string.call.message") : null; + } + // args.length == 2 + if (isLengthOf(args[1], stringExpression)) { + if (ExpressionUtils.isZero(args[0])) { + return getProblem(call, "inspection.redundant.string.call.message"); + } + DeleteElementFix fix = + new DeleteElementFix(args[1], InspectionGadgetsBundle.message("inspection.redundant.string.remove.argument.fix.name")); + return myManager.createProblemDescriptor(args[1], + InspectionGadgetsBundle.message("inspection.redundant.string.call.message"), + fix, ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly); + } + PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent()); + if (parent instanceof PsiExpressionList && ((PsiExpressionList)parent).getExpressionCount() == 1) { + PsiMethodCallExpression parentCall = tryCast(parent.getParent(), PsiMethodCallExpression.class); + if (STRING_BUILDER_APPEND.test(parentCall)) { + PsiElement nameElement = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()); + return myManager.createProblemDescriptor(nameElement, + InspectionGadgetsBundle.message("inspection.redundant.string.call.message"), + new RemoveRedundantStringCallFix( + nameElement.getText(), FixType.REPLACE_WITH_ARGUMENTS), + ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly); + } + } + return null; + } + + private static boolean isLengthOf(PsiExpression stringLengthCandidate, PsiExpression stringExpression) { + PsiMethodCallExpression argCall = tryCast(PsiUtil.skipParenthesizedExprDown(stringLengthCandidate), PsiMethodCallExpression.class); + return STRING_LENGTH.test(argCall) && + EquivalenceChecker.getCanonicalPsiEquivalence() + .expressionsAreEquivalent(stringExpression, argCall.getMethodExpression().getQualifierExpression()); + } + + @NotNull + private ProblemDescriptor getProblem(PsiMethodCallExpression call, @NotNull @PropertyKey(resourceBundle = BUNDLE) String key) { + String name = call.getMethodExpression().getReferenceName(); + return myManager.createProblemDescriptor(call, getRange(call), InspectionGadgetsBundle.message(key), + ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly, + new RemoveRedundantStringCallFix(name, FixType.REPLACE_WITH_QUALIFIER)); + } + + @NotNull + private static TextRange getRange(PsiMethodCallExpression call) { + PsiElement nameElement = call.getMethodExpression().getReferenceNameElement(); + if (nameElement != null) { + TextRange callRange = call.getTextRange(); + return new TextRange(nameElement.getTextRange().getStartOffset(), callRange.getEndOffset()).shiftLeft( + callRange.getStartOffset()); + } + return call.getTextRange(); } - return call.getTextRange(); } private static class RemoveRedundantStringCallFix implements LocalQuickFix {