diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/JdkObsolete.java b/core/src/main/java/com/google/errorprone/bugpatterns/JdkObsolete.java index f00df25d4d9..7346d46b429 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/JdkObsolete.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/JdkObsolete.java @@ -21,6 +21,7 @@ import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.method.MethodMatchers.constructor; import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod; import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod; @@ -29,6 +30,7 @@ import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.MemberReferenceTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher; import com.google.errorprone.fixes.Fix; @@ -36,6 +38,7 @@ import com.google.errorprone.fixes.SuggestedFixes; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.suppliers.Suppliers; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ClassTree; @@ -70,7 +73,7 @@ severity = WARNING ) public class JdkObsolete extends BugChecker - implements NewClassTreeMatcher, ClassTreeMatcher, MemberReferenceTreeMatcher { + implements NewClassTreeMatcher, ClassTreeMatcher, MethodInvocationTreeMatcher, MemberReferenceTreeMatcher { static class Obsolete { final String qualifiedName; @@ -118,6 +121,9 @@ Optional fix(Tree tree, VisitorState state) { "Stack is a nonstandard class that predates the Java Collections Framework;" + " prefer ArrayDeque. Note that the Stack methods push/pop/peek correspond" + " to the Deque methods addFirst/removeFirst/peekFirst."), + new Obsolete( + "java.lang.String", + "String methods should use typed Charset instead of String charset."), new Obsolete( "java.lang.StringBuffer", "StringBuffer performs synchronization that is usually unnecessary;" @@ -157,6 +163,17 @@ Optional fix(Tree tree, VisitorState state) { .onExactClass("com.google.re2j.Matcher") .withSignature("appendReplacement(java.lang.StringBuffer,java.lang.String)")); + static final Matcher MATCHER_STRING = + anyOf( + constructor() + .forClass("java.lang.String") + .withParameters(ImmutableList.of( + Suppliers.arrayOf(Suppliers.BYTE_TYPE), + Suppliers.typeFromClass(String.class))), + instanceMethod() + .onExactClass("java.lang.String") + .withSignature("getBytes(java.lang.String)")); + @Override public Description matchNewClass(NewClassTree tree, VisitorState state) { MethodSymbol constructor = ASTHelpers.getSymbol(tree); @@ -189,6 +206,10 @@ public Void visitMethodInvocation(MethodInvocationTree tree, Void unused) { if (found[0]) { return NO_MATCH; } + } else if (owner.getQualifiedName().contentEquals("java.lang.String")) { + if (!MATCHER_STRING.matches(tree, state)) { + return NO_MATCH; + } } return description; } @@ -207,6 +228,16 @@ public Description matchClass(ClassTree tree, VisitorState state) { return describeIfObsolete(null, state.getTypes().directSupertypes(symbol.asType()), state); } + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (MATCHER_STRING.matches(tree, state)) { + MethodSymbol symbol = ASTHelpers.getSymbol(tree); + return describeIfObsolete( + tree, ImmutableList.of(symbol.owner.asType()), state); + } + return NO_MATCH; + } + @Override public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) { MethodSymbol symbol = ASTHelpers.getSymbol(tree); diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/JdkObsoleteTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/JdkObsoleteTest.java index d2f332c6048..c6178737075 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/JdkObsoleteTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/JdkObsoleteTest.java @@ -45,9 +45,9 @@ public void positive() { testHelper .addSourceLines( "Test.java", - "import java.nio.file.Path;", + "import java.io.UnsupportedEncodingException;", "class Test {", - " {", + " void f() throws UnsupportedEncodingException {", " // BUG: Diagnostic contains:", " new java.util.LinkedList<>();", " // BUG: Diagnostic contains:", @@ -60,6 +60,25 @@ public void positive() { " new StringBuffer();", " // BUG: Diagnostic contains:", " new java.util.Hashtable() {};", + " // BUG: Diagnostic contains:", + " new String(new byte[0], \"UTF-8\");", + " // BUG: Diagnostic contains:", + " \"\".getBytes(\"UTF-8\");", + " }", + "}") + .doTest(); + } + + @Test + public void negative() { + testHelper + .addSourceLines( + "Test.java", + "import java.nio.charset.StandardCharsets;", + "class Test {", + " void f() {", + " new String(new byte[0], StandardCharsets.UTF_8);", + " \"\".getBytes(StandardCharsets.UTF_8);", " }", "}") .doTest();