From 65b26d2972c32d9eaa3d6034ba67c0c6bfba0b18 Mon Sep 17 00:00:00 2001 From: Nick Mancuso Date: Thu, 3 Dec 2020 01:28:06 -0500 Subject: [PATCH] Issue #4358: add requiredJavadocPhrase to DesignForExtensionCheck --- .../design/DesignForExtensionCheck.java | 109 +++++++++++++++++- .../checks/design/DesignForExtensionCheck.xml | 6 + .../design/DesignForExtensionCheckTest.java | 25 ++++ ...signForExtensionRequiredJavadocPhrase.java | 66 +++++++++++ ...tensionRequiredJavadocPhraseMultiLine.java | 22 ++++ src/xdocs/config_design.xml | 69 +++++++++++ 6 files changed, 293 insertions(+), 4 deletions(-) create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhrase.java create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhraseMultiLine.java diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheck.java index 4173381a493..0e62445cc91 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheck.java @@ -23,6 +23,8 @@ import java.util.Optional; import java.util.Set; import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import com.puppycrawl.tools.checkstyle.StatelessCheck; @@ -159,6 +161,12 @@ * Type is {@code java.lang.String[]}. * Default value is {@code After, AfterClass, Before, BeforeClass, Test}. * + *
  • + * Property {@code requiredJavadocPhrase} - Specify the comment text pattern which qualifies a + * method as designed for extension. Supports multi-line regex. + * Type is {@code java.util.regex.Pattern}. + * Default value is {@code ".*"}. + *
    • * *

    * To configure the check: @@ -208,6 +216,66 @@ * } * *

    + * To configure the check to allow methods which contain a specified comment text + * pattern in their javadoc to be designed for extension. + *

    + * 
    + * <module name="DesignForExtension">
+ *   <property name="requiredJavadocPhrase"
+ *     value="This implementation"/>
+ * </module>
+ *
    + * 
    + * public class A extends B {
+ *   /**
+ *   * This implementation ...
+ *   *
+ *   */
+ *   public int foo() { // ok, required javadoc phrase in comment
+ *     return 2;
+ *   }
+ *
+ *   /**
+ *   * Do not extend ...
+ *   *
+ *   */
+ *   public int foo2() {return 8;} // violation, required javadoc phrase not in comment
+ *
+ *   public int foo3() {return 3;} // violation, required javadoc phrase not in comment
+ * }
+ *
    + *

    + * To configure the check to allow methods which contain a specified comment text + * pattern in their javadoc which can span multiple lines + * to be designed for extension. + *

    + * 
    + * <module name="DesignForExtension">
+ *   <property name="requiredJavadocPhrase"
+ *     value="This[\s\S]*implementation"/>
+ * </module>
+ *
    + * 
    + * public class A extends B {
+ *   /**
+ *   * This
+ *   * implementation ...
+ *   *
+ *   */
+ *   public int foo() { // ok, required javadoc phrase in comment
+ *     return 2;
+ *   }
+ *
+ *   /**
+ *   * Do not extend ...
+ *   *
+ *   */
+ *   public int foo2() {return 8;} // violation, required javadoc phrase not in comment
+ *
+ *   public int foo3() {return 3;} // violation, required javadoc phrase not in comment
+ * }
+ *
    + *

    * Parent is {@code com.puppycrawl.tools.checkstyle.TreeWalker} *

    *

    @@ -236,6 +304,12 @@ public class DesignForExtensionCheck extends AbstractCheck { private Set ignoredAnnotations = Arrays.stream(new String[] {"Test", "Before", "After", "BeforeClass", "AfterClass", }).collect(Collectors.toSet()); + /** + * Specify the comment text pattern which qualifies a method as designed for extension. + * Supports multi-line regex. + */ + private Pattern requiredJavadocPhrase = Pattern.compile(".*"); + /** * Setter to specify annotations which allow the check to skip the method from validation. * @@ -245,6 +319,16 @@ public void setIgnoredAnnotations(String... ignoredAnnotations) { this.ignoredAnnotations = Arrays.stream(ignoredAnnotations).collect(Collectors.toSet()); } + /** + * Setter to specify the comment text pattern which qualifies a + * method as designed for extension. Supports multi-line regex. + * + * @param requiredJavadocPhrase method annotations. + */ + public void setRequiredJavadocPhrase(Pattern requiredJavadocPhrase) { + this.requiredJavadocPhrase = requiredJavadocPhrase; + } + @Override public int[] getDefaultTokens() { return getRequiredTokens(); @@ -291,7 +375,7 @@ && canBeOverridden(ast) * @param methodDef method definition token. * @return true if a method has a javadoc comment. */ - private static boolean hasJavadocComment(DetailAST methodDef) { + private boolean hasJavadocComment(DetailAST methodDef) { return hasJavadocCommentOnToken(methodDef, TokenTypes.MODIFIERS) || hasJavadocCommentOnToken(methodDef, TokenTypes.TYPE); } @@ -303,7 +387,7 @@ private static boolean hasJavadocComment(DetailAST methodDef) { * @param tokenType token type. * @return true if a token has a javadoc comment. */ - private static boolean hasJavadocCommentOnToken(DetailAST methodDef, int tokenType) { + private boolean hasJavadocCommentOnToken(DetailAST methodDef, int tokenType) { final DetailAST token = methodDef.findFirstToken(tokenType); return branchContainsJavadocComment(token); } @@ -314,13 +398,13 @@ private static boolean hasJavadocCommentOnToken(DetailAST methodDef, int tokenTy * @param token tree token. * @return true if a javadoc comment exists under the token. */ - private static boolean branchContainsJavadocComment(DetailAST token) { + private boolean branchContainsJavadocComment(DetailAST token) { boolean result = false; DetailAST curNode = token; while (curNode != null) { if (curNode.getType() == TokenTypes.BLOCK_COMMENT_BEGIN && JavadocUtil.isJavadocComment(curNode)) { - result = true; + result = hasValidJavadocComment(curNode); break; } @@ -339,6 +423,23 @@ private static boolean branchContainsJavadocComment(DetailAST token) { return result; } + /** + * Checks whether a javadoc contains the specified comment pattern that denotes + * a method as designed for extension. + * + * @param detailAST the ast we are checking for possible extension + * @return true if the javadoc of this ast contains the required comment pattern + */ + private boolean hasValidJavadocComment(DetailAST detailAST) { + final String javadocString = + JavadocUtil.getBlockCommentContent(detailAST); + + final Matcher requiredJavadocPhraseMatcher = + requiredJavadocPhrase.matcher(javadocString); + + return requiredJavadocPhraseMatcher.find(); + } + /** * Checks whether a methods is native. * diff --git a/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/design/DesignForExtensionCheck.xml b/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/design/DesignForExtensionCheck.xml index 791167bdfab..9626ff3fbea 100644 --- a/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/design/DesignForExtensionCheck.xml +++ b/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/design/DesignForExtensionCheck.xml @@ -128,6 +128,12 @@ Specify annotations which allow the check to skip the method from validation. + + Specify the comment text pattern which qualifies a + method as designed for extension. Supports multi-line regex. + diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheckTest.java index 458fcd2251c..98ec3593f92 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/DesignForExtensionCheckTest.java @@ -138,4 +138,29 @@ public void testDesignForExtensionRecords() throws Exception { getNonCompilablePath("InputDesignForExtensionRecords.java"), expected); } + @Test + public void testRequiredJavadocPhrase() throws Exception { + final DefaultConfiguration checkConfig = createModuleConfig(DesignForExtensionCheck.class); + checkConfig.addAttribute("requiredJavadocPhrase", "This implementation"); + final String className = "InputDesignForExtensionRequiredJavadocPhrase"; + final String[] expected = { + "37:5: " + getCheckMessage(MSG_KEY, className, "foo5"), + "44:5: " + getCheckMessage(MSG_KEY, className, "foo8"), + "47:5: " + getCheckMessage(MSG_KEY, className, "foo9"), + "63:5: " + getCheckMessage(MSG_KEY, className, "foo12"), + }; + verify(checkConfig, getPath("InputDesignForExtensionRequiredJavadocPhrase.java"), expected); + } + + @Test + public void testRequiredJavadocPhraseMultiLine() throws Exception { + final DefaultConfiguration checkConfig = createModuleConfig(DesignForExtensionCheck.class); + checkConfig.addAttribute("requiredJavadocPhrase", "This[\\s\\S]*implementation"); + final String className = "InputDesignForExtensionRequiredJavadocPhraseMultiLine"; + final String[] expected = { + "19:5: " + getCheckMessage(MSG_KEY, className, "foo2"), + }; + verify(checkConfig, getPath("InputDesignForExtensionRequiredJavadocPhraseMultiLine.java"), + expected); + } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhrase.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhrase.java new file mode 100644 index 00000000000..bae19a995fe --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhrase.java @@ -0,0 +1,66 @@ +package com.puppycrawl.tools.checkstyle.checks.design.designforextension; + +/* Config: + * requiredJavadocPhrase = "This implementation" + * + */ +public class InputDesignForExtensionRequiredJavadocPhrase { + + /** + * This implementation is for

    some html code + *

    . + * + * @param a + * @param b + * @return sum + */ + public int foo1(int a, int b) {return a + b;} // ok, required comment pattern in javadoc + + /** + * This implementation is required for ... + * + * @param a + * @param b + * @return sum + */ + public int foo2(int a, int b) {return a + b;} // ok, required comment pattern in javadoc + + /** This implementation is for ... */ + public int foo3(int a, int b) {return a + b;} // ok, required comment pattern in javadoc + + /** + * This implementation ... + */ + public int foo4(int a, int b) {return a + b;} // ok, required comment pattern in javadoc + + /** This method can safely be overridden. */ + public int foo5(int a, int b) {return a + b;} // violation + + public final int foo6(int a) {return a - 2;} // ok, final + + protected final int foo7(int a) {return a - 2;} // ok, final + + /** */ + public int foo8(int a) {return a - 2;} // violation + + // This implementation + public int foo9(int a, int b) {return a + b;} // violation, not javadoc + + @Deprecated + protected final int foo10(int a) {return a - 2;} // ok, deprecated + + /** + * This implementation is for

    some html code + *

    . + * + * @param a + * @param b + * @return sum + */ + public int foo11(int a, int b) {return a + b;} // ok, required comment pattern in javadoc + + /**This method can safely be overridden. */ + public int foo12(int a, int b) { // violation + return a + b; + } +} diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhraseMultiLine.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhraseMultiLine.java new file mode 100644 index 00000000000..e68f26552b9 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/design/designforextension/InputDesignForExtensionRequiredJavadocPhraseMultiLine.java @@ -0,0 +1,22 @@ +package com.puppycrawl.tools.checkstyle.checks.design.designforextension; + +/* Config: + * requiredJavadocPhrase = "This[\s\S]*implementation" + * + */ +public class InputDesignForExtensionRequiredJavadocPhraseMultiLine { + /** + * This + * implementation .. + */ + public int foo1(int a, int b) { + return a * b; + } + + /** + * This method can safely be overridden. + */ + public int foo2(int a, int b) { // violation + return a + b; + } +} diff --git a/src/xdocs/config_design.xml b/src/xdocs/config_design.xml index f380f335afd..27f2dbd569a 100644 --- a/src/xdocs/config_design.xml +++ b/src/xdocs/config_design.xml @@ -157,6 +157,16 @@ public abstract class Plant { After, AfterClass, Before, BeforeClass, Test 7.2 + + requiredJavadocPhrase + + Specify the comment text pattern which qualifies a method as designed for extension. + Supports multi-line regex. + + Pattern + ".*" + 8.40 + @@ -211,6 +221,65 @@ public class FooTest { } public int foo4() {return 4;} // violation +} + +

    + To configure the check to allow methods which contain a specified comment text + pattern in their javadoc to be designed for extension. +

    + +<module name="DesignForExtension"> + <property name="requiredJavadocPhrase" value="This implementation"/> +</module> + + +public class A extends B { + /** + * This implementation ... + * + */ + public int foo() { // ok, required javadoc phrase in comment + return 2; + } + + /** + * Do not extend ... + * + */ + public int foo2() {return 8;} // violation, required javadoc phrase not in comment + + public int foo3() {return 3;} // violation, required javadoc phrase not in comment +} + +

    + To configure the check to allow methods which contain a specified comment text + pattern in their javadoc which can span multiple lines + to be designed for extension. +

    + +<module name="DesignForExtension"> + <property name="requiredJavadocPhrase" + value="This[\s\S]*implementation"/> +</module> + + +public class A extends B { + /** + * This + * implementation ... + * + */ + public int foo() { // ok, required javadoc phrase in comment + return 2; + } + + /** + * Do not extend ... + * + */ + public int foo2() {return 8;} // violation, required javadoc phrase not in comment + + public int foo3() {return 3;} // violation, required javadoc phrase not in comment }