Skip to content

Commit

Permalink
Suggest Splitter.on(Pattern.compile(...)) instead of `Splitter.onPa…
Browse files Browse the repository at this point in the history 
…ttern`

onPattern is now deprecated.

PiperOrigin-RevId: 678828054
  • Loading branch information
@cushon
cushon authored and Error Prone Team committed Sep 25, 2024
1 parent 50d0983 commit aad69a3
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 30 deletions.
16 changes: 11 additions & 5 deletions core/src/main/java/com/google/errorprone/bugpatterns/StringSplitter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,25 +226,26 @@ public Boolean visitMemberSelect(MemberSelectTree tree, Void unused) {
return Optional.of(fix.build());
}

private static String getMethodAndArgument(Tree origArg, VisitorState state) {
private static String getMethodAndArgument(
SuggestedFix.Builder fix, Tree origArg, VisitorState state) {
String argSource = state.getSourceForNode(origArg);
Tree arg = ASTHelpers.stripParentheses(origArg);
if (arg.getKind() != Tree.Kind.STRING_LITERAL) {
// Even if the regex is a constant, it still needs to be treated as a regex, since the
// value comes from the symbol and/or a concatenation; the values of the subexpressions may be
// changed subsequently.
return String.format("onPattern(%s)", argSource);
return onPattern(fix, argSource);
}
String constValue = ASTHelpers.constValue(arg, String.class);
if (constValue == null) {
// Not a constant value, so we can't assume anything about pattern: have to treat it as a
// regex.
return String.format("onPattern(%s)", argSource);
return onPattern(fix, argSource);
}
Optional<String> regexAsLiteral = convertRegexToLiteral(constValue);
if (!regexAsLiteral.isPresent()) {
// Can't convert the regex to a literal string: have to treat it as a regex.
return String.format("onPattern(%s)", argSource);
return onPattern(fix, argSource);
}
String escaped = SourceCodeEscapers.javaCharEscaper().escape(regexAsLiteral.get());
if (regexAsLiteral.get().length() == 1) {
Expand All @@ -253,6 +254,11 @@ private static String getMethodAndArgument(Tree origArg, VisitorState state) {
return String.format("on(\"%s\")", escaped);
}

private static String onPattern(SuggestedFix.Builder fix, String argSource) {
fix.addImport("java.util.regex.Pattern");
return String.format("on(Pattern.compile(%s))", argSource);
}

private static SuggestedFix.Builder replaceWithSplitter(
SuggestedFix.Builder fix,
MethodInvocationTree tree,
Expand All @@ -267,7 +273,7 @@ private static SuggestedFix.Builder replaceWithSplitter(
fix.addImport("com.google.common.base.Splitter");
Type receiverType = getType(receiver);
if (isSubtype(receiverType, state.getSymtab().stringType, state)) {
String methodAndArgument = getMethodAndArgument(arg, state);
String methodAndArgument = getMethodAndArgument(fix, arg, state);
return fix.prefixWith(
receiver,
String.format(
Expand Down
57 changes: 32 additions & 25 deletions core/src/test/java/com/google/errorprone/bugpatterns/StringSplitterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,24 @@ void f() {
.addOutputLines(
"Test.java",
"""
import com.google.common.base.Splitter;
import com.google.common.base.Splitter;
import java.util.regex.Pattern;
class Test {
static final String NON_REGEX_PATTERN_STRING = ":";
static final String REGEX_PATTERN_STRING = ".*";
static final String CONVERTIBLE_PATTERN_STRING = "\\\\Q\\\\E:";
class Test {
static final String NON_REGEX_PATTERN_STRING = ":";
static final String REGEX_PATTERN_STRING = ".*";
static final String CONVERTIBLE_PATTERN_STRING = "\\\\Q\\\\E:";
void f() {
for (String s : Splitter.onPattern(NON_REGEX_PATTERN_STRING).split("")) {}
for (String s : Splitter.onPattern(REGEX_PATTERN_STRING).split("")) {}
for (String s : Splitter.onPattern(CONVERTIBLE_PATTERN_STRING).split("")) {}
for (String s : Splitter.onPattern((CONVERTIBLE_PATTERN_STRING)).split("")) {}
}
}
""")
void f() {
for (String s : Splitter.on(Pattern.compile(NON_REGEX_PATTERN_STRING)).split("")) {}
for (String s : Splitter.on(Pattern.compile(REGEX_PATTERN_STRING)).split("")) {}
for (String s :
Splitter.on(Pattern.compile(CONVERTIBLE_PATTERN_STRING)).split("")) {}
for (String s :
Splitter.on(Pattern.compile((CONVERTIBLE_PATTERN_STRING))).split("")) {}
}
}
""")
.doTest(TestMode.TEXT_MATCH);
}

Expand All @@ -139,10 +142,11 @@ void f() {
"Test.java",
"""
import com.google.common.base.Splitter;
import java.util.regex.Pattern;
class Test {
void f() {
for (String s : Splitter.onPattern(":" + 0).split("")) {}
for (String s : Splitter.on(Pattern.compile(":" + 0)).split("")) {}
}
}
""")
Expand All @@ -166,11 +170,12 @@ void f() {
"Test.java",
"""
import com.google.common.base.Splitter;
import java.util.regex.Pattern;
class Test {
void f() {
String pattern = ":";
for (String s : Splitter.onPattern(pattern).split("")) {}
for (String s : Splitter.on(Pattern.compile(pattern)).split("")) {}
}
}
""")
Expand Down Expand Up @@ -307,10 +312,11 @@ void f() {
"Test.java",
"""
import com.google.common.base.Splitter;
import java.util.regex.Pattern;
class Test {
void f() {
for (String s : Splitter.onPattern(".*foo\\\\t").split("")) {}
for (String s : Splitter.on(Pattern.compile(".*foo\\\\t")).split("")) {}
}
}
""")
Expand Down Expand Up @@ -408,16 +414,17 @@ void f(String input) {
.addOutputLines(
"Test.java",
"""
import com.google.common.base.Splitter;
import java.util.List;
import com.google.common.base.Splitter;
import java.util.List;
import java.util.regex.Pattern;
class Test {
void f(String input) {
List<String> lines = Splitter.onPattern("\\\\r?\\\\n").splitToList(input);
System.err.println(lines.get(0));
}
}
""")
class Test {
void f(String input) {
List<String> lines = Splitter.on(Pattern.compile("\\\\r?\\\\n")).splitToList(input);
System.err.println(lines.get(0));
}
}
""")
.doTest();
}

Expand Down

0 comments on commit aad69a3

Please sign in to comment.