Fix flagging serialization methods in Unused.

Extract the test so it's a bit clearer. MOE_MIGRATED_REVID=211495096
google · Sep 13, 2018 · 91c3443 · 91c3443
1 parent 915758f
commit 91c3443
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 13 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/Unused.java b/core/src/main/java/com/google/errorprone/bugpatterns/Unused.java
@@ -22,7 +22,9 @@
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.Matchers.allOf;
 import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.isSameType;
 import static com.google.errorprone.matchers.Matchers.isVoidType;
+import static com.google.errorprone.matchers.Matchers.methodHasParameters;
 import static com.google.errorprone.matchers.Matchers.methodIsNamed;
 import static com.google.errorprone.matchers.Matchers.methodReturns;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
@@ -112,14 +114,15 @@ public final class Unused extends BugChecker implements CompilationUnitTreeMatch
 
   private static final Supplier<Type> OBJECT = Suppliers.typeFromString("java.lang.Object");
 
-  private static final Supplier<Type> OBJECT_OUTPUT_STREAM =
-      Suppliers.typeFromString("java.io.ObjectOutputStream");
-
   /** Method signature of special methods. */
   private static final Matcher<MethodTree> SPECIAL_METHODS =
       anyOf(
-          allOf(methodIsNamed("readObject"), methodReturns(OBJECT_OUTPUT_STREAM)),
-          allOf(methodIsNamed("writeObject"), methodReturns(OBJECT_OUTPUT_STREAM)),
+          allOf(
+              methodIsNamed("readObject"),
+              methodHasParameters(isSameType("java.io.ObjectInputStream"))),
+          allOf(
+              methodIsNamed("writeObject"),
+              methodHasParameters(isSameType("java.io.ObjectOutputStream"))),
           allOf(methodIsNamed("readObjectNoData"), methodReturns(isVoidType())),
           allOf(methodIsNamed("readResolve"), methodReturns(OBJECT)),
           allOf(methodIsNamed("writeReplace"), methodReturns(OBJECT)));

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedTest.java
@@ -332,14 +332,6 @@ public void unuseds() {
             "  private void notUsedMethod() {}",
             "  // BUG: Diagnostic contains:",
             "  private static void staticNotUsedMethod() {}",
-            "  // BUG: Diagnostic contains:",
-            "  private void writeObject(java.io.ObjectOutputStream out) throws IOException {",
-            "    out.writeInt(123);",
-            "  }",
-            "  private Object readResolve() {",
-            "    return null;",
-            "  }",
-            "  private void readObjectNoData() throws ObjectStreamException {}",
             "  void memberSelectUpdate1() {",
             "    List<Unuseds> l = null;",
             "    // `u` should not be reported as unused.",
@@ -387,6 +379,29 @@ public void unuseds() {
         .doTest();
   }
 
+  @Test
+  public void exemptedMethods() {
+    helper
+        .addSourceLines(
+            "Unuseds.java",
+            "package unusedvars;",
+            "import java.io.IOException;",
+            "import java.io.ObjectStreamException;",
+            "public class Unuseds {",
+            "  private void readObject(java.io.ObjectInputStream in) throws IOException {",
+            "    in.hashCode();",
+            "  }",
+            "  private void writeObject(java.io.ObjectOutputStream out) throws IOException {",
+            "    out.writeInt(123);",
+            "  }",
+            "  private Object readResolve() {",
+            "    return null;",
+            "  }",
+            "  private void readObjectNoData() throws ObjectStreamException {}",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void refactoring() {
     refactoringHelper