IG: warn about more suspicious regex string arguments and add quick fix (IDEA-227611)

GitOrigin-RevId: d6522a26d4721d042df52ebc6a7fa575760931a7

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 <code>#ref</code> 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 <code>String.#ref(".", ...)</code> #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 <code>#ref</code> extends utility class ''{0}'' #loc
 class.extends.utility.class.ignore.utility.class.option=Ignore if overriding class is a utility class

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);
     }
   }
 }

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"/>
-    <localInspection groupPath="Java" language="JAVA" shortName="ReplaceAllDot" bundle="com.siyeh.InspectionGadgetsBundle" key="replace.all.dot.display.name"
+    <localInspection groupPath="Java" language="JAVA" suppressId="SuspiciousRegexArgument" alternativeId="ReplaceAllDot"
+                     shortName="ReplaceAllDot" bundle="com.siyeh.InspectionGadgetsBundle" key="replace.all.dot.display.name"
                      groupBundle="messages.InspectionsBundle" groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING"
                      implementationClass="com.siyeh.ig.bugs.ReplaceAllDotInspection"/>
     <localInspection groupPath="Java" language="JAVA" shortName="ResultOfObjectAllocationIgnored" bundle="com.siyeh.InspectionGadgetsBundle"

plugins/InspectionGadgets/src/inspectionDescriptions/ReplaceAllDot.html

@@ -1,10 +1,10 @@
 <html>
 <body>
 Reports any calls to
-<b>java.lang.String.replaceAll()</b> with <b>"."</b>
-as the first argument. Calling <b>replaceAll(".", ...)</b> replaces
-all of the characters in a string with its second argument, which is rarely the desired functionality.
-More probably, <b>replaceAll("\.", ...)</b> was intended.
+<b>String.replaceAll()</b> or <b>String.split()</b> 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 <b>"ab.cd".replaceAll(".", "-")</b> produces <b>"-----"</b>, because the dot matches any character.
+Most likely the escaped variant <b>"\\."</b> was intended instead.
 <!-- tooltip end -->
 <p>
 

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("}");
+}}

plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/replace_all_dot/SuspiciousRegexExpressionArgument.java

@@ -0,0 +1,9 @@
+class SuspicousRegexExpressionArgument {{
+
+  "a.s.d.f".split(<warning descr="Suspicious regex expression \".\" in call to 'split()'"><caret>"."</warning>);
+  "vb|amna".replaceAll(<warning descr="Suspicious regex expression \"|\" in call to 'replaceAll()'">"|"</warning>, "-");
+  "1+2+3".split(<warning descr="Suspicious regex expression \"+\" in call to 'split()'">"<error descr="Dangling metacharacter">+</error>"</warning>);
+  "one two".split(" ");
+  "[][][]".split("]");
+  "{}{}{}".split("}");
+}}

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();
+  }
+}