diff --git a/docs/checkstyle_checks.xml b/docs/checkstyle_checks.xml index aea02960f..7a328c42d 100644 --- a/docs/checkstyle_checks.xml +++ b/docs/checkstyle_checks.xml @@ -37,6 +37,7 @@ + diff --git a/docs/config_javadoc.html b/docs/config_javadoc.html index eec91c875..022836dbf 100644 --- a/docs/config_javadoc.html +++ b/docs/config_javadoc.html @@ -226,6 +226,13 @@ boolean false + + allowThrowsTagsForSubclasses + whether to allow documented exceptions that + are subclass of one of declared exception. + boolean + false + allowMissingParamTags whether to ignore errors when a method has parameters diff --git a/docs/releasenotes.html b/docs/releasenotes.html index b9c805411..7be5f7b81 100644 --- a/docs/releasenotes.html +++ b/docs/releasenotes.html @@ -115,10 +115,13 @@
  • Added check to find empty statements (module EmptyStatement, request 724573).
  • - -
  • Added check to find magic numbers + +
  • Added check to find magic numbers (module MagicNumber, request 564206).
  • - + +
  • One more option for JavadocMethodCheck + (allowThrowsTagsForSubclasses, request 540383)
  • +

    diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheck.java index e7ae3af7e..39ce31c91 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheck.java @@ -77,7 +77,8 @@ import org.apache.regexp.RE; * * @author Oliver Burn * @author Rick Giles - * @version 1.0 + * @author o_sukhodoslky + * @version 1.1 */ public class JavadocMethodCheck extends AbstractImportCheck @@ -151,6 +152,13 @@ public class JavadocMethodCheck **/ private boolean mAllowUndeclaredRTE = false; + /** + * controls whether to allow documented exceptions that + * are subclass of one of declared exception. + * Defaults to false (backward compatibility). + **/ + private boolean mAllowThrowsTagsForSubclasses = false; + /** * controls whether to ignore errors when a method has parameters * but does not have matching param tags in the javadoc. @@ -192,6 +200,16 @@ public class JavadocMethodCheck mAllowUndeclaredRTE = aFlag; } + /** + * controls whether to allow documented exception that + * are subclass of one of declared exceptions. + * @param aFlag a Boolean value + */ + public void setAllowThrowsTagsForSubclasses(boolean aFlag) + { + mAllowThrowsTagsForSubclasses = aFlag; + } + /** * controls whether to allow a method which has parameters * to omit matching param tags in the javadoc. @@ -234,7 +252,7 @@ public class JavadocMethodCheck TokenTypes.METHOD_DEF, TokenTypes.CTOR_DEF}; } - + /** @see com.puppycrawl.tools.checkstyle.api.Check */ public int[] getAcceptableTokens() { @@ -242,7 +260,7 @@ public class JavadocMethodCheck TokenTypes.METHOD_DEF, TokenTypes.CTOR_DEF}; } - + /** @see com.puppycrawl.tools.checkstyle.api.Check */ public int[] getRequiredTokens() { @@ -256,6 +274,7 @@ public class JavadocMethodCheck { mPackageFullIdent = FullIdent.createFullIdent(null); mImports.clear(); + mClassResolver = null; } /** @see com.puppycrawl.tools.checkstyle.api.Check */ @@ -473,8 +492,9 @@ public class JavadocMethodCheck if ((child.getType() == TokenTypes.IDENT) || (child.getType() == TokenTypes.DOT)) { - final FullIdent fi = FullIdent.createFullIdent(child); - retVal.add(fi); + final ExceptionInfo ei = + new ExceptionInfo(FullIdent.createFullIdent(child)); + retVal.add(ei); } child = (DetailAST) child.getNextSibling(); } @@ -641,7 +661,7 @@ public class JavadocMethodCheck private void checkThrowsTags(List aTags, List aThrows) { // Loop over the tags, checking to see they exist in the throws. - final Set foundThrows = new HashSet(); + final Set foundThrows = new HashSet(); //used for performance only final ListIterator tagIt = aTags.listIterator(); while (tagIt.hasNext()) { final JavadocTag tag = (JavadocTag) tagIt.next(); @@ -657,24 +677,43 @@ public class JavadocMethodCheck boolean found = foundThrows.contains(documentedEx); final ListIterator throwIt = aThrows.listIterator(); while (!found && throwIt.hasNext()) { - final FullIdent fi = (FullIdent) throwIt.next(); + final ExceptionInfo ei = (ExceptionInfo) throwIt.next(); + final FullIdent fi = ei.getFullIdent(); final String declaredEx = fi.getText(); if (isSameType(declaredEx, documentedEx)) { found = true; - throwIt.remove(); + ei.setFound(); foundThrows.add(documentedEx); } + else if (mAllowThrowsTagsForSubclasses) { + final ClassResolver cr = getClassResolver(); + try { + final Class documentedClass = cr.resolve(documentedEx); + try { + final Class declaredClass = cr.resolve(declaredEx); + found = + declaredClass.isAssignableFrom(documentedClass); + if (found) { + ei.setFound(); + } + } + catch (ClassNotFoundException e) { + log(tag.getLineNo(), "javadoc.classInfo", + "@throws", declaredEx); + } + } + catch (ClassNotFoundException e) { + log(tag.getLineNo(), "javadoc.classInfo", + "@throws", documentedEx); + } + } } // Handle extra JavadocTag. if (!found) { boolean reqd = true; if (mAllowUndeclaredRTE) { - final ClassResolver cr = - new ClassResolver( - getClassLoader(), - mPackageFullIdent.getText(), - mImports); + final ClassResolver cr = getClassResolver(); try { final Class clazz = cr.resolve(tag.getArg1()); reqd = @@ -699,10 +738,68 @@ public class JavadocMethodCheck if (!mAllowMissingThrowsTags) { final ListIterator throwIt = aThrows.listIterator(); while (throwIt.hasNext()) { - final FullIdent fi = (FullIdent) throwIt.next(); - log(fi.getLineNo(), fi.getColumnNo(), - "javadoc.expectedTag", "@throws", fi.getText()); + final ExceptionInfo ei = (ExceptionInfo) throwIt.next(); + if (!ei.isFound()) { + final FullIdent fi = ei.getFullIdent(); + log(fi.getLineNo(), fi.getColumnNo(), + "javadoc.expectedTag", "@throws", fi.getText()); + } } } } + + /** @return ClassResolver for current tree. */ + final ClassResolver getClassResolver() + { + if (mClassResolver == null) { + mClassResolver = new ClassResolver(getClassLoader(), + mPackageFullIdent.getText(), + mImports); + + } + return mClassResolver; + } + + /** ClassResolver instance for current tree. */ + private ClassResolver mClassResolver; +} + +/** + * Stores useful information about declared exception. + * @author o_sukhodoslky + */ +final class ExceptionInfo +{ + /** FullIdent of the exception. */ + private final FullIdent mIdent; + + /** does the exception have throws tag associated with. */ + private boolean mFound; + + /** + * Creates new instance for FullIdent. + * @param aIdent FullIdent of the exception + */ + ExceptionInfo(FullIdent aIdent) + { + mIdent = aIdent; + } + + /** @return FullIdent of the exception. */ + final FullIdent getFullIdent() + { + return mIdent; + } + + /** Mark that the exception has associated throws tag */ + final void setFound() + { + mFound = true; + } + + /** @return whether the exception has throws tag associated with */ + final boolean isFound() + { + return mFound; + } } diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/InputTags.java b/src/testinputs/com/puppycrawl/tools/checkstyle/InputTags.java index 9d9ad3385..5d79ff2f9 100644 --- a/src/testinputs/com/puppycrawl/tools/checkstyle/InputTags.java +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/InputTags.java @@ -217,4 +217,26 @@ class InputTags { return 579190; } + + /** + * Bug XXX, "two tags for the same exception" + * + * @exception java.io.IOException for some reasons + * @exception IOException for another reason + */ + void method21() + throws IOException + { + } + + /** + * RFE 540383, "Unused throws tag for exception subclass" + * + * @exception IOException for some reasons + * @exception java.io.FileNotFoundException for another reasons + */ + void method22() + throws IOException + { + } } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheckTest.java index c57456a0d..15abb2c6f 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/JavadocMethodCheckTest.java @@ -36,6 +36,7 @@ public class JavadocMethodCheckTest "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'.", }; verify(checkConfig, getPath("InputTags.java"), expected); @@ -69,6 +70,7 @@ public class JavadocMethodCheckTest "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'.", }; verify(checkConfig, getPath("InputTags.java"), expected); } @@ -163,7 +165,7 @@ public class JavadocMethodCheckTest verify(checkConfig, getPath("InputScopeAnonInner.java"), expected); } - public void testScopeAnonInnerWithResolver() + public void testScopeAnonInnerWithResolver() throws Exception { final DefaultConfiguration checkConfig = @@ -173,4 +175,38 @@ public class JavadocMethodCheckTest }; verify(checkConfig, getPath("InputScopeAnonInner.java"), expected); } + + public void testTagsWithSubclassesAllowed() + throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(JavadocMethodCheck.class); + checkConfig.addAttribute("allowThrowsTagsForSubclasses", "true"); + 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.", + "40:16: Expected @throws tag for 'Exception'.", + "49:16: Expected @throws tag for 'Exception'.", + "53: Unable to get class information for @throws tag 'WrongException'.", + "53: 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'.", + "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.", + "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'.", + }; + verify(checkConfig, getPath("InputTags.java"), expected); + } }