diff --git a/docs/releasenotes.html b/docs/releasenotes.html index 491a283a5..5099e8141 100644 --- a/docs/releasenotes.html +++ b/docs/releasenotes.html @@ -146,14 +146,25 @@ checkstyle-user).
  • Documentation error for naming convention checks (bug 987503).
  • -
  • FinalParametersCheck checks parameters of - abstract methods. - (bug 1002849).
  • +
  • FinalParametersCheck checks parameters of + abstract methods. (bug 1002849).
  • -
  • 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).
  • +
  • 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).
  • + +
  • Fixed HiddenFieldCheck to correctly handle + static inner classes (bug 1032426).
  • + +
  • Fixed misspelling of variable (patch + 1032618 contibuted by Paul Wagland).
  • + +
  • Fixed ClassResolver to handle exceptions + which are inner for current class (bug 945149).
  • + +
  • Fixed ClassResolver to handle + fully-qualified inner classes (bug 1037667).
  • @@ -1067,4 +1078,4 @@ checkstyle-user).
    Back to the Checkstyle Home Page
    - \ No newline at end of file + diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java index fa6991a36..e8313fc77 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java @@ -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; + /** ClassResolver 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 IOException @@ -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 null * 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 Class 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 FullIdent * and Class object if we can load it. @@ -238,36 +283,24 @@ public abstract class AbstractTypeAwareCheck private FullIdent mName; /** Class 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 FullIdent associated with new object. - * @param aClass Class 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 FullIdent 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; } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ClassResolver.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ClassResolver.java index adba082a8..f129b2955 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ClassResolver.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ClassResolver.java @@ -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)) { diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheck.java index 60a32053f..c25b5029f 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheck.java @@ -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; } } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java index bbd90e099..7e2cc9ea1 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java @@ -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())) { diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java index 184a37be8..941cf0874 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java @@ -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 FullIdent. * @param aIdent FullIdent 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() diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/ClassResolverTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/ClassResolverTest.java index a8c1b178a..56e38097c 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/ClassResolverTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/ClassResolverTest.java @@ -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) {