diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java index c3d569ebe..c3b0dc8e6 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/TreeWalker.java @@ -48,6 +48,9 @@ import com.puppycrawl.tools.checkstyle.api.Utils; import com.puppycrawl.tools.checkstyle.grammars.GeneratedJavaRecognizer; import com.puppycrawl.tools.checkstyle.grammars.GeneratedJavaLexer; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + /** * Responsible for walking an abstract syntax tree and notifying interested * checks at each each node. @@ -125,11 +128,15 @@ public final class TreeWalker /** a factory for creating submodules (i.e. the Checks) */ private ModuleFactory mModuleFactory; - /** contrals whether we should use recursive or iterative + /** controls whether we should use recursive or iterative * algorithm for tree processing. */ private final boolean mRecursive; + /** logger for debug purpose */ + private static Log sLog = + LogFactory.getLog("com.puppycrawl.tools.checkstyle.TreeWalker"); + /** * Creates a new TreeWalker instance. */ @@ -143,12 +150,10 @@ public final class TreeWalker System.getProperty("checkstyle.use.recursive.algorithm", "true"); mRecursive = "true".equals(recursive); if (mRecursive) { - Utils.getExceptionLogger() - .debug("TreeWalker uses recursive algorithm"); + sLog.debug("TreeWalker uses recursive algorithm"); } else { - Utils.getExceptionLogger() - .debug("TreeWalker uses iterative algorithm"); + sLog.debug("TreeWalker uses iterative algorithm"); } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java index cf9f3c097..701dd2531 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java @@ -25,7 +25,10 @@ import com.puppycrawl.tools.checkstyle.api.LocalizedMessage; import com.puppycrawl.tools.checkstyle.api.TokenTypes; import com.puppycrawl.tools.checkstyle.api.Utils; import java.util.HashSet; +import java.util.HashMap; +import java.util.Map; import java.util.Set; +import java.util.Vector; /** * Abstract class that endeavours to maintain type information for the Java @@ -35,8 +38,7 @@ import java.util.Set; * @author Oliver Burn * @version 1.0 */ -public abstract class AbstractTypeAwareCheck - extends Check +public abstract class AbstractTypeAwareCheck extends Check { /** imports details **/ private Set mImports = new HashSet(); @@ -50,6 +52,9 @@ public abstract class AbstractTypeAwareCheck /** ClassResolver instance for current tree. */ private ClassResolver mClassResolver; + /** Stack of maps for type params. */ + private Vector mTypeParams = new Vector(); + /** * Called to process an AST when visiting it. * @param aAST the AST to process. Guaranteed to not be PACKAGE_DEF or @@ -57,6 +62,17 @@ public abstract class AbstractTypeAwareCheck */ protected abstract void processAST(DetailAST aAST); + /** @see com.puppycrawl.tools.checkstyle.api.Check */ + public final int[] getRequiredTokens() + { + return new int[] { + TokenTypes.PACKAGE_DEF, + TokenTypes.IMPORT, + TokenTypes.CLASS_DEF, + TokenTypes.ENUM_DEF, + }; + } + /** @see com.puppycrawl.tools.checkstyle.api.Check */ public void beginTree(DetailAST aRootAST) { @@ -66,6 +82,7 @@ public abstract class AbstractTypeAwareCheck mImports.add("java.lang.*"); mClassResolver = null; mCurrentClass = ""; + mTypeParams.clear(); } /** @see com.puppycrawl.tools.checkstyle.api.Check */ @@ -83,6 +100,9 @@ public abstract class AbstractTypeAwareCheck processClass(aAST); } else { + if (aAST.getType() == TokenTypes.METHOD_DEF) { + processTypeParams(aAST); + } processAST(aAST); } } @@ -106,6 +126,10 @@ public abstract class AbstractTypeAwareCheck else { mCurrentClass = mCurrentClass.substring(0, dotIdx); } + mTypeParams.remove(mTypeParams.size() - 1); + } + else if (aAST.getType() == TokenTypes.METHOD_DEF) { + mTypeParams.remove(mTypeParams.size() - 1); } else if (aAST.getType() != TokenTypes.PACKAGE_DEF && aAST.getType() != TokenTypes.IMPORT) @@ -241,8 +265,7 @@ public abstract class AbstractTypeAwareCheck * @param aCurrentClass name of surrounding class. * @return Class for a ident. */ - protected final Class tryLoadClass(FullIdent aIdent, - String aCurrentClass) + protected final Class tryLoadClass(Token aIdent, String aCurrentClass) { final Class clazz = resolveClass(aIdent.getText(), aCurrentClass); if (clazz == null) { @@ -256,7 +279,7 @@ public abstract class AbstractTypeAwareCheck * Abstract, should be overrided in subclasses. * @param aIdent class name for which we can no load class. */ - protected abstract void logLoadError(FullIdent aIdent); + protected abstract void logLoadError(Token aIdent); /** * Common implementation for logLoadError() method. @@ -300,15 +323,52 @@ public abstract class AbstractTypeAwareCheck } } + /** + * Process type params (if any) for given class, enum or method. + * @param aAST class, enum or method to process. + */ + private void processTypeParams(DetailAST aAST) + { + final DetailAST typeParams = + aAST.findFirstToken(TokenTypes.TYPE_PARAMETERS); + + Map paramsMap = new HashMap(); + mTypeParams.add(paramsMap); + + if (typeParams == null) { + return; + } + + for (DetailAST child = (DetailAST) typeParams.getFirstChild(); + child != null; + child = (DetailAST) child.getNextSibling()) + { + if (child.getType() == TokenTypes.TYPE_PARAMETER) { + DetailAST param = child; + String alias = param.findFirstToken(TokenTypes.IDENT).getText(); + DetailAST bounds = + param.findFirstToken(TokenTypes.TYPE_UPPER_BOUNDS); + if (bounds != null) { + FullIdent name = FullIdent.createFullIdentBelow(bounds); + ClassInfo ci = + createClassInfo(new Token(name), getCurrentClassName()); + paramsMap.put(alias, ci); + } + } + } + } + /** * Processes class definition. - * @param aAST class defition to process. + * @param aAST class definition to process. */ private void processClass(DetailAST aAST) { final DetailAST ident = aAST.findFirstToken(TokenTypes.IDENT); mCurrentClass += ("".equals(mCurrentClass) ? "" : "$") + ident.getText(); + + processTypeParams(aAST); } /** @@ -321,54 +381,107 @@ public abstract class AbstractTypeAwareCheck } /** - * Contains class's FullIdent - * and Class object if we can load it. + * Creates class info for given name. + * @param aName name of type. + * @param aSurroundingClass name of surrounding class. + * @return class infor for given name. */ - protected class ClassInfo + protected final ClassInfo createClassInfo(final Token aName, + final String aSurroundingClass) + { + ClassInfo ci = findClassAlias(aName.getText()); + if (ci != null) { + return new ClassAlias(aName, ci); + } + return new RegularClass(aName, aSurroundingClass, this); + } + + /** + * Looking if a given name is alias. + * @param aName given name + * @return ClassInfo for alias if it exists, null otherwise + */ + protected final ClassInfo findClassAlias(final String aName) + { + ClassInfo ci = null; + for (int i = mTypeParams.size() - 1; i >= 0; i--) { + Map paramMap = (Map) mTypeParams.get(i); + ci = (ClassInfo) paramMap.get(aName); + if (ci != null) { + break; + } + } + return ci; + } + + /** + * Contains class's Token. + */ + protected abstract static class ClassInfo { /** FullIdent associated with this class. */ - private FullIdent mName; - /** Class object of this class if it's loadable. */ - private Class mClass; - /** name of surrounding class. */ - private String mSurroundingClass; - /** is class loadable. */ - private boolean mIsLoadable; + private final Token mName; + + /** @return class name */ + public final Token getName() + { + return mName; + } + + /** @return Class associated with an object. */ + public abstract Class getClazz(); /** - * Creates new instance of of class information object. - * @param aName FullIdent associated with new object. - * @param aSurroundingClass name of current surrounding class. + * Creates new instance of class inforamtion object. + * @param aName token which represents class name. */ - public ClassInfo(final FullIdent aName, final String aSurroundingClass) + protected ClassInfo(final Token aName) { if (aName == null) { throw new NullPointerException( "ClassInfo's name should be non-null"); } mName = aName; - mSurroundingClass = aSurroundingClass; - mIsLoadable = true; } + } - /** @return class name */ - public final FullIdent getName() + /** Represents regular classes/enumes. */ + private static final class RegularClass extends ClassInfo + { + /** name of surrounding class. */ + private String mSurroundingClass; + /** is class loadable. */ + private boolean mIsLoadable = true; + /** Class object of this class if it's loadable. */ + private Class mClass; + /** the check we use to resolve classes. */ + private final AbstractTypeAwareCheck mCheck; + + /** + * Creates new instance of of class information object. + * @param aName FullIdent associated with new object. + * @param aSurroundingClass name of current surrounding class. + * @param aCheck the check we use to load class. + */ + private RegularClass(final Token aName, + final String aSurroundingClass, + final AbstractTypeAwareCheck aCheck) { - return mName; + super(aName); + mSurroundingClass = aSurroundingClass; + mCheck = aCheck; } - /** @return if class is loadable ot not. */ - public final boolean isLoadable() + private boolean isLoadable() { return mIsLoadable; } /** @return Class associated with an object. */ - public final Class getClazz() + public Class getClazz() { if (isLoadable() && mClass == null) { - setClazz(AbstractTypeAwareCheck.this. - tryLoadClass(getName(), mSurroundingClass)); + setClazz(mCheck.tryLoadClass(getName(), mSurroundingClass)); } return mClass; } @@ -377,10 +490,98 @@ public abstract class AbstractTypeAwareCheck * Associates Class with an object. * @param aClass Class to associate with. */ - public final void setClazz(Class aClass) + private void setClazz(Class aClass) { mClass = aClass; mIsLoadable = (mClass != null); } + + /** {@inheritDoc} */ + public String toString() + { + return "RegularClass[name=" + getName() + + ", in class=" + mSurroundingClass + + ", loadable=" + mIsLoadable + + ", class=" + mClass + "]"; + } + } + + /** Represents type param which is "alias" for real type. */ + private static class ClassAlias extends ClassInfo + { + /** Class information associated with the alias. */ + private final ClassInfo mClassInfo; + + /** + * Creates nnew instance of the class. + * @param aName token which represents name of class alias. + * @param aClassInfo class information associated with the alias. + */ + ClassAlias(final Token aName, ClassInfo aClassInfo) + { + super(aName); + mClassInfo = aClassInfo; + } + + /** {@inheritDoc} */ + public final Class getClazz() + { + return mClassInfo.getClazz(); + } + } + + /** + * Represents text element with location in the text. + */ + protected static class Token + { + /** token's column number. */ + private final int mColumn; + /** token's line number. */ + private final int mLine; + /** token's text. */ + private final String mText; + + /** + * Creates token. + * @param aText token's text + * @param aLine token's line number + * @param aColumn token's column number + */ + public Token(String aText, int aLine, int aColumn) + { + mText = aText; + mLine = aLine; + mColumn = aColumn; + } + + /** + * Converts FullIdent to Token. + * @param aFullIdent full ident to convert. + */ + public Token(FullIdent aFullIdent) + { + mText = aFullIdent.getText(); + mLine = aFullIdent.getLineNo(); + mColumn = aFullIdent.getColumnNo(); + } + + /** @return line number of the token */ + public int getLineNo() + { + return mLine; + } + + /** @return column number of the token */ + public int getColumnNo() + { + return mColumn; + } + + /** @return text of the token */ + public String getText() + { + return mText; + } } } 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 602e23f25..a396cebfd 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java @@ -116,10 +116,9 @@ public class RedundantThrowsCheck extends AbstractTypeAwareCheck * Logs error if unable to load class information. * @param aIdent class name for which we can no load class. */ - protected final void logLoadError(FullIdent aIdent) + protected final void logLoadError(Token aIdent) { - logLoadErrorImpl(aIdent.getLineNo(), - aIdent.getColumnNo(), + logLoadErrorImpl(aIdent.getLineNo(), aIdent.getColumnNo(), "redundant.throws.classInfo", new Object[] {aIdent.getText()}); } @@ -140,7 +139,7 @@ public class RedundantThrowsCheck extends AbstractTypeAwareCheck { // Let's try to load class. final ClassInfo newClassInfo = - new ClassInfo(aExc, getCurrentClassName()); + createClassInfo(new Token(aExc), getCurrentClassName()); if (!mAllowUnchecked) { if (isUnchecked(newClassInfo.getClazz())) { @@ -152,7 +151,7 @@ public class RedundantThrowsCheck extends AbstractTypeAwareCheck boolean shouldAdd = true; for (Iterator known = aKnownExcs.iterator(); known.hasNext();) { final ClassInfo ci = (ClassInfo) known.next(); - final FullIdent fi = ci.getName(); + final Token fi = ci.getName(); if (isSameType(fi.getText(), aExc.getText())) { shouldAdd = false; 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 a6dfdc341..d6a8c3478 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java @@ -267,14 +267,6 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck TokenTypes.ANNOTATION_FIELD_DEF, }; } - /** @see com.puppycrawl.tools.checkstyle.api.Check */ - public int[] getRequiredTokens() - { - return new int[] { - TokenTypes.PACKAGE_DEF, TokenTypes.IMPORT, - TokenTypes.CLASS_DEF, TokenTypes.ENUM_DEF, }; - } - /** * Checks Javadoc comments for a method or constructor. * @@ -305,7 +297,7 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck * * @param aIdent class name for which we can no load class. */ - protected final void logLoadError(FullIdent aIdent) + protected final void logLoadError(Token aIdent) { logLoadErrorImpl(aIdent.getLineNo(), aIdent.getColumnNo(), "javadoc.classInfo", @@ -400,21 +392,37 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck Matcher noargMultilineStart = MATCH_JAVADOC_NOARG_MULTILINE_START.matcher(lines[i]); if (javadocArgMatcher.find()) { - tags.add(new JavadocTag(currentLine, + int col = javadocArgMatcher.start(1) - 1; + if (i == 0) { + col += aComment.getStartColNo(); + } + tags.add(new JavadocTag(currentLine, col, javadocArgMatcher.group(1), javadocArgMatcher.group(2))); } else if (javadocNoargMatcher.find()) { - tags.add(new JavadocTag(currentLine, + int col = javadocNoargMatcher.start(1) - 1; + if (i == 0) { + col += aComment.getStartColNo(); + } + tags.add(new JavadocTag(currentLine, col, javadocNoargMatcher.group(1))); } else if (noargCurlyMatcher.find()) { - tags.add(new JavadocTag(currentLine, + int col = noargCurlyMatcher.start(1) - 1; + if (i == 0) { + col += aComment.getStartColNo(); + } + tags.add(new JavadocTag(currentLine, col, noargCurlyMatcher.group(1))); } else if (argMultilineStart.find()) { final String p1 = argMultilineStart.group(1); final String p2 = argMultilineStart.group(2); + int col = argMultilineStart.start(1) - 1; + if (i == 0) { + col += aComment.getStartColNo(); + } // Look for the rest of the comment if all we saw was // the tag and the name. Stop when we see '*/' (end of @@ -429,7 +437,7 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck String lFin = multilineCont.group(1); if (!lFin.equals(NEXT_TAG) && !lFin.equals(END_JAVADOC)) { - tags.add(new JavadocTag(currentLine, p1, p2)); + tags.add(new JavadocTag(currentLine, col, p1, p2)); } } remIndex++; @@ -437,6 +445,10 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck } else if (noargMultilineStart.find()) { final String p1 = noargMultilineStart.group(1); + int col = noargMultilineStart.start(1) - 1; + if (i == 0) { + col += aComment.getStartColNo(); + } // Look for the rest of the comment if all we saw was // the tag and the name. Stop when we see '*/' (end of @@ -451,7 +463,7 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck String lFin = multilineCont.group(1); if (!lFin.equals(NEXT_TAG) && !lFin.equals(END_JAVADOC)) { - tags.add(new JavadocTag(currentLine, p1)); + tags.add(new JavadocTag(currentLine, col, p1)); } } remIndex++; @@ -500,9 +512,9 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck if ((child.getType() == TokenTypes.IDENT) || (child.getType() == TokenTypes.DOT)) { + FullIdent fi = FullIdent.createFullIdent(child); final ExceptionInfo ei = - new ExceptionInfo(FullIdent.createFullIdent(child), - getCurrentClassName()); + new ExceptionInfo(new Token(fi), getCurrentClassName()); retVal.add(ei); } child = (DetailAST) child.getNextSibling(); @@ -568,8 +580,8 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck // Handle extra JavadocTag if (!found) { - log(tag.getLineNo(), "javadoc.unusedTag", "@param", tag - .getArg1()); + log(tag.getLineNo(), tag.getColumnNo(), + "javadoc.unusedTag", "@param", tag.getArg1()); } } @@ -633,7 +645,8 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck final JavadocTag jt = (JavadocTag) it.next(); if (jt.isReturnTag()) { if (found) { - log(jt.getLineNo(), "javadoc.return.duplicate"); + log(jt.getLineNo(), jt.getColumnNo(), + "javadoc.return.duplicate"); } found = true; it.remove(); @@ -672,14 +685,16 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck // Loop looking for matching throw final String documentedEx = tag.getArg1(); + Token token = new Token(tag.getArg1(), tag.getLineNo(), + tag.getColumnNo()); + ClassInfo documentedCI = + createClassInfo(token, getCurrentClassName()); boolean found = foundThrows.contains(documentedEx); - Class documentedClass = null; - boolean classLoaded = false; final ListIterator throwIt = aThrows.listIterator(); while (!found && throwIt.hasNext()) { final ExceptionInfo ei = (ExceptionInfo) throwIt.next(); - final FullIdent fi = ei.getName(); + final Token fi = ei.getName(); final String declaredEx = fi.getText(); if (isSameType(declaredEx, documentedEx)) { @@ -688,11 +703,7 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck foundThrows.add(documentedEx); } else if (mAllowThrowsTagsForSubclasses) { - if (!classLoaded) { - documentedClass = loadClassForTag(tag); - classLoaded = true; - } - found = isSubclass(documentedClass, ei.getClazz()); + found = isSubclass(documentedCI.getClazz(), ei.getClazz()); } } @@ -700,16 +711,13 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck if (!found) { boolean reqd = true; if (mAllowUndeclaredRTE) { - if (!classLoaded) { - documentedClass = loadClassForTag(tag); - classLoaded = true; - } - reqd = !isUnchecked(documentedClass); + reqd = !isUnchecked(documentedCI.getClazz()); } if (reqd) { - log(tag.getLineNo(), "javadoc.unusedTag", "@throws", tag - .getArg1()); + log(tag.getLineNo(), tag.getColumnNo(), + "javadoc.unusedTag", "@throws", tag.getArg1()); + } } } @@ -721,7 +729,7 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck while (throwIt.hasNext()) { final ExceptionInfo ei = (ExceptionInfo) throwIt.next(); if (!ei.isFound()) { - final FullIdent fi = ei.getName(); + final Token fi = ei.getName(); log(fi.getLineNo(), fi.getColumnNo(), "javadoc.expectedTag", "@throws", fi.getText()); } @@ -729,22 +737,6 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck } } - /** - * Tries to load class for throws tag. Logs error if unable. - * - * @param aTag name of class which we try to load. - * @return Class for the tag. - */ - private Class loadClassForTag(JavadocTag aTag) - { - Class clazz = resolveClass(aTag.getArg1(), getCurrentClassName()); - if (clazz == null) { - log(aTag.getLineNo(), "javadoc.classInfo", "@throws", aTag - .getArg1()); - } - return clazz; - } - /** * Returns whether an AST represents a setter method. * @param aAST the AST to check with @@ -858,20 +850,22 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck } /** Stores useful information about declared exception. */ - class ExceptionInfo extends ClassInfo + private class ExceptionInfo { /** does the exception have throws tag associated with. */ private boolean mFound; + /** class information associated with this exception. */ + private ClassInfo mClassInfo; /** * Creates new instance for FullIdent. * - * @param aIdent FullIdent of the exception + * @param aIdent the exception * @param aCurrentClass name of current class. */ - ExceptionInfo(FullIdent aIdent, String aCurrentClass) + ExceptionInfo(Token aIdent, String aCurrentClass) { - super(aIdent, aCurrentClass); + mClassInfo = createClassInfo(aIdent, aCurrentClass); } /** Mark that the exception has associated throws tag */ @@ -885,5 +879,17 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck { return mFound; } + + /** @return exception's name */ + final Token getName() + { + return mClassInfo.getName(); + } + + /** @return class for this exception */ + final Class getClazz() + { + return mClassInfo.getClazz(); + } } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTag.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTag.java index 6d8d3dfbc..ce60eff9b 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTag.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTag.java @@ -26,6 +26,8 @@ class JavadocTag { /** the line number of the tag **/ private final int mLineNo; + /** the column number of the tag **/ + private int mColumnNo; /** the tag string **/ private final String mTag; /** an optional first argument. For example the parameter name. **/ @@ -34,12 +36,14 @@ class JavadocTag /** * Constructs the object. * @param aLine the line number of the tag + * @param aColumn the column number of the tag * @param aTag the tag string * @param aArg1 the tag argument **/ - JavadocTag(int aLine, String aTag, String aArg1) + JavadocTag(int aLine, int aColumn, String aTag, String aArg1) { mLineNo = aLine; + mColumnNo = aColumn; mTag = aTag; mArg1 = aArg1; } @@ -47,11 +51,12 @@ class JavadocTag /** * Constructs the object. * @param aLine the line number of the tag + * @param aColumn the column number of the tag * @param aTag the tag string **/ - JavadocTag(int aLine, String aTag) + JavadocTag(int aLine, int aColumn, String aTag) { - this(aLine, aTag, null); + this(aLine, aColumn, aTag, null); } /** @return the tag string **/ @@ -72,11 +77,17 @@ class JavadocTag return mLineNo; } + /** @return the column number */ + int getColumnNo() + { + return mColumnNo; + } + /** @return a string representation of the object **/ public String toString() { return "{Tag = '" + getTag() + "', lineNo = " + getLineNo() - + ", Arg1 = '" + getArg1() + "'}"; + + ", columnNo=" + mColumnNo + ", Arg1 = '" + getArg1() + "'}"; } /** @return whether the tag is an 'author' tag **/ diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheck.java index 0332f2d94..778332b11 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheck.java @@ -214,9 +214,12 @@ public class JavadocTypeCheck if (content.endsWith("*/")) { content = content.substring(0, content.length() - 2); } - tags.add(new JavadocTag(aCmt.getStartLineNo() + i, - tagName, - content.trim())); + int col = tagMatcher.start(1) - 1; + if (i == 0) { + col += aCmt.getStartColNo(); + } + tags.add(new JavadocTag(aCmt.getStartLineNo() + i, col, + tagName, content.trim())); } tagPattern = Utils.getPattern("^\\s*\\**\\s*@(\\p{Alpha}+)\\s"); } @@ -300,16 +303,19 @@ public class JavadocTypeCheck if (matcher.matches()) { typeParamName = matcher.group(1).trim(); if (!aTypeParamNames.contains(typeParamName)) { - log(tag.getLineNo(), "javadoc.unusedTag", + log(tag.getLineNo(), tag.getColumnNo(), + "javadoc.unusedTag", "@param", "<" + typeParamName + ">"); } } else { - log(tag.getLineNo(), "javadoc.unusedTagGeneral"); + log(tag.getLineNo(), tag.getColumnNo(), + "javadoc.unusedTagGeneral"); } } else { - log(tag.getLineNo(), "javadoc.unusedTagGeneral"); + log(tag.getLineNo(), tag.getColumnNo(), + "javadoc.unusedTagGeneral"); } } } diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/javadoc/TestGenerics.java b/src/testinputs/com/puppycrawl/tools/checkstyle/javadoc/TestGenerics.java new file mode 100644 index 000000000..06f7ba077 --- /dev/null +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/javadoc/TestGenerics.java @@ -0,0 +1,46 @@ +public class TestGenerics +{ + /** + * @throws E in some cases + * @throws RE in some cases + */ + public void method1() throws E + { + } + + /** + * RuntimeException is not declared. + */ + public void method2() throws RE + { + } + + /** + * @throws E in some cases + * @throws RE in other cases + */ + public void method3() throws E, RE + { + } + + /** + * @throws RE in some cases + * @throws NPE in some other cases + */ + public void method4() throws NPE, RE + { + } + + public class InnerClass + { + /** + * @throws E in some case + * @throws RE in some other cases + */ + public void method1() throws RuntimeException, RE, + java.lang.RuntimeException + { + } + } +} diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheckTest.java index 4b6de5823..1f14f1dbc 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheckTest.java @@ -92,4 +92,24 @@ public class RedundantThrowsCheckTest final String[] expected = {}; verify(checkConfig, getPath("javadoc/BadCls.java"), expected); } + + public void test_generics_params() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(RedundantThrowsCheck.class); + final String[] expected = { + "15:34: Redundant throws: 'RE' is unchecked exception.", + "23:37: Redundant throws: 'RE' is subclass of 'E'.", + "23:37: Redundant throws: 'RE' is unchecked exception.", + "31:69: Redundant throws: 'NPE' is subclass of 'RE'.", + "31:69: Redundant throws: 'NPE' is unchecked exception.", + "31:74: Redundant throws: 'RE' is unchecked exception.", + "41:38: Redundant throws: 'RuntimeException' is subclass of 'RE'.", + "41:38: Redundant throws: 'RuntimeException' is unchecked exception.", + "41:56: Redundant throws: 'RE' is subclass of 'java.lang.RuntimeException'.", + "41:56: Redundant throws: 'RE' is unchecked exception.", + "42:13: Redundant throws: 'java.lang.RuntimeException' is unchecked exception.", + }; + verify(checkConfig, getPath("javadoc/TestGenerics.java"), expected); + } } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheckTest.java index 5c2a7324f..3ec5d3201 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheckTest.java @@ -14,32 +14,35 @@ public class JavadocMethodCheckTest extends BaseCheckTestCase createCheckConfig(JavadocMethodCheck.class); final String[] expected = { "14:5: Missing a Javadoc comment.", - "18: Unused @param tag for 'unused'.", - "24: Expected an @return tag.", "33: Expected an @return tag.", + "18:9: Unused @param tag for 'unused'.", + "24: Expected an @return tag.", + "33: Expected an @return tag.", "40:16: Expected @throws tag for 'Exception'.", "49:16: Expected @throws tag for 'Exception'.", - "53: Unused @throws tag for 'WrongException'.", + "53:9: Unused @throws tag for 'WrongException'.", "55:16: Expected @throws tag for 'Exception'.", "55:27: Expected @throws tag for 'NullPointerException'.", "60:22: Expected @param tag for 'aOne'.", "68:22: Expected @param tag for 'aOne'.", - "72: Unused @param tag for 'WrongParam'.", + "72:9: Unused @param tag for 'WrongParam'.", "73:23: Expected @param tag for 'aOne'.", "73:33: Expected @param tag for 'aTwo'.", - "78: Unused @param tag for 'Unneeded'.", - "79: Unused Javadoc tag.", "87: Duplicate @return tag.", + "78:8: Unused @param tag for 'Unneeded'.", + "79: Unused Javadoc tag.", + "87:8: Duplicate @return tag.", "109:23: Expected @param tag for 'aOne'.", "109:55: Expected @param tag for 'aFour'.", "109:66: Expected @param tag for 'aFive'.", - "178: Unused @throws tag for 'ThreadDeath'.", - "179: Unused @throws tag for 'ArrayStoreException'.", - "236: Unused @throws tag for 'java.io.FileNotFoundException'.", - "254: Unused @throws tag for 'java.io.FileNotFoundException'.", + "178:8: Unused @throws tag for 'ThreadDeath'.", + "179:8: Unused @throws tag for 'ArrayStoreException'.", + "236:8: Unused @throws tag for 'java.io.FileNotFoundException'.", + "254:8: Unused @throws tag for 'java.io.FileNotFoundException'.", "256:28: Expected @throws tag for 'IOException'.", - "262: Unused @param tag for 'aParam'.", + "262:8: Unused @param tag for 'aParam'.", "320:9: Missing a Javadoc comment.", "329:5: Missing a Javadoc comment.", - "333: Unused Javadoc tag.", }; + "333: Unused Javadoc tag.", + }; verify(checkConfig, getPath("InputTags.java"), expected); } @@ -51,7 +54,7 @@ public class JavadocMethodCheckTest extends BaseCheckTestCase checkConfig.addAttribute("allowUndeclaredRTE", "true"); final String[] expected = { "14:5: Missing a Javadoc comment.", - "18: Unused @param tag for 'unused'.", + "18:9: Unused @param tag for 'unused'.", "24: Expected an @return tag.", "33: Expected an @return tag.", "40:16: Expected @throws tag for 'Exception'.", @@ -60,18 +63,19 @@ public class JavadocMethodCheckTest extends BaseCheckTestCase "55:27: Expected @throws tag for 'NullPointerException'.", "60:22: Expected @param tag for 'aOne'.", "68:22: Expected @param tag for 'aOne'.", - "72: Unused @param tag for 'WrongParam'.", + "72:9: Unused @param tag for 'WrongParam'.", "73:23: Expected @param tag for 'aOne'.", "73:33: Expected @param tag for 'aTwo'.", - "78: Unused @param tag for 'Unneeded'.", - "79: Unused Javadoc tag.", "87: Duplicate @return tag.", + "78:8: Unused @param tag for 'Unneeded'.", + "79: Unused Javadoc tag.", + "87:8: Duplicate @return tag.", "109:23: Expected @param tag for 'aOne'.", "109:55: Expected @param tag for 'aFour'.", "109:66: Expected @param tag for 'aFive'.", - "236: Unused @throws tag for 'java.io.FileNotFoundException'.", - "254: Unused @throws tag for 'java.io.FileNotFoundException'.", + "236:8: Unused @throws tag for 'java.io.FileNotFoundException'.", + "254:8: Unused @throws tag for 'java.io.FileNotFoundException'.", "256:28: Expected @throws tag for 'IOException'.", - "262: Unused @param tag for 'aParam'.", + "262:8: Unused @param tag for 'aParam'.", "320:9: Missing a Javadoc comment.", "329:5: Missing a Javadoc comment.", "333: Unused Javadoc tag.", }; @@ -171,7 +175,7 @@ public class JavadocMethodCheckTest extends BaseCheckTestCase checkConfig.addAttribute("allowThrowsTagsForSubclasses", "true"); final String[] expected = { "14:5: Missing a Javadoc comment.", - "18: Unused @param tag for 'unused'.", + "18:9: Unused @param tag for 'unused'.", "24: Expected an @return tag.", "33: Expected an @return tag.", "40:16: Expected @throws tag for 'Exception'.", @@ -180,18 +184,19 @@ public class JavadocMethodCheckTest extends BaseCheckTestCase "55:27: Expected @throws tag for 'NullPointerException'.", "60:22: Expected @param tag for 'aOne'.", "68:22: Expected @param tag for 'aOne'.", - "72: Unused @param tag for 'WrongParam'.", + "72:9: Unused @param tag for 'WrongParam'.", "73:23: Expected @param tag for 'aOne'.", "73:33: Expected @param tag for 'aTwo'.", - "78: Unused @param tag for 'Unneeded'.", - "79: Unused Javadoc tag.", "87: Duplicate @return tag.", + "78:8: Unused @param tag for 'Unneeded'.", + "79: Unused Javadoc tag.", + "87:8: Duplicate @return tag.", "109:23: Expected @param tag for 'aOne'.", "109:55: Expected @param tag for 'aFour'.", "109:66: Expected @param tag for 'aFive'.", - "178: Unused @throws tag for 'ThreadDeath'.", - "179: Unused @throws tag for 'ArrayStoreException'.", + "178:8: Unused @throws tag for 'ThreadDeath'.", + "179:8: Unused @throws tag for 'ArrayStoreException'.", "256:28: Expected @throws tag for 'IOException'.", - "262: Unused @param tag for 'aParam'.", + "262:8: Unused @param tag for 'aParam'.", "320:9: Missing a Javadoc comment.", "329:5: Missing a Javadoc comment.", "333: Unused Javadoc tag.", }; @@ -349,7 +354,7 @@ public class JavadocMethodCheckTest extends BaseCheckTestCase final DefaultConfiguration checkConfig = createCheckConfig(JavadocMethodCheck.class); final String[] expected = { - "26: Unused @param tag for ''.", + "26:8: Unused @param tag for ''.", "28:13: Expected @param tag for ''.", }; verify(checkConfig, getPath("InputTypeParamsTags.java"), expected); } @@ -379,4 +384,52 @@ public class JavadocMethodCheckTest extends BaseCheckTestCase final String[] expected = {}; verify(checkConfig, getPath("javadoc/Test3.java"), expected); } + + public void test_generics_1() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(JavadocMethodCheck.class); + checkConfig.addAttribute("allowThrowsTagsForSubclasses", "true"); + checkConfig.addAttribute("allowUndeclaredRTE", "true"); + final String[] expected = { + "15:34: Expected @throws tag for 'RE'.", + "23:37: Expected @throws tag for 'RE'.", + "31:13: Expected @param tag for ''.", + "38:12: Unused @throws tag for 'E'.", + "41:38: Expected @throws tag for 'RuntimeException'.", + "42:13: Expected @throws tag for 'java.lang.RuntimeException'.", + }; + verify(checkConfig, getPath("javadoc/TestGenerics.java"), expected); + } + + public void test_generics_2() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(JavadocMethodCheck.class); + checkConfig.addAttribute("allowThrowsTagsForSubclasses", "true"); + final String[] expected = { + "15:34: Expected @throws tag for 'RE'.", + "23:37: Expected @throws tag for 'RE'.", + "31:13: Expected @param tag for ''.", + "38:12: Unused @throws tag for 'E'.", + "41:38: Expected @throws tag for 'RuntimeException'.", + "42:13: Expected @throws tag for 'java.lang.RuntimeException'.", + }; + verify(checkConfig, getPath("javadoc/TestGenerics.java"), expected); + } + + public void test_generics_3() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(JavadocMethodCheck.class); + final String[] expected = { + "6:8: Unused @throws tag for 'RE'.", + "15:34: Expected @throws tag for 'RE'.", + "31:13: Expected @param tag for ''.", + "38:12: Unused @throws tag for 'E'.", + "41:38: Expected @throws tag for 'RuntimeException'.", + "42:13: Expected @throws tag for 'java.lang.RuntimeException'.", + }; + verify(checkConfig, getPath("javadoc/TestGenerics.java"), expected); + } } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheckTest.java index 8119cac55..129752d80 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheckTest.java @@ -280,7 +280,7 @@ public class JavadocTypeCheckTest extends BaseCheckTestCase final DefaultConfiguration checkConfig = createCheckConfig(JavadocTypeCheck.class); final String[] expected = { - "7: Unused @param tag for ''.", + "7:4: Unused @param tag for ''.", "11: Type Javadoc comment is missing an @param tag.", }; verify(checkConfig, getPath("InputTypeParamsTags.java"), expected); @@ -291,7 +291,7 @@ public class JavadocTypeCheckTest extends BaseCheckTestCase createCheckConfig(JavadocTypeCheck.class); checkConfig.addAttribute("allowMissingParamTags", "true"); final String[] expected = { - "7: Unused @param tag for ''.", + "7:4: Unused @param tag for ''.", }; verify(checkConfig, getPath("InputTypeParamsTags.java"), expected); } diff --git a/src/xdocs/releasenotes.xml b/src/xdocs/releasenotes.xml index c8ea7c626..cd428b611 100755 --- a/src/xdocs/releasenotes.xml +++ b/src/xdocs/releasenotes.xml @@ -73,6 +73,9 @@
  • Fixed implementation of DetailAST so getPreviousSibling() works for all nodes in AST tree (bug 1244994)
  • +
  • Now type-aware check know more about generics (bug 1249707, + modules: RedundantThrows and JavadocMethodCheck)
  • +

    Other improvements:

    diff --git a/suppressions.xml b/suppressions.xml index e6dacb2bb..d6bc0de2a 100755 --- a/suppressions.xml +++ b/suppressions.xml @@ -13,7 +13,7 @@ lines="176"/> + lines="751,783,808"/>