-
Notifications
You must be signed in to change notification settings - Fork 751
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Move check for the regex "." to a new WARNING-level check
PiperOrigin-RevId: 407388923
- Loading branch information
Showing
6 changed files
with
190 additions
and
95 deletions.
There are no files selected for viewing
78 changes: 78 additions & 0 deletions
78
core/src/main/java/com/google/errorprone/bugpatterns/AbstractPatternSyntaxChecker.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| /* | ||
| * Copyright 2021 The Error Prone Authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.errorprone.bugpatterns; | ||
|
|
||
| import static com.google.errorprone.matchers.Description.NO_MATCH; | ||
| import static com.google.errorprone.matchers.Matchers.anyOf; | ||
| import static com.google.errorprone.matchers.Matchers.instanceMethod; | ||
| import static com.google.errorprone.matchers.Matchers.staticMethod; | ||
|
|
||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.annotations.ForOverride; | ||
| import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.google.errorprone.matchers.Matcher; | ||
| import com.google.errorprone.util.ASTHelpers; | ||
| import com.sun.source.tree.ExpressionTree; | ||
| import com.sun.source.tree.MethodInvocationTree; | ||
| import javax.annotation.CheckReturnValue; | ||
|
|
||
| /** Finds calls to regex-accepting methods with literal strings. */ | ||
| @CheckReturnValue | ||
| abstract class AbstractPatternSyntaxChecker extends BugChecker | ||
| implements MethodInvocationTreeMatcher { | ||
|
|
||
| /* | ||
| * Match invocations to regex-accepting methods. Subclasses will be consulted to see whether the | ||
| * pattern passed to such methods are acceptable. | ||
| * | ||
| * <p>We deliberately omit Pattern.compile itself, as most of its users appear to be either | ||
| * e.g. passing LITERAL flags or deliberately testing the regex compiler. | ||
| */ | ||
| private static final Matcher<MethodInvocationTree> REGEX_USAGE = | ||
| anyOf( | ||
| instanceMethod() | ||
| .onExactClass("java.lang.String") | ||
| .namedAnyOf("matches", "split") | ||
| .withParameters("java.lang.String"), | ||
| instanceMethod() | ||
| .onExactClass("java.lang.String") | ||
| .named("split") | ||
| .withParameters("java.lang.String", "int"), | ||
| instanceMethod() | ||
| .onExactClass("java.lang.String") | ||
| .namedAnyOf("replaceFirst", "replaceAll") | ||
| .withParameters("java.lang.String", "java.lang.String"), | ||
| staticMethod().onClass("java.util.regex.Pattern").named("matches"), | ||
| staticMethod().onClass("com.google.common.base.Splitter").named("onPattern")); | ||
|
|
||
| @Override | ||
| public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { | ||
| if (!REGEX_USAGE.matches(tree, state)) { | ||
| return NO_MATCH; | ||
| } | ||
| ExpressionTree arg = tree.getArguments().get(0); | ||
| String value = ASTHelpers.constValue(arg, String.class); | ||
| if (value == null) { | ||
| return NO_MATCH; | ||
| } | ||
| return matchRegexLiteral(tree, value); | ||
| } | ||
|
|
||
| @ForOverride | ||
| protected abstract Description matchRegexLiteral(MethodInvocationTree tree, String pattern); | ||
| } |
48 changes: 48 additions & 0 deletions
48
core/src/main/java/com/google/errorprone/bugpatterns/BareDotMetacharacter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| /* | ||
| * Copyright 2021 The Error Prone Authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.errorprone.bugpatterns; | ||
|
|
||
| import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; | ||
| import static com.google.errorprone.matchers.Description.NO_MATCH; | ||
|
|
||
| import com.google.errorprone.BugPattern; | ||
| import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; | ||
| import com.google.errorprone.fixes.SuggestedFix; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.sun.source.tree.MethodInvocationTree; | ||
|
|
||
| /** A BugChecker; see the associated BugPattern for details. */ | ||
| @BugPattern( | ||
| name = "BareDotMetacharacter", | ||
| summary = | ||
| "\".\" is rarely useful as a regex, as it matches any character. To match a literal '.'" | ||
| + " character, instead write \"\\\\.\".", | ||
| severity = WARNING, | ||
| // So that suppressions added before this check was split into two apply to both halves. | ||
| altNames = {"InvalidPatternSyntax"}) | ||
| public class BareDotMetacharacter extends AbstractPatternSyntaxChecker | ||
| implements MethodInvocationTreeMatcher { | ||
|
|
||
| @Override | ||
| protected final Description matchRegexLiteral(MethodInvocationTree tree, String regex) { | ||
| if (regex.equals(".")) { | ||
| return describeMatch(tree, SuggestedFix.replace(tree.getArguments().get(0), "\"\\\\.\"")); | ||
| } else { | ||
| return NO_MATCH; | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletions
54
core/src/test/java/com/google/errorprone/bugpatterns/BareDotMetacharacterTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /* | ||
| * Copyright 2012 The Error Prone Authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.errorprone.bugpatterns; | ||
|
|
||
| import com.google.errorprone.BugCheckerRefactoringTestHelper; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.junit.runners.JUnit4; | ||
|
|
||
| /** Tests for {@link BareDotMetacharacter}. */ | ||
| @RunWith(JUnit4.class) | ||
| public class BareDotMetacharacterTest { | ||
|
|
||
| private final BugCheckerRefactoringTestHelper refactoringHelper = | ||
| BugCheckerRefactoringTestHelper.newInstance(BareDotMetacharacter.class, getClass()); | ||
|
|
||
| @Test | ||
| public void testPositiveCase() { | ||
| refactoringHelper | ||
| .addInputLines( | ||
| "Test.java", | ||
| "import com.google.common.base.Splitter;", | ||
| "class Test {", | ||
| " private static final String DOT = \".\";", | ||
| " Object x = \"foo.bar\".split(\".\");", | ||
| " Object y = \"foo.bonk\".split(DOT);", | ||
| " Object z = Splitter.onPattern(\".\");", | ||
| "}") | ||
| .addOutputLines( | ||
| "Test.java", | ||
| "import com.google.common.base.Splitter;", | ||
| "class Test {", | ||
| " private static final String DOT = \".\";", | ||
| " Object x = \"foo.bar\".split(\"\\\\.\");", | ||
| " Object y = \"foo.bonk\".split(\"\\\\.\");", | ||
| " Object z = Splitter.onPattern(\"\\\\.\");", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,13 +16,11 @@ | |
|
|
||
| package com.google.errorprone.bugpatterns.testdata; | ||
|
|
||
| import com.google.common.base.Splitter; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** @author [email protected] (Matthew Dempsky) */ | ||
| public class InvalidPatternSyntaxPositiveCases { | ||
| public static final String INVALID = "*"; | ||
| public static final String DOT = "."; | ||
|
|
||
| { | ||
| // BUG: Diagnostic contains: Unclosed character class | ||
|
|
@@ -44,13 +42,5 @@ public class InvalidPatternSyntaxPositiveCases { | |
| "".split(INVALID); | ||
| // BUG: Diagnostic contains: | ||
| "".split(INVALID, 0); | ||
|
|
||
| // BUG: Diagnostic contains: "foo.bar".split("\\.") | ||
| "foo.bar".split("."); | ||
| // BUG: Diagnostic contains: | ||
| "foo.bonk".split(DOT); | ||
|
|
||
| // BUG: Diagnostic contains: Splitter.onPattern("\\.") | ||
| Splitter.onPattern("."); | ||
| } | ||
| } | ||