UnusedPrivateMethod check should allow serialization-related methods (rfe 1036387)

2005-01-31 13:53:22 +00:00 · 2005-01-31 13:53:22 +00:00 · 621d9a9bf7
parent 7bef7b4011
commit 621d9a9bf7
5 changed files with 332 additions and 2 deletions
--- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheck.java
+++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheck.java
@ -19,6 +19,7 @@
 package com.puppycrawl.tools.checkstyle.checks.usage;

 import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.FullIdent;
 import com.puppycrawl.tools.checkstyle.api.Scope;
 import com.puppycrawl.tools.checkstyle.api.ScopeUtils;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
@ -39,6 +40,8 @@ public class UnusedPrivateMethodCheck
    extends AbstractUsageCheck

 {
+    /** Controls if checks skips serialization methods.*/
+    private boolean mAllowSerializationMethods;
    /** @see com.puppycrawl.tools.checkstyle.api.Check */
    public int[] getDefaultTokens()
    {
@ -53,11 +56,189 @@ public class UnusedPrivateMethodCheck
        return "unused.method";
    }

+    /**
+     * Configure the check to allow (or not) serialization-related methods.
+     * @param aFlag new value for allowSerializationMethods value.
+     */
+    public void setAllowSerializationMethods(boolean aFlag)
+    {
+        mAllowSerializationMethods = aFlag;
+    }
+
    /** @see com.puppycrawl.tools.checkstyle.checks.usage.AbstractUsageCheck */
    public boolean mustCheckReferenceCount(DetailAST aAST)
    {
        final DetailAST mods = aAST.findFirstToken(TokenTypes.MODIFIERS);
-        return ((mods != null)
-            && (ScopeUtils.getScopeFromMods(mods) == Scope.PRIVATE));
+        if ((mods == null)
+            || (ScopeUtils.getScopeFromMods(mods) != Scope.PRIVATE))
+        {
+            return false;
+        }
+
+        return !mAllowSerializationMethods
+               || !(isWriteObject(aAST) || isReadObject(aAST)
+                    || isWriteReplaceOrReadResolve(aAST));
+    }
+
+    /**
+     * Checks if a given method is writeObject().
+     * @param aAST method def to check
+     * @return true if this is a writeObject() definition
+     */
+    private boolean isWriteObject(DetailAST aAST)
+    {
+        // name is writeObject...
+        final DetailAST ident = aAST.findFirstToken(TokenTypes.IDENT);
+        if (!"writeObject".equals(ident.getText())) {
+            return false;
+        }
+
+        // returns void...
+        final DetailAST typeAST =
+            (DetailAST) aAST.findFirstToken(TokenTypes.TYPE).getFirstChild();
+        if (typeAST.getType() != TokenTypes.LITERAL_VOID) {
+            return false;
+        }
+
+        // should have one parameter...
+        final DetailAST params = aAST.findFirstToken(TokenTypes.PARAMETERS);
+        if (params == null || params.getChildCount() != 1) {
+            return false;
+        }
+        // and paramter's type should be java.io.ObjectOutputStream
+        final DetailAST type =
+            (DetailAST) ((DetailAST) params.getFirstChild())
+                .findFirstToken(TokenTypes.TYPE).getFirstChild();
+        final String typeName = FullIdent.createFullIdent(type).getText();
+        if (!"java.io.ObjectOutputStream".equals(typeName)
+            && !"ObjectOutputStream".equals(typeName))
+        {
+            return false;
+        }
+
+        // and, finally, it should throws java.io.IOException
+        final DetailAST throwsAST =
+            aAST.findFirstToken(TokenTypes.LITERAL_THROWS);
+        if (throwsAST == null || throwsAST.getChildCount() != 1) {
+            return false;
+        }
+        final DetailAST expt = (DetailAST) throwsAST.getFirstChild();
+        final String exceptionName = FullIdent.createFullIdent(expt).getText();
+        if (!"java.io.IOException".equals(exceptionName)
+            && !"IOException".equals(exceptionName))
+        {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Checks if a given method is readObject().
+     * @param aAST method def to check
+     * @return true if this is a readObject() definition
+     */
+    private boolean isReadObject(DetailAST aAST)
+    {
+        // name is readObject...
+        final DetailAST ident = aAST.findFirstToken(TokenTypes.IDENT);
+        if (!"readObject".equals(ident.getText())) {
+            return false;
+        }
+
+        // returns void...
+        final DetailAST typeAST =
+            (DetailAST) aAST.findFirstToken(TokenTypes.TYPE).getFirstChild();
+        if (typeAST.getType() != TokenTypes.LITERAL_VOID) {
+            return false;
+        }
+
+        // should have one parameter...
+        final DetailAST params = aAST.findFirstToken(TokenTypes.PARAMETERS);
+        if (params == null || params.getChildCount() != 1) {
+            return false;
+        }
+        // and paramter's type should be java.io.ObjectInputStream
+        final DetailAST type =
+            (DetailAST) ((DetailAST) params.getFirstChild())
+                .findFirstToken(TokenTypes.TYPE).getFirstChild();
+        final String typeName = FullIdent.createFullIdent(type).getText();
+        if (!"java.io.ObjectInputStream".equals(typeName)
+            && !"ObjectInputStream".equals(typeName))
+        {
+            return false;
+        }
+
+        // and, finally, it should throws java.io.IOException
+        // and java.lang.ClassNotFoundException
+        final DetailAST throwsAST =
+            aAST.findFirstToken(TokenTypes.LITERAL_THROWS);
+        if (throwsAST == null || throwsAST.getChildCount() != 3) {
+            return false;
+        }
+        final DetailAST excpt1 = (DetailAST) throwsAST.getFirstChild();
+        final String exception1 = FullIdent.createFullIdent(excpt1).getText();
+        final String exception2 =
+            FullIdent.createFullIdent(throwsAST.getLastChild()).getText();
+        if (!"java.io.IOException".equals(exception1)
+            && !"IOException".equals(exception1)
+            && !"java.io.IOException".equals(exception2)
+            && !"IOException".equals(exception2)
+            || !"java.lang.ClassNotFoundException".equals(exception1)
+            && !"ClassNotFoundException".equals(exception1)
+            && !"java.lang.ClassNotFoundException".equals(exception2)
+            && !"ClassNotFoundException".equals(exception2))
+        {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Checks if a given method is writeReplace() or readResolve().
+     * @param aAST method def to check
+     * @return true if this is a writeReplace() definition
+     */
+    private boolean isWriteReplaceOrReadResolve(DetailAST aAST)
+    {
+        // name is writeReplace or readResolve...
+        final DetailAST ident = aAST.findFirstToken(TokenTypes.IDENT);
+        if (!"writeReplace".equals(ident.getText())
+            && !"readResolve".equals(ident.getText()))
+        {
+            return false;
+        }
+
+        // returns Object...
+        final DetailAST typeAST =
+            (DetailAST) aAST.findFirstToken(TokenTypes.TYPE).getFirstChild();
+        if (typeAST.getType() != TokenTypes.DOT
+            && typeAST.getType() != TokenTypes.IDENT)
+        {
+            return false;
+        }
+
+        // should have no parameters...
+        final DetailAST params = aAST.findFirstToken(TokenTypes.PARAMETERS);
+        if (params != null && params.getChildCount() != 0) {
+            return false;
+        }
+
+        // and, finally, it should throws java.io.ObjectStreamException
+        final DetailAST throwsAST =
+            aAST.findFirstToken(TokenTypes.LITERAL_THROWS);
+        if (throwsAST == null || throwsAST.getChildCount() != 1) {
+            return false;
+        }
+        final DetailAST excpt = (DetailAST) throwsAST.getFirstChild();
+        final String exception = FullIdent.createFullIdent(excpt).getText();
+        if (!"java.io.ObjectStreamException".equals(exception)
+            && !"ObjectStreamException".equals(exception))
+        {
+            return false;
+        }
+
+        return true;
    }
 }
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/usage/InputUnusedMethod.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/usage/InputUnusedMethod.java
@ -60,3 +60,93 @@ class Ternary
        int j = (i == 0) ? (i) : m();
    }
 }
+
+class SerializableTest implements java.io.Serializable
+{
+    private void writeObject(java.io.ObjectOutputStream out) throws java.io.IOException
+    {
+        // it's ok to have this method in serializable class
+    }
+
+    private void readObject(java.io.ObjectInputStream in) throws java.io.IOException, ClassNotFoundException
+    {
+        // it's ok to have this method in serializable class
+    }
+
+    private Object writeReplace() throws java.io.ObjectStreamException
+    {
+        // it's ok to have this method in serializable class
+        return new SerializableTest();
+    }
+
+    private Object readResolve() throws java.io.ObjectStreamException
+    {
+        // it's ok to have this method in serializable class
+        return new SerializableTest();
+    }
+}
+
+class BadSerializableTest implements java.io.Serializable
+{
+    private void writeObject(Object out) throws java.io.IOException
+    {
+    }
+
+    private void writeObject(java.io.ObjectOutputStream out, int i) throws java.io.IOException
+    {
+    }
+
+    private void writeObject(java.io.ObjectOutputStream out)
+    {
+    }
+
+    private int writeObject(java.io.ObjectOutputStream out) throws java.io.IOException
+    {
+    }
+
+    private void readObject(java.io.ObjectInputStream in) throws java.io.IOException
+    {
+    }
+
+    private void readObject(java.io.ObjectInputStream in) throws ClassNotFoundException
+    {
+    }
+
+    private void readObject(Object in) throws java.io.IOException, ClassNotFoundException
+    {
+    }
+
+    private int readObject(java.io.ObjectInputStream in) throws java.io.IOException, ClassNotFoundException
+    {
+    }
+
+    private int writeReplace() throws java.io.ObjectStreamException
+    {
+        return 1;
+    }
+
+    private java.lang.Object writeReplace()
+    {
+        return new SerializableTest();
+    }
+
+    private Object writeReplace(int i) throws java.io.ObjectStreamException
+    {
+        return new SerializableTest();
+    }
+
+    private int readResolve() throws java.io.ObjectStreamException
+    {
+        return 1;
+    }
+
+    private Object readResolve()
+    {
+        return new SerializableTest();
+    }
+
+    private Object readResolve(int i) throws java.io.ObjectStreamException
+    {
+        return new SerializableTest();
+    }
+}
--- a/src/tests/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheckTest.java
+++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheckTest.java
@ -12,6 +12,50 @@ public class UnusedPrivateMethodCheckTest
            createCheckConfig(UnusedPrivateMethodCheck.class);
        final String[] expected = {
            "7:18: Unused private method 'methodUnused0'.",
+            "66:18: Unused private method 'writeObject'.",
+            "71:18: Unused private method 'readObject'.",
+            "76:20: Unused private method 'writeReplace'.",
+            "82:20: Unused private method 'readResolve'.",
+            "91:18: Unused private method 'writeObject'.",
+            "95:18: Unused private method 'writeObject'.",
+            "99:18: Unused private method 'writeObject'.",
+            "103:17: Unused private method 'writeObject'.",
+            "107:18: Unused private method 'readObject'.",
+            "111:18: Unused private method 'readObject'.",
+            "115:18: Unused private method 'readObject'.",
+            "119:17: Unused private method 'readObject'.",
+            "123:17: Unused private method 'writeReplace'.",
+            "128:30: Unused private method 'writeReplace'.",
+            "133:20: Unused private method 'writeReplace'.",
+            "138:17: Unused private method 'readResolve'.",
+            "143:20: Unused private method 'readResolve'.",
+            "148:20: Unused private method 'readResolve'.",
+        };
+        verify(checkConfig, getPath("usage/InputUnusedMethod.java"), expected);
+    }
+
+    public void testAllowSerializationMethods() throws Exception
+    {
+        final DefaultConfiguration checkConfig =
+            createCheckConfig(UnusedPrivateMethodCheck.class);
+        checkConfig.addAttribute("allowSerializationMethods", "true");
+
+        final String[] expected = {
+            "7:18: Unused private method 'methodUnused0'.",
+            "91:18: Unused private method 'writeObject'.",
+            "95:18: Unused private method 'writeObject'.",
+            "99:18: Unused private method 'writeObject'.",
+            "103:17: Unused private method 'writeObject'.",
+            "107:18: Unused private method 'readObject'.",
+            "111:18: Unused private method 'readObject'.",
+            "115:18: Unused private method 'readObject'.",
+            "119:17: Unused private method 'readObject'.",
+            "123:17: Unused private method 'writeReplace'.",
+            "128:30: Unused private method 'writeReplace'.",
+            "133:20: Unused private method 'writeReplace'.",
+            "138:17: Unused private method 'readResolve'.",
+            "143:20: Unused private method 'readResolve'.",
+            "148:20: Unused private method 'readResolve'.",
        };
        verify(checkConfig, getPath("usage/InputUnusedMethod.java"), expected);
    }
--- a/src/xdocs/config_usage.xml
+++ b/src/xdocs/config_usage.xml
@ -299,6 +299,18 @@
              </td>
              <td><span class="default">^$</span> (empty)</td>
            </tr>
+            <tr>
+              <td>allowSerializationMethods</td>
+              <td>whether the check should allow serialization-related methods
+              (<span class="code">readObject()</span>, <span
+              class="code">writeObject()</span>, <span
+              class="code">readResolve()</span> and <span
+              class="code">writeReplace()</span></td>
+              <td>
+                <a href="property_types.html#boolean">boolean</a>
+              </td>
+              <td><span class="default">false</span></td>
+            </tr>
          </table>
        </subsection>

--- a/suppressions.xml
+++ b/suppressions.xml
@ -8,4 +8,7 @@
    <suppress checks="FileLength"
              files="TokenTypes.java"
              lines="1"/>
+    <suppress checks="MagicNumber"
+              files="UnusedPrivateMethodCheck.java"
+              lines="176"/>
 </suppressions>