From 621d9a9bf7fa8a0743dc6080c6f684a2e6b0abdd Mon Sep 17 00:00:00 2001 From: Oleg Sukhodolsky Date: Mon, 31 Jan 2005 13:53:22 +0000 Subject: [PATCH] UnusedPrivateMethod check should allow serialization-related methods (rfe 1036387) --- .../usage/UnusedPrivateMethodCheck.java | 185 +++++++++++++++++- .../checkstyle/usage/InputUnusedMethod.java | 90 +++++++++ .../usage/UnusedPrivateMethodCheckTest.java | 44 +++++ src/xdocs/config_usage.xml | 12 ++ suppressions.xml | 3 + 5 files changed, 332 insertions(+), 2 deletions(-) diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheck.java index 06cd30fe3..f7b4fbf32 100644 --- 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; } } diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/usage/InputUnusedMethod.java b/src/testinputs/com/puppycrawl/tools/checkstyle/usage/InputUnusedMethod.java index 96d679f72..78037508c 100644 --- 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(); + } +} diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/usage/UnusedPrivateMethodCheckTest.java index 3e53e141f..b2e5cd32c 100644 --- 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); } diff --git a/src/xdocs/config_usage.xml b/src/xdocs/config_usage.xml index 3338a674b..fafc9eef7 100755 --- a/src/xdocs/config_usage.xml +++ b/src/xdocs/config_usage.xml @@ -299,6 +299,18 @@ ^$ (empty) + + allowSerializationMethods + whether the check should allow serialization-related methods + (readObject(), writeObject(), readResolve() and writeReplace() + + boolean + + false + diff --git a/suppressions.xml b/suppressions.xml index cbfbe82f3..fe85ab759 100755 --- a/suppressions.xml +++ b/suppressions.xml @@ -8,4 +8,7 @@ +