Yahoo! Implemented checking for unused private fields. Just need to document
the new feature. It is off by default as the class path must be set up. Can use the same technique for private methods.
This commit is contained in:
Oliver Burn
parent
98c8a0db55
commit
7f06106f4e
|
|
@ -10,6 +10,7 @@
|
|||
<property name="antlr.jar" value="lib/antlr.jar" />
|
||||
<property name="antlr-tools.jar" value="lib/antlr-tools.jar" />
|
||||
<property name="regexp.jar" value="lib/jakarta-regexp-1.2.jar" />
|
||||
<property name="bcel.jar" value="lib/bcel.jar" />
|
||||
<property name="junit.jar" value="lib/junit.jar" />
|
||||
|
||||
<property name="checkstyle.dir"
|
||||
|
|
@ -22,6 +23,7 @@
|
|||
<path id="build.classpath">
|
||||
<pathelement location="${antlr.jar}" />
|
||||
<pathelement location="${regexp.jar}" />
|
||||
<pathelement location="${bcel.jar}" />
|
||||
<pathelement location="${ant.home}/lib/ant.jar" />
|
||||
</path>
|
||||
|
||||
|
|
@ -217,7 +219,7 @@
|
|||
excludes="**/Generated*.java"/>
|
||||
<formatter type="plain"/>
|
||||
<formatter type="xml" toFile="target/cs_errors.xml"/>
|
||||
<classpath refid="build.classpath"/>
|
||||
<classpath refid="run.classpath"/>
|
||||
<property key="checkstyle.cache.file" file="target/cachefile"/>
|
||||
<property key="checkstyle.header.file" file="docs/java.header"/>
|
||||
</checkstyle>
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
checkstyle.block.catch=text
|
||||
checkstyle.fields.checkUnused=true
|
||||
checkstyle.javadoc.checkUnusedThrows=true
|
||||
checkstyle.lcurly.method=nl
|
||||
checkstyle.lcurly.other=nlow
|
||||
|
|
|
|||
Binary file not shown.
|
|
@ -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()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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 */
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
* <ul>
|
||||
* <li>It is private as another class can access it</li>
|
||||
* <li>It is not final as the compiler can optimise these away</li>
|
||||
* </ul>
|
||||
*
|
||||
* @author <a href="mailto:checkstyle@puppycrawl.com">Oliver Burn</a>
|
||||
* @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 <code>UnusedDetector</code> instance.
|
||||
*
|
||||
* @param aJavaClass a <code>JavaClass</code> 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 <code>FieldStats</code> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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();
|
||||
}
|
||||
}
|
||||
// }}}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue