Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest Splitter.on(Pattern.compile(...)) instead of Splitter.onPattern #4591

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Suggest Splitter.on(Pattern.compile(...)) instead of Splitter.onPattern #4591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions core/src/main/java/com/google/errorprone/bugpatterns/StringSplitter.java
View file
Open in desktop
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
Open in desktop
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
Loading