diff --git a/build.xml b/build.xml index f4842340c..5846db2bf 100644 --- a/build.xml +++ b/build.xml @@ -10,6 +10,7 @@ + + @@ -217,7 +219,7 @@ excludes="**/Generated*.java"/> - + diff --git a/docs/checkstyle.rules b/docs/checkstyle.rules index 338f8f92a..a24de7ca7 100644 --- a/docs/checkstyle.rules +++ b/docs/checkstyle.rules @@ -1,4 +1,5 @@ checkstyle.block.catch=text +checkstyle.fields.checkUnused=true checkstyle.javadoc.checkUnusedThrows=true checkstyle.lcurly.method=nl checkstyle.lcurly.other=nlow diff --git a/lib/bcel.jar b/lib/bcel.jar new file mode 100644 index 000000000..3195ae587 Binary files /dev/null and b/lib/bcel.jar differ diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java index 9bcd6df37..34feab212 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Configuration.java @@ -589,6 +589,12 @@ public class Configuration return getBooleanProperty(Defn.JAVADOC_CHECK_UNUSED_THROWS_PROP); } + /** @return whether to check unused fields **/ + boolean isCheckUnusedFields() + { + return getBooleanProperty(Defn.FIELDS_CHECK_UNUSED_PROP); + } + /** @return Set of pkg prefixes that are illegal in import statements */ Set getIllegalImports() { diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java index ed1c47597..4ed980a7a 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Defn.java @@ -125,6 +125,8 @@ public interface Defn String BASEDIR_PROP = "checkstyle.basedir"; /** property name for wrapping lines on operators **/ String WRAP_OP_PROP = "checkstyle.wrap.operator"; + /** property name for checking for unused fields **/ + String FIELDS_CHECK_UNUSED_PROP = "checkstyle.fields.checkUnused"; /** property name for the locale language for reporting **/ String LOCALE_LANGUAGE_PROP = "checkstyle.locale.language"; @@ -149,6 +151,7 @@ public interface Defn JAVADOC_CHECK_UNUSED_THROWS_PROP, REQUIRE_PACKAGE_HTML_PROP, REQUIRE_VERSION_PROP, + FIELDS_CHECK_UNUSED_PROP, }; /** All the properties that are a regulare expression */ diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/UnusedDetector.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/UnusedDetector.java new file mode 100644 index 000000000..81686634c --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/UnusedDetector.java @@ -0,0 +1,277 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 2.1 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +//////////////////////////////////////////////////////////////////////////////// +package com.puppycrawl.tools.checkstyle; + +import org.apache.bcel.classfile.JavaClass; +import org.apache.bcel.classfile.Code; +import org.apache.bcel.classfile.Field; +import org.apache.bcel.classfile.ConstantPool; +import org.apache.bcel.generic.InstructionHandle; +import org.apache.bcel.generic.InstructionList; +import org.apache.bcel.generic.ConstantPoolGen; +import org.apache.bcel.generic.GETFIELD; +import org.apache.bcel.generic.PUTFIELD; +import org.apache.bcel.generic.GETSTATIC; +import org.apache.bcel.generic.PUTSTATIC; +import org.apache.bcel.generic.FieldInstruction; + +import java.util.HashMap; +import java.util.Map; +import java.util.Iterator; +import java.util.ArrayList; + +/** + * Used to locate unused constructs using BCEL. + * A field is considered if: + *
    + *
  • It is private as another class can access it
  • + *
  • It is not final as the compiler can optimise these away
  • + *
+ * + * @author Oliver Burn + * @version 1.0 + */ +class UnusedDetector + extends org.apache.bcel.classfile.EmptyVisitor +{ + /** the class being examined */ + private final JavaClass mJavaClass; + /** the found field statistics */ + private final Map mFieldStats = new HashMap(); + /** a BCEL pool */ + private final ConstantPoolGen mPoolGen; + /** a BCEL pool */ + private final ConstantPool mPool; + /** used to process the instructions */ + private final InstructionVisitor mIV = new InstructionVisitor(); + + /** + * Creates a new UnusedDetector instance. + * + * @param aJavaClass a JavaClass value + */ + UnusedDetector(JavaClass aJavaClass) + { + mJavaClass = aJavaClass; + mPool = aJavaClass.getConstantPool(); + mPoolGen = new ConstantPoolGen(mPool); + } + + /** @return the names of all unusued fields */ + String[] getUnusedFields() + { + final ArrayList unused = new ArrayList(); + final Iterator it = mFieldStats.values().iterator(); + while (it.hasNext()) { + final FieldStats fs = (FieldStats) it.next(); + final Field details = fs.getDetails(); + if (details.isPrivate() + && !details.isFinal() + && !"this$0".equals(fs.getName())) + { + if (fs.getReads() == 0) { + unused.add(fs.getName()); + } + } + } + + return (String[]) unused.toArray(new String[unused.size()]); + } + + /////////////////////////////////////////////////////////////////////////// + // Classfile Visitor methods + /////////////////////////////////////////////////////////////////////////// + + /** @see org.apache.bcel.classfile.Visitor */ + public void visitField(Field aField) + { + reportField(aField); + } + + /** @see org.apache.bcel.classfile.Visitor */ + public void visitCode(Code aCode) + { + // Process all the instructions + final InstructionList list = new InstructionList(aCode.getCode()); + for (InstructionHandle handle = list.getStart(); + handle != null; + handle = handle.getNext()) + { + handle.getInstruction().accept(mIV); + } + } + + /////////////////////////////////////////////////////////////////////////// + // Private methods + /////////////////////////////////////////////////////////////////////////// + + /** + * @param aField report the field found + */ + private void reportField(Field aField) + { + final FieldStats fs = findFieldStats(aField.getName()); + fs.setDetails(aField); + } + + /** @param aFI report the field is read */ + private void reportRead(FieldInstruction aFI) + { + if (memberOfClass(aFI)) { + findFieldStats(aFI.getName(mPoolGen)).addRead(); + } + } + + /** @param aFI report the field is written to */ + private void reportWrite(FieldInstruction aFI) + { + if (memberOfClass(aFI)) { + findFieldStats(aFI.getName(mPoolGen)).addWrite(); + } + } + + /** + * @return whether the field is from this class + * @param aFI the field to check + */ + private boolean memberOfClass(FieldInstruction aFI) + { + return mJavaClass.getClassName().equals( + aFI.getClassType(mPoolGen).getClassName()); + } + + /** + * @return the FieldStats for the specifed name. If one does not exist, it + * is created + * @param aName the field name + */ + private FieldStats findFieldStats(String aName) + { + FieldStats retVal = (FieldStats) mFieldStats.get(aName); + if (retVal == null) { + retVal = new FieldStats(aName); + mFieldStats.put(aName, retVal); + } + return retVal; + } + + /////////////////////////////////////////////////////////////////////////// + // Internal types + /////////////////////////////////////////////////////////////////////////// + + /** + * Helper class for visiting all the Class instructions. Looks for access + * to fields. + * + */ + private class InstructionVisitor + extends org.apache.bcel.generic.EmptyVisitor + { + /** @see org.apache.bcel.generic.EmptyVisitor */ + public void visitGETFIELD(GETFIELD aField) + { + reportRead(aField); + } + + /** @see org.apache.bcel.generic.EmptyVisitor */ + public void visitPUTFIELD(PUTFIELD aField) + { + reportWrite(aField); + } + + /** @see org.apache.bcel.generic.EmptyVisitor */ + public void visitGETSTATIC(GETSTATIC aField) + { + reportRead(aField); + } + + /** @see org.apache.bcel.generic.EmptyVisitor */ + public void visitPUTSTATIC(PUTSTATIC aField) + { + reportWrite(aField); + } + } + + /** + * Represents the statistics for a field. + */ + private static class FieldStats + { + /** the name of the field */ + private final String mName; + /** number of reads */ + private int mReads = 0; + /** number of writes */ + private int mWrites = 0; + /** field details */ + private Field mDetails; + + /** + * Creates a new FieldStats instance. + * + * @param aName name of the field + */ + FieldStats(String aName) + { + mName = aName; + } + + /** @return the name of the field */ + String getName() + { + return mName; + } + + /** @return the number of field reads */ + int getReads() + { + return mReads; + } + + /** @return the number of field writes */ + int getWrites() + { + return mWrites; + } + + /** Add a field read */ + void addRead() + { + mReads++; + } + + /** Add a field write */ + void addWrite() + { + mWrites++; + } + + /** @return the field details */ + Field getDetails() + { + return mDetails; + } + + /** @param aDetails set the field detail */ + void setDetails(Field aDetails) + { + mDetails = aDetails; + } + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java index c18c47a7b..a534a44b7 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Verifier.java @@ -28,8 +28,14 @@ import java.util.ListIterator; import java.util.Map; import java.util.Set; import java.util.Stack; +import java.io.InputStream; +import java.io.IOException; + import org.apache.regexp.RE; import org.apache.regexp.RESyntaxException; +import org.apache.bcel.classfile.JavaClass; +import org.apache.bcel.classfile.DescendingVisitor; +import org.apache.bcel.classfile.ClassParser; /** * Verifier of Java rules. Each rule verifier takes the form of @@ -109,6 +115,14 @@ class Verifier /** stack tracking the visibility scope currently in **/ private final Stack mInScope = new Stack(); + /** stack for tracking the full class name of current scope */ + private final Stack mTypeNames = new Stack(); + /** represents the current type name */ + private String mCurrentTypeName; + + /** Map of type names to a map of variables that are indexed on name */ + private final Map mTypeFieldsMap = new HashMap(); + /** tracks the level of block definitions for methods **/ private int mMethodBlockLevel = 0; @@ -165,6 +179,7 @@ class Verifier **/ LocalizedMessage[] getMessages() { + checkUnused(); checkImports(); return mMessages.getMessages(); } @@ -180,6 +195,8 @@ class Verifier mComments.clear(); mImports.clear(); mReferenced.clear(); + mTypeNames.clear(); + mTypeFieldsMap.clear(); mMethodBlockLevel = 0; } @@ -409,6 +426,14 @@ class Verifier mConfig.getStaticFinalPat()); } else { + // Record the name of the variable for detection of unused + Map typeVars = (Map) mTypeFieldsMap.get(mCurrentTypeName); + if (typeVars == null) { + typeVars = new HashMap(); + mTypeFieldsMap.put(mCurrentTypeName, typeVars); + } + typeVars.put(aVar.getText(), aVar); + /////////////////////////////////////////////////////////////////// // THIS BLOCK NEEDS REFACTORING!! /////////////////////////////////////////////////////////////////// @@ -894,19 +919,33 @@ class Verifier * to the reportEndBlock(). * @param aScope the Scope of the type block * @param aIsInterface indicates if the block is for an interface + * @param aType the name of the type */ - void reportStartTypeBlock(Scope aScope, boolean aIsInterface) + void reportStartTypeBlock(Scope aScope, + boolean aIsInterface, + MyCommonAST aType) { mInScope.push(aScope); mInInterface.push(aIsInterface ? Boolean.TRUE : Boolean.FALSE); + if (aType != null) { + mTypeNames.push(aType.getText()); + calculateTypeName(); + } } - /** Report that the parser is leaving a type block. **/ - void reportEndTypeBlock() + /** + * Report that the parser is leaving a type block. + * @param aNamed is this a named type block + */ + void reportEndTypeBlock(boolean aNamed) { mInScope.pop(); mInInterface.pop(); + if (aNamed) { + mTypeNames.pop(); + calculateTypeName(); + } } @@ -1437,6 +1476,59 @@ class Verifier return (i == -1) ? aType : aType.substring(i + 1); } + /** + * Checks for unused variables using BCEL. + */ + private void checkUnused() + { + if (!mConfig.isCheckUnusedFields()) { + return; + } + + final Iterator typeNames = mTypeFieldsMap.keySet().iterator(); + while (typeNames.hasNext()) { + final String type = (String) typeNames.next(); + String cname = type.replace('.', '$'); + if (mPkgName != null) { + cname = mPkgName + "." + cname; + } + final String fname = cname.replace('.', '/') + ".class"; + final InputStream is = + mConfig.getClassLoader().getResourceAsStream(fname); + + final ClassParser parser = new ClassParser(is, cname); + JavaClass jc = null; + try { + jc = parser.parse(); + } + catch (IOException e) { + e.printStackTrace(); + return; + } + catch (ClassFormatError e) { + e.printStackTrace(); + return; + } + + // Visit the code using BCEL + final UnusedDetector ud = new UnusedDetector(jc); + final DescendingVisitor dv = new DescendingVisitor(jc, ud); + dv.visit(); + + // Report all unused fields + final Map typeVars = (Map) mTypeFieldsMap.get(type); + final String[] unusedFields = ud.getUnusedFields(); + for (int i = 0; i < unusedFields.length; i++) { + final MyVariable var = + (MyVariable) typeVars.get(unusedFields[i]); + mMessages.add(var.getLineNo(), + var.getColumnNo() - 1, + "field.unused", + var.getText()); + } + } + } + /** Check the imports that are unused or unrequired. **/ private void checkImports() { @@ -1759,5 +1851,19 @@ class Verifier } } } + + /** Calcuates the current type name */ + private void calculateTypeName() + { + mCurrentTypeName = ""; + final Iterator it = mTypeNames.iterator(); + if (it.hasNext()) { + mCurrentTypeName = (String) it.next(); + } + + while (it.hasNext()) { + mCurrentTypeName += "." + (String) it.next(); + } + } // }}} } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g b/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g index e850df630..f75bd505c 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/java.g @@ -239,13 +239,13 @@ classDefinition![MyCommonAST modifiers, MyModifierSet modSet] // now parse the body of the class { ver.verifyType(modSet, #IDENT); - ver.reportStartTypeBlock(modSet.getVisibilityScope(), false); + ver.reportStartTypeBlock(modSet.getVisibilityScope(), false, #IDENT); } cb:classBlock[(modSet.size() == 0) ? #cc.getLineNo() : modSet.getFirstLineNo()] {#classDefinition = #(#[CLASS_DEF,"CLASS_DEF"], modifiers,IDENT,sc,ic,cb);} { - ver.reportEndTypeBlock(); + ver.reportEndTypeBlock(true); } ; @@ -262,13 +262,13 @@ interfaceDefinition![MyCommonAST modifiers, MyModifierSet modSet] // now parse the body of the interface (looks like a class...) { ver.verifyType(modSet, #IDENT); - ver.reportStartTypeBlock(modSet.getVisibilityScope(), true); + ver.reportStartTypeBlock(modSet.getVisibilityScope(), true, #IDENT); } cb:classBlock[(modSet.size() == 0) ? #ii.getLineNo() : modSet.getFirstLineNo()] {#interfaceDefinition = #(#[INTERFACE_DEF,"INTERFACE_DEF"], modifiers,IDENT,ie,cb);} { - ver.reportEndTypeBlock(); + ver.reportEndTypeBlock(true); } ; @@ -1067,7 +1067,7 @@ primaryExpression */ newExpression : n:"new"^ t:type {ver.reportInstantiation(#n, sLastIdentifier);} - ( LPAREN! argList RPAREN! ({ver.reportStartTypeBlock(Scope.ANONINNER, false);} classBlock[-1] {ver.reportEndTypeBlock();})? + ( LPAREN! argList RPAREN! ({ver.reportStartTypeBlock(Scope.ANONINNER, false, null);} classBlock[-1] {ver.reportEndTypeBlock(false);})? //java 1.1 // Note: This will allow bad constructs like diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/messages.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/messages.properties index 5619972aa..8deb6ba58 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/messages.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/messages.properties @@ -70,3 +70,5 @@ block.noStmt=Must have at least one statement. block.empty=Empty {0} block. maxParam=More than {0,number,integer} parameters. + +field.unused=''{0}'' is an unused field. \ No newline at end of file