merging changes from v3 branch
This commit is contained in:
parent
6125beffc4
commit
1ac7fc3f8f
|
|
@ -146,14 +146,25 @@ checkstyle-user</a>).</li>
|
|||
<li class="body">Documentation error for naming convention checks
|
||||
(bug 987503).</li>
|
||||
|
||||
<li class="body">FinalParametersCheck checks parameters of
|
||||
abstract methods.
|
||||
(bug 1002849).</li>
|
||||
<li class="body">FinalParametersCheck checks parameters of
|
||||
abstract methods. (bug 1002849).</li>
|
||||
|
||||
<li>Bug in ClassResolver where it was mismatching imports -
|
||||
example would match SecurityDataException when looking for
|
||||
DataException. Bug was visible in RedundantThrows check (no
|
||||
known bug).</li>
|
||||
<li class="body">Bug in ClassResolver where it was
|
||||
mismatching imports - example would match
|
||||
SecurityDataException when looking for DataException. Bug was
|
||||
visible in RedundantThrows check (no known bug).</li>
|
||||
|
||||
<li class="body">Fixed HiddenFieldCheck to correctly handle
|
||||
static inner classes (bug 1032426).</li>
|
||||
|
||||
<li class="body">Fixed misspelling of variable (patch
|
||||
1032618 contibuted by Paul Wagland).</li>
|
||||
|
||||
<li class="body">Fixed ClassResolver to handle exceptions
|
||||
which are inner for current class (bug 945149).</li>
|
||||
|
||||
<li class="body">Fixed ClassResolver to handle
|
||||
fully-qualified inner classes (bug 1037667).</li>
|
||||
|
||||
</ul>
|
||||
|
||||
|
|
@ -1067,4 +1078,4 @@ checkstyle-user</a>).</li>
|
|||
<hr />
|
||||
<div><a href="index.html">Back to the Checkstyle Home Page</a></div>
|
||||
<p class="copyright">Copyright © 2002-2004 Oliver Burn. All rights Reserved.</p>
|
||||
</body> </html>
|
||||
</body> </html>
|
||||
|
|
|
|||
|
|
@ -43,6 +43,9 @@ public abstract class AbstractTypeAwareCheck
|
|||
/** full identifier for package of the method **/
|
||||
private FullIdent mPackageFullIdent;
|
||||
|
||||
/** Name of current class. */
|
||||
private String mCurrentClass;
|
||||
|
||||
/** <code>ClassResolver</code> instance for current tree. */
|
||||
private ClassResolver mClassResolver;
|
||||
|
||||
|
|
@ -59,6 +62,7 @@ public abstract class AbstractTypeAwareCheck
|
|||
mPackageFullIdent = FullIdent.createFullIdent(null);
|
||||
mImports.clear();
|
||||
mClassResolver = null;
|
||||
mCurrentClass = "";
|
||||
}
|
||||
|
||||
/** @see com.puppycrawl.tools.checkstyle.api.Check */
|
||||
|
|
@ -70,11 +74,28 @@ public abstract class AbstractTypeAwareCheck
|
|||
else if (aAST.getType() == TokenTypes.IMPORT) {
|
||||
processImport(aAST);
|
||||
}
|
||||
else if (aAST.getType() == TokenTypes.CLASS_DEF) {
|
||||
processClass(aAST);
|
||||
}
|
||||
else {
|
||||
processAST(aAST);
|
||||
}
|
||||
}
|
||||
|
||||
/** @see com.puppycrawl.tools.checkstyle.api.Check */
|
||||
public final void leaveToken(DetailAST aAST)
|
||||
{
|
||||
if (aAST.getType() == TokenTypes.CLASS_DEF) {
|
||||
int dotIdx = mCurrentClass.lastIndexOf(".");
|
||||
if (dotIdx == -1) {
|
||||
mCurrentClass = "";
|
||||
}
|
||||
else {
|
||||
mCurrentClass = mCurrentClass.substring(0, dotIdx);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculate if one type name is a shortname for another type name.
|
||||
* @param aShortName a shorthand, such as <code>IOException</code>
|
||||
|
|
@ -171,13 +192,15 @@ public abstract class AbstractTypeAwareCheck
|
|||
/**
|
||||
* Attempts to resolve the Class for a specified name.
|
||||
* @param aClassName name of the class to resolve
|
||||
* @param aCurrentClass name of surrounding class.
|
||||
* @return the resolved class or <code>null</code>
|
||||
* if unable to resolve the class.
|
||||
*/
|
||||
protected final Class resolveClass(String aClassName)
|
||||
protected final Class resolveClass(String aClassName,
|
||||
String aCurrentClass)
|
||||
{
|
||||
try {
|
||||
return getClassResolver().resolve(aClassName);
|
||||
return getClassResolver().resolve(aClassName, aCurrentClass);
|
||||
}
|
||||
catch (ClassNotFoundException e) {
|
||||
return null;
|
||||
|
|
@ -187,11 +210,13 @@ public abstract class AbstractTypeAwareCheck
|
|||
/**
|
||||
* Tries to load class. Logs error if unable.
|
||||
* @param aIdent name of class which we try to load.
|
||||
* @param aCurrentClass name of surrounding class.
|
||||
* @return <code>Class</code> for a ident.
|
||||
*/
|
||||
protected final Class tryLoadClass(FullIdent aIdent)
|
||||
protected final Class tryLoadClass(FullIdent aIdent,
|
||||
String aCurrentClass)
|
||||
{
|
||||
final Class clazz = resolveClass(aIdent.getText());
|
||||
final Class clazz = resolveClass(aIdent.getText(), aCurrentClass);
|
||||
if (clazz == null) {
|
||||
logLoadError(aIdent);
|
||||
}
|
||||
|
|
@ -228,6 +253,26 @@ public abstract class AbstractTypeAwareCheck
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Processes class definition.
|
||||
* @param aAST class defition to process.
|
||||
*/
|
||||
private void processClass(DetailAST aAST)
|
||||
{
|
||||
final DetailAST ident = aAST.findFirstToken(TokenTypes.IDENT);
|
||||
mCurrentClass += ("".equals(mCurrentClass) ? "" : "$")
|
||||
+ ident.getText();
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns current class.
|
||||
* @return name of current class.
|
||||
*/
|
||||
protected final String getCurrentClassName()
|
||||
{
|
||||
return mCurrentClass;
|
||||
}
|
||||
|
||||
/**
|
||||
* Contains class's <code>FullIdent</code>
|
||||
* and <code>Class</code> object if we can load it.
|
||||
|
|
@ -238,36 +283,24 @@ public abstract class AbstractTypeAwareCheck
|
|||
private FullIdent mName;
|
||||
/** <code>Class</code> object of this class if it's loadable. */
|
||||
private Class mClass;
|
||||
/** name of surrundeing class. */
|
||||
private String mCurrentClass;
|
||||
/** is class loadable. */
|
||||
private boolean mIsLoadable;
|
||||
|
||||
/**
|
||||
* Creates new instance of of class information object.
|
||||
* @param aName <code>FullIdent</code> associated with new object.
|
||||
* @param aClass <code>Class</code> associated with new object
|
||||
* or null id class is not loadable.
|
||||
* @param aCurrentClass name of current surrounding class.
|
||||
*/
|
||||
public ClassInfo(FullIdent aName, Class aClass)
|
||||
{
|
||||
if (aName == null && aClass == null) {
|
||||
throw new NullPointerException(
|
||||
"ClassInfo's name or class should be non-null");
|
||||
}
|
||||
mName = aName;
|
||||
setClazz(aClass);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates new instance of of class information object.
|
||||
* @param aName <code>FullIdent</code> associated with new object.
|
||||
*/
|
||||
public ClassInfo(FullIdent aName)
|
||||
public ClassInfo(FullIdent aName, String aCurrentClass)
|
||||
{
|
||||
if (aName == null) {
|
||||
throw new NullPointerException(
|
||||
"ClassInfo's name should be non-null");
|
||||
}
|
||||
mName = aName;
|
||||
mCurrentClass = aCurrentClass;
|
||||
mIsLoadable = true;
|
||||
}
|
||||
|
||||
|
|
@ -287,7 +320,8 @@ public abstract class AbstractTypeAwareCheck
|
|||
public final Class getClazz()
|
||||
{
|
||||
if (isLoadable() && mClass == null) {
|
||||
setClazz(AbstractTypeAwareCheck.this.tryLoadClass(getName()));
|
||||
setClazz(AbstractTypeAwareCheck.this.
|
||||
tryLoadClass(getName(), mCurrentClass));
|
||||
}
|
||||
return mClass;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -60,15 +60,26 @@ public class ClassResolver
|
|||
* - enclosing package
|
||||
* - star imports
|
||||
* @param aName name of the class to resolve
|
||||
* @param aCurrentClass name of current class (for inner classes).
|
||||
* @return the resolved class
|
||||
* @throws ClassNotFoundException if unable to resolve the class
|
||||
*/
|
||||
public Class resolve(String aName) throws ClassNotFoundException
|
||||
public Class resolve(String aName, String aCurrentClass)
|
||||
throws ClassNotFoundException
|
||||
{
|
||||
// See if the class is full qualified
|
||||
if (isLoadable(aName)) {
|
||||
return safeLoad(aName);
|
||||
}
|
||||
//Perhaps it's fullyqualified inner class
|
||||
int dotIdx = aName.lastIndexOf(".");
|
||||
if (dotIdx != -1) {
|
||||
final String cn = aName.substring(0, dotIdx) + "$"
|
||||
+ aName.substring(dotIdx + 1);
|
||||
if (isLoadable(cn)) {
|
||||
return safeLoad(cn);
|
||||
}
|
||||
}
|
||||
|
||||
// try matching explicit imports
|
||||
Iterator it = mImports.iterator();
|
||||
|
|
@ -103,6 +114,15 @@ public class ClassResolver
|
|||
}
|
||||
}
|
||||
|
||||
//inner class of this class???
|
||||
if (!"".equals(aCurrentClass)) {
|
||||
final String innerClass = ((mPkg != null) ? (mPkg + ".") : "")
|
||||
+ aCurrentClass + "$" + aName;
|
||||
if (isLoadable(innerClass)) {
|
||||
return safeLoad(innerClass);
|
||||
}
|
||||
}
|
||||
|
||||
// try "java.lang."
|
||||
final String langClass = "java.lang." + aName;
|
||||
if (isLoadable(langClass)) {
|
||||
|
|
|
|||
|
|
@ -19,8 +19,6 @@
|
|||
package com.puppycrawl.tools.checkstyle.checks.coding;
|
||||
|
||||
import java.util.HashSet;
|
||||
import java.util.ListIterator;
|
||||
import java.util.LinkedList;
|
||||
import java.util.Set;
|
||||
|
||||
import org.apache.commons.beanutils.ConversionException;
|
||||
|
|
@ -77,8 +75,9 @@ public class HiddenFieldCheck
|
|||
extends Check
|
||||
{
|
||||
/** stack of sets of field names,
|
||||
* one for each class of a set of nested classes */
|
||||
private LinkedList mFieldsStack = new LinkedList();
|
||||
* one for each class of a set of nested classes.
|
||||
*/
|
||||
private FieldFrame mCurrentFrame;
|
||||
|
||||
/** the regexp to match against */
|
||||
private RE mRegexp;
|
||||
|
|
@ -123,7 +122,7 @@ public class HiddenFieldCheck
|
|||
/** @see com.puppycrawl.tools.checkstyle.api.Check */
|
||||
public void beginTree(DetailAST aRootAST)
|
||||
{
|
||||
mFieldsStack.clear();
|
||||
mCurrentFrame = new FieldFrame(null, true);
|
||||
}
|
||||
|
||||
/** @see com.puppycrawl.tools.checkstyle.api.Check */
|
||||
|
|
@ -144,7 +143,8 @@ public class HiddenFieldCheck
|
|||
(typeMods == null)
|
||||
? false
|
||||
: typeMods.branchContains(TokenTypes.LITERAL_STATIC);
|
||||
final FieldFrame frame = new FieldFrame(isStaticInnerType);
|
||||
final FieldFrame frame =
|
||||
new FieldFrame(mCurrentFrame, isStaticInnerType);
|
||||
|
||||
//add fields to container
|
||||
final DetailAST objBlock =
|
||||
|
|
@ -168,7 +168,8 @@ public class HiddenFieldCheck
|
|||
child = (DetailAST) child.getNextSibling();
|
||||
}
|
||||
}
|
||||
mFieldsStack.addLast(frame); //push container
|
||||
// push container
|
||||
mCurrentFrame = frame;
|
||||
}
|
||||
else {
|
||||
//must be VARIABLE_DEF or PARAMETER_DEF
|
||||
|
|
@ -184,7 +185,7 @@ public class HiddenFieldCheck
|
|||
|| (aAST.getType() == TokenTypes.ENUM_CONSTANT_DEF))
|
||||
{
|
||||
//pop
|
||||
mFieldsStack.removeLast();
|
||||
mCurrentFrame = mCurrentFrame.getParent();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -204,21 +205,14 @@ public class HiddenFieldCheck
|
|||
final DetailAST nameAST = aAST.findFirstToken(TokenTypes.IDENT);
|
||||
final String name = nameAST.getText();
|
||||
boolean inStatic = inStatic(aAST);
|
||||
final ListIterator it =
|
||||
mFieldsStack.listIterator(mFieldsStack.size());
|
||||
while (it.hasPrevious()) {
|
||||
final FieldFrame frame = (FieldFrame) it.previous();
|
||||
inStatic |= frame.isStaticType();
|
||||
if ((frame.containsStaticField(name)
|
||||
|| (!inStatic && frame.containsInstanceField(name)))
|
||||
&& ((mRegexp == null) || (!getRegexp().match(name)))
|
||||
&& !isIgnoredSetterParam(aAST, name)
|
||||
&& !isIgnoredConstructorParam(aAST))
|
||||
{
|
||||
log(nameAST.getLineNo(), nameAST.getColumnNo(),
|
||||
"hidden.field", name);
|
||||
break;
|
||||
}
|
||||
if ((mCurrentFrame.containsStaticField(name)
|
||||
|| (!inStatic(aAST)
|
||||
&& mCurrentFrame.containsInstanceField(name)))
|
||||
&& ((mRegexp == null) || (!getRegexp().match(name)))
|
||||
&& !isIgnoredSetterParam(aAST, name)
|
||||
&& !isIgnoredConstructorParam(aAST))
|
||||
{
|
||||
log(nameAST, "hidden.field", name);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -362,6 +356,9 @@ public class HiddenFieldCheck
|
|||
/** is this a static inner type */
|
||||
private boolean mStaticType;
|
||||
|
||||
/** parent frame. */
|
||||
private FieldFrame mParent;
|
||||
|
||||
/** set of instance field names */
|
||||
private Set mInstanceFields = new HashSet();
|
||||
|
||||
|
|
@ -370,9 +367,11 @@ public class HiddenFieldCheck
|
|||
|
||||
/** Creates new frame.
|
||||
* @param aStaticType is this a static inner type (class or enum).
|
||||
* @param aParent parent frame.
|
||||
*/
|
||||
public FieldFrame(boolean aStaticType)
|
||||
public FieldFrame(FieldFrame aParent, boolean aStaticType)
|
||||
{
|
||||
mParent = aParent;
|
||||
mStaticType = aStaticType;
|
||||
}
|
||||
|
||||
|
|
@ -409,7 +408,14 @@ public class HiddenFieldCheck
|
|||
*/
|
||||
public boolean containsInstanceField(String aField)
|
||||
{
|
||||
return mInstanceFields.contains(aField);
|
||||
if (mInstanceFields.contains(aField)) {
|
||||
return true;
|
||||
}
|
||||
if (mStaticType) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return (mParent != null) && mParent.containsInstanceField(aField);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -419,7 +425,20 @@ public class HiddenFieldCheck
|
|||
*/
|
||||
public boolean containsStaticField(String aField)
|
||||
{
|
||||
return mStaticFields.contains(aField);
|
||||
if (mStaticFields.contains(aField)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return (mParent != null) && mParent.containsStaticField(aField);
|
||||
}
|
||||
|
||||
/**
|
||||
* Getter for parent frame.
|
||||
* @return parent frame.
|
||||
*/
|
||||
public FieldFrame getParent()
|
||||
{
|
||||
return mParent;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -83,6 +83,7 @@ public class RedundantThrowsCheck
|
|||
return new int[] {
|
||||
TokenTypes.PACKAGE_DEF,
|
||||
TokenTypes.IMPORT,
|
||||
TokenTypes.CLASS_DEF,
|
||||
TokenTypes.METHOD_DEF,
|
||||
TokenTypes.CTOR_DEF,
|
||||
};
|
||||
|
|
@ -136,7 +137,8 @@ public class RedundantThrowsCheck
|
|||
private void checkException(FullIdent aExc, List aKnownExcs)
|
||||
{
|
||||
// Let's try to load class.
|
||||
final ClassInfo newClassInfo = new ClassInfo(aExc);
|
||||
final ClassInfo newClassInfo =
|
||||
new ClassInfo(aExc, getCurrentClassName());
|
||||
|
||||
if (!mAllowUnchecked) {
|
||||
if (isUnchecked(newClassInfo.getClazz())) {
|
||||
|
|
|
|||
|
|
@ -254,6 +254,7 @@ public class JavadocMethodCheck
|
|||
return new int[] {
|
||||
TokenTypes.PACKAGE_DEF,
|
||||
TokenTypes.IMPORT,
|
||||
TokenTypes.CLASS_DEF,
|
||||
TokenTypes.METHOD_DEF,
|
||||
TokenTypes.CTOR_DEF,
|
||||
TokenTypes.ANNOTATION_FIELD_DEF,
|
||||
|
|
@ -276,6 +277,7 @@ public class JavadocMethodCheck
|
|||
return new int[] {
|
||||
TokenTypes.PACKAGE_DEF,
|
||||
TokenTypes.IMPORT,
|
||||
TokenTypes.CLASS_DEF,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -472,7 +474,8 @@ public class JavadocMethodCheck
|
|||
|| (child.getType() == TokenTypes.DOT))
|
||||
{
|
||||
final ExceptionInfo ei =
|
||||
new ExceptionInfo(FullIdent.createFullIdent(child));
|
||||
new ExceptionInfo(FullIdent.createFullIdent(child),
|
||||
getCurrentClassName());
|
||||
retVal.add(ei);
|
||||
}
|
||||
child = (DetailAST) child.getNextSibling();
|
||||
|
|
@ -673,7 +676,8 @@ public class JavadocMethodCheck
|
|||
*/
|
||||
private Class loadClassForTag(JavadocTag aTag)
|
||||
{
|
||||
Class clazz = resolveClass(aTag.getArg1());
|
||||
final String currentClassName = "";
|
||||
Class clazz = resolveClass(aTag.getArg1(), currentClassName);
|
||||
if (clazz == null) {
|
||||
log(aTag.getLineNo(), "javadoc.classInfo",
|
||||
"@throws", aTag.getArg1());
|
||||
|
|
@ -700,10 +704,11 @@ public class JavadocMethodCheck
|
|||
/**
|
||||
* Creates new instance for <code>FullIdent</code>.
|
||||
* @param aIdent <code>FullIdent</code> of the exception
|
||||
* @param aCurrentClass name of current class.
|
||||
*/
|
||||
ExceptionInfo(FullIdent aIdent)
|
||||
ExceptionInfo(FullIdent aIdent, String aCurrentClass)
|
||||
{
|
||||
super(aIdent);
|
||||
super(aIdent, aCurrentClass);
|
||||
}
|
||||
/** Mark that the exception has associated throws tag */
|
||||
final void setFound()
|
||||
|
|
|
|||
|
|
@ -19,30 +19,30 @@ public class ClassResolverTest
|
|||
null, imps);
|
||||
assertNotNull(cr);
|
||||
try {
|
||||
cr.resolve("who.will.win.the.world.cup");
|
||||
cr.resolve("who.will.win.the.world.cup", "");
|
||||
fail("Should not resolve class");
|
||||
}
|
||||
catch (ClassNotFoundException e) {
|
||||
}
|
||||
cr.resolve("java.lang.String");
|
||||
cr.resolve("StringBuffer");
|
||||
cr.resolve("AppletContext");
|
||||
cr.resolve("java.lang.String", "");
|
||||
cr.resolve("StringBuffer", "");
|
||||
cr.resolve("AppletContext", "");
|
||||
|
||||
try {
|
||||
cr.resolve("ChoiceFormat");
|
||||
cr.resolve("ChoiceFormat", "");
|
||||
fail();
|
||||
}
|
||||
catch (ClassNotFoundException e) {
|
||||
}
|
||||
|
||||
imps.add("java.text.ChoiceFormat");
|
||||
cr.resolve("ChoiceFormat");
|
||||
cr.resolve("ChoiceFormat", "");
|
||||
|
||||
cr = new ClassResolver(Thread.currentThread().getContextClassLoader(),
|
||||
"java.util", imps);
|
||||
cr.resolve("List");
|
||||
cr.resolve("List", "");
|
||||
try {
|
||||
cr.resolve("two.nil.england");
|
||||
cr.resolve("two.nil.england", "");
|
||||
fail();
|
||||
}
|
||||
catch (ClassNotFoundException e) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue