diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index c3ba8c07a7b4..719f5b6e6067 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1500,8 +1500,9 @@ reflection.for.unavailable.annotation.problem.descriptor=Annotation '#ref' is no access.to.static.field.locked.on.instance.display.name=Access to static field locked on instance data access.to.static.field.locked.on.instance.problem.descriptor=Access to static field #ref locked on instance data #loc make.method.ctr.quickfix=Make method constructor -replace.all.dot.display.name=Call to String.replaceAll(".", ...) -replace.all.dot.problem.descriptor=Call to String.#ref(".", ...) #loc +replace.all.dot.display.name=Suspicious regex expression argument +replace.all.dot.problem.descriptor=Suspicious regex expression #ref in call to ''{0}()'' #loc +replace.all.dot.quickfix=Escape regex meta character class.extends.utility.class.display.name=Class extends utility class class.extends.utility.class.problem.descriptor=Class #ref extends utility class ''{0}'' #loc class.extends.utility.class.ignore.utility.class.option=Ignore if overriding class is a utility class diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ReplaceAllDotInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ReplaceAllDotInspection.java index 4cf325918d22..b6fbd11a8bd2 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ReplaceAllDotInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ReplaceAllDotInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2006-2019 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,29 +15,36 @@ */ package com.siyeh.ig.bugs; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.project.Project; import com.intellij.psi.*; -import com.intellij.psi.util.ConstantExpressionUtil; import com.intellij.psi.util.PsiUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; -import org.jetbrains.annotations.NonNls; +import com.siyeh.ig.InspectionGadgetsFix; +import com.siyeh.ig.PsiReplacementUtil; +import com.siyeh.ig.callMatcher.CallMatcher; +import com.siyeh.ig.psiutils.ExpressionUtils; +import org.intellij.lang.annotations.Pattern; +import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public class ReplaceAllDotInspection extends BaseInspection { @Override @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "replace.all.dot.display.name"); + return InspectionGadgetsBundle.message("replace.all.dot.display.name"); } @Override @NotNull public String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "replace.all.dot.problem.descriptor"); + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)infos[0]; + final String methodName = methodCallExpression.getMethodExpression().getReferenceName(); + return InspectionGadgetsBundle.message("replace.all.dot.problem.descriptor", methodName); } @Override @@ -45,61 +52,79 @@ public class ReplaceAllDotInspection extends BaseInspection { return true; } + @Pattern(VALID_ID_PATTERN) + @NotNull + @Override + public String getID() { + return "SuspiciousRegexArgument"; + } + + @Nullable + @Override + public String getAlternativeID() { + return "ReplaceAllDot"; + } + + @Nullable + @Override + protected InspectionGadgetsFix buildFix(Object... infos) { + final PsiExpression expression = (PsiExpression)infos[1]; + if (!(expression instanceof PsiLiteralExpression)) { + return null; + } + return new EscapeCharacterFix(); + } + + private static class EscapeCharacterFix extends InspectionGadgetsFix { + + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getFamilyName() { + return InspectionGadgetsBundle.message("replace.all.dot.quickfix"); + } + + @Override + protected void doFix(Project project, ProblemDescriptor descriptor) { + final PsiElement element = descriptor.getPsiElement(); + if (!(element instanceof PsiExpression)) { + return; + } + final PsiExpression expression = (PsiExpression)element; + final String text = expression.getText(); + + PsiReplacementUtil.replaceExpression(expression, text.substring(0, 1) + "\\\\" + text.substring(1)); + } + } + @Override public BaseInspectionVisitor buildVisitor() { return new ReplaceAllDotVisitor(); } - private static class ReplaceAllDotVisitor - extends BaseInspectionVisitor { + private static class ReplaceAllDotVisitor extends BaseInspectionVisitor { + + private static final CallMatcher.Simple MATCHER = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "replaceAll", "split"); @Override - public void visitMethodCallExpression( - @NotNull PsiMethodCallExpression expression) { + public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) { super.visitMethodCallExpression(expression); - final PsiReferenceExpression methodExpression = - expression.getMethodExpression(); - @NonNls final String methodName = - methodExpression.getReferenceName(); - if (!"replaceAll".equals(methodName)) { + if (!MATCHER.test(expression)) { return; } - final PsiExpressionList argumentList = expression.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 2) { + final PsiExpression argument = expression.getArgumentList().getExpressions()[0]; + if (!PsiUtil.isConstantExpression(argument) || !ExpressionUtils.hasStringType(argument)) { return; } - final PsiExpression argument = arguments[0]; - if (!PsiUtil.isConstantExpression(argument)) { + final String value = (String)ExpressionUtils.computeConstantExpression(argument); + if (!isRegexMetaChar(value)) { return; } - final PsiType argumentType = argument.getType(); - if (argumentType == null) { - return; - } - final String canonicalText = argumentType.getCanonicalText(); - if (!CommonClassNames.JAVA_LANG_STRING.equals(canonicalText)) { - return; - } - final String argValue = - (String)ConstantExpressionUtil.computeCastTo(argument, - argumentType); - if (!".".equals(argValue)) { - return; - } - final PsiMethod method = expression.resolveMethod(); - if (method == null) { - return; - } - final PsiClass containingClass = method.getContainingClass(); - if (containingClass == null) { - return; - } - final String qualifiedName = containingClass.getQualifiedName(); - if (!CommonClassNames.JAVA_LANG_STRING.equals(qualifiedName)) { - return; - } - registerMethodCallError(expression); + registerError(argument, expression, argument); + } + + private static boolean isRegexMetaChar(String s) { + return s != null && s.length() == 1 && ".$|()[{^?*+\\".contains(s); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml b/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml index 1780fb31bcc5..f09abc1b49dc 100644 --- a/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml +++ b/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml @@ -332,7 +332,8 @@ key="reflection.for.unavailable.annotation.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING" implementationClass="com.siyeh.ig.bugs.ReflectionForUnavailableAnnotationInspection"/> - Reports any calls to -java.lang.String.replaceAll() with "." -as the first argument. Calling replaceAll(".", ...) replaces -all of the characters in a string with its second argument, which is rarely the desired functionality. -More probably, replaceAll("\.", ...) was intended. +String.replaceAll() or String.split() where the first argument is a single regex meta character argument. +The regex meta characters are one of ".$|()[{^?*+\", and these have a special meaning in regular expressions. +For example calling "ab.cd".replaceAll(".", "-") produces "-----", because the dot matches any character. +Most likely the escaped variant "\\." was intended instead.

diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/replace_all_dot/SuspiciousRegexExpressionArgument.after.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/replace_all_dot/SuspiciousRegexExpressionArgument.after.java new file mode 100644 index 000000000000..ea90a63cec18 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/replace_all_dot/SuspiciousRegexExpressionArgument.after.java @@ -0,0 +1,9 @@ +class SuspicousRegexExpressionArgument {{ + + "a.s.d.f".split("\\."); + "vb|amna".replaceAll("\\|", "-"); + "1+2+3".split("\\+"); + "one two".split(" "); + "[][][]".split("]"); + "{}{}{}".split("}"); +}} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/replace_all_dot/SuspiciousRegexExpressionArgument.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/replace_all_dot/SuspiciousRegexExpressionArgument.java new file mode 100644 index 000000000000..87d33fa137eb --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/replace_all_dot/SuspiciousRegexExpressionArgument.java @@ -0,0 +1,9 @@ +class SuspicousRegexExpressionArgument {{ + + "a.s.d.f".split("."); + "vb|amna".replaceAll("|", "-"); + "1+2+3".split("+"); + "one two".split(" "); + "[][][]".split("]"); + "{}{}{}".split("}"); +}} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ReplaceAllDotInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ReplaceAllDotInspectionTest.java new file mode 100644 index 000000000000..b77f782223dd --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ReplaceAllDotInspectionTest.java @@ -0,0 +1,23 @@ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.siyeh.ig.bugs; + +import com.intellij.codeInspection.InspectionProfileEntry; +import com.siyeh.ig.LightJavaInspectionTestCase; +import org.jetbrains.annotations.Nullable; + +/** + * @author Bas Leijdekkers + */ +public class ReplaceAllDotInspectionTest extends LightJavaInspectionTestCase { + + public void testSuspiciousRegexExpressionArgument() { + doTest(); + checkQuickFixAll(); + } + + @Nullable + @Override + protected InspectionProfileEntry getInspection() { + return new ReplaceAllDotInspection(); + } +} \ No newline at end of file