From 4310b16242102343d58bf31376d16bdfa06bbd69 Mon Sep 17 00:00:00 2001 From: patchwork01 <110390516+patchwork01@users.noreply.github.com> Date: Tue, 19 Mar 2024 12:13:51 +0000 Subject: [PATCH] Issue #14689: Stop applying Javadoc summary validation outside summary --- .../InputIncorrectSummaryJavaDocCheck.java | 2 +- .../com/puppycrawl/tools/checkstyle/Main.java | 2 +- .../checks/SuppressWarningsHolder.java | 2 +- .../coding/ModifiedControlVariableCheck.java | 2 +- .../checks/imports/PkgImportControl.java | 4 +- .../checks/javadoc/SummaryJavadocCheck.java | 74 ++++++++++++++----- .../metrics/AbstractClassCouplingCheck.java | 2 +- .../javadoc/SummaryJavadocCheckTest.java | 4 + .../InputSummaryJavadocCorrect.java | 10 ++- .../InputSummaryJavadocIncorrect.java | 2 +- .../InputSummaryJavadocIncorrect2.java | 2 +- .../InputSummaryJavadocPeriod.java | 22 +++++- .../InputSummaryJavadocPeriodAtEnd.java | 7 +- 13 files changed, 106 insertions(+), 29 deletions(-) diff --git a/src/it/resources/com/google/checkstyle/test/chapter7javadoc/rule72thesummaryfragment/InputIncorrectSummaryJavaDocCheck.java b/src/it/resources/com/google/checkstyle/test/chapter7javadoc/rule72thesummaryfragment/InputIncorrectSummaryJavaDocCheck.java index 78d116711fb..8444dec4ae6 100644 --- a/src/it/resources/com/google/checkstyle/test/chapter7javadoc/rule72thesummaryfragment/InputIncorrectSummaryJavaDocCheck.java +++ b/src/it/resources/com/google/checkstyle/test/chapter7javadoc/rule72thesummaryfragment/InputIncorrectSummaryJavaDocCheck.java @@ -7,7 +7,7 @@ class InputIncorrectSummaryJavaDocCheck { /** - * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)} + * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)}. */ void foo3() {} diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/Main.java b/src/main/java/com/puppycrawl/tools/checkstyle/Main.java index 889beac3aef..6eced717a64 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/Main.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/Main.java @@ -503,7 +503,7 @@ private static AuditListener createListener(OutputFormat format, Path outputLoca } /** - * Create output stream or return System.out + * Create output stream or return System.out. * * @param outputPath output location * @return output stream diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/SuppressWarningsHolder.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/SuppressWarningsHolder.java index 1a617e33a7c..3cf258ce9fe 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/SuppressWarningsHolder.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/SuppressWarningsHolder.java @@ -72,7 +72,7 @@ public class SuppressWarningsHolder */ private static final String CHECKSTYLE_PREFIX = "checkstyle:"; - /** Java.lang namespace prefix, which is stripped from SuppressWarnings */ + /** Java.lang namespace prefix, which is stripped from SuppressWarnings. */ private static final String JAVA_LANG_PREFIX = "java.lang."; /** Suffix to be removed from subclasses of Check. */ diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ModifiedControlVariableCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ModifiedControlVariableCheck.java index 9680bab002b..c70dc27c39d 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ModifiedControlVariableCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ModifiedControlVariableCheck.java @@ -408,7 +408,7 @@ private static Set getForIteratorVariables(DetailAST ast) { } /** - * Find all child of given AST of type TokenType.EXPR + * Find all child of given AST of type TokenType.EXPR. * * @param ast parent of expressions to find * @return all child of given ast diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/PkgImportControl.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/PkgImportControl.java index 517e0635a62..1e3fd3cd616 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/PkgImportControl.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/PkgImportControl.java @@ -29,13 +29,13 @@ * be a sub-package, a class, or an allow/disallow rule. */ class PkgImportControl extends AbstractImportControl { - /** The package separator: "." */ + /** The package separator: ".". */ private static final String DOT = "."; /** The regex for the package separator: "\\.". */ private static final String DOT_REGEX = "\\."; - /** A pattern matching the package separator: "\." */ + /** A pattern matching the package separator: "\.". */ private static final Pattern DOT_REGEX_PATTERN = Pattern.compile(DOT_REGEX); /** The regex for the escaped package separator: "\\\\.". */ diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheck.java index 4f15de5c1fb..a0eb4178628 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheck.java @@ -218,13 +218,11 @@ private void validateUntaggedSummary(DetailNode ast) { log(ast.getLineNumber(), MSG_SUMMARY_JAVADOC_MISSING); } else if (!period.isEmpty()) { - final String firstSentence = getFirstSentence(ast); - final int endOfSentence = firstSentence.lastIndexOf(period); - if (!summaryDoc.contains(period)) { + final String firstSentence = getFirstSentence(ast, period); + if (!summaryDoc.contains(period) || firstSentence.isEmpty()) { log(ast.getLineNumber(), MSG_SUMMARY_FIRST_SENTENCE); } - if (endOfSentence != -1 - && containsForbiddenFragment(firstSentence.substring(0, endOfSentence))) { + else if (containsForbiddenFragment(firstSentence)) { log(ast.getLineNumber(), MSG_SUMMARY_JAVADOC); } } @@ -579,28 +577,70 @@ private static String getStringInsideTag(String result, DetailNode detailNode) { * Finds and returns first sentence. * * @param ast Javadoc root node. + * @param period Period character. * @return first sentence. */ - private static String getFirstSentence(DetailNode ast) { + private static String getFirstSentence(DetailNode ast, String period) { final StringBuilder result = new StringBuilder(256); - final String periodSuffix = DEFAULT_PERIOD + ' '; - for (DetailNode child : ast.getChildren()) { - final String text; - if (child.getChildren().length == 0) { - text = child.getText(); + final boolean foundEnd = appendFirstSentence(ast, period, result); + if (!foundEnd) { + result.setLength(0); + } + return result.toString(); + } + + /** + * Finds and appends first sentence to result. + * + * @param ast Javadoc node to append. + * @param period Period character. + * @param result Builder to append to. + * @return true if the period character was found, false otherwise + */ + private static boolean appendFirstSentence( + DetailNode ast, String period, StringBuilder result) { + boolean foundEnd = false; + if (ast.getChildren().length == 0) { + final String text = ast.getText(); + final int periodIndex = findEndingPeriod(text, period); + if (periodIndex >= 0) { + result.append(text, 0, periodIndex); + foundEnd = true; } else { - text = getFirstSentence(child); + result.append(text); } - - if (text.contains(periodSuffix)) { - result.append(text, 0, text.indexOf(periodSuffix) + 1); + } + for (DetailNode child : ast.getChildren()) { + if (appendFirstSentence(child, period, result)) { + foundEnd = true; break; } + } + return foundEnd; + } - result.append(text); + /** + * Find position of an ending period in the text. Ignores any period not followed by + * whitespace. + * + * @param text text to search + * @param period period character + * @return position of period character, or -1 if there is no ending period + */ + private static int findEndingPeriod(String text, String period) { + int periodIndex = text.indexOf(period); + while (periodIndex >= 0) { + final int afterPeriodIndex = periodIndex + period.length(); + if (afterPeriodIndex >= text.length() + || Character.isWhitespace(text.charAt(afterPeriodIndex))) { + break; + } + else { + periodIndex = text.indexOf(period, afterPeriodIndex); + } } - return result.toString(); + return periodIndex; } } diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/AbstractClassCouplingCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/AbstractClassCouplingCheck.java index d50bfd6d9a8..22bbd9a9012 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/AbstractClassCouplingCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/AbstractClassCouplingCheck.java @@ -49,7 +49,7 @@ @FileStatefulCheck public abstract class AbstractClassCouplingCheck extends AbstractCheck { - /** A package separator - "." */ + /** A package separator - ".". */ private static final char DOT = '.'; /** Class names to ignore. */ diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheckTest.java index fc11a82b503..1729c0bdcc2 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheckTest.java @@ -113,6 +113,9 @@ public void testPeriod() throws Exception { "14: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE), "19: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE), "37: " + getCheckMessage(MSG_SUMMARY_MISSING_PERIOD), + "42: " + getCheckMessage(MSG_SUMMARY_JAVADOC), + "49: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE), + "55: " + getCheckMessage(MSG_SUMMARY_JAVADOC), }; verifyWithInlineConfigParser( @@ -230,6 +233,7 @@ public void testPeriodAtEnd() throws Exception { "33: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING), "40: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE), "60: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE), + "70: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE), }; verifyWithInlineConfigParser( diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocCorrect.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocCorrect.java index 8760f2873ef..528dcd07dde 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocCorrect.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocCorrect.java @@ -2,7 +2,7 @@ SummaryJavadoc violateExecutionOnNonTightHtml = (default)false forbiddenSummaryFragments = ^@return the *|^This method returns *|^A \ - [{]@code [a-zA-Z0-9]+[}]( is a ) + [{]@code [a-zA-Z0-9]+[}]( is a )|fail-summary-fragment period = (default). @@ -89,6 +89,14 @@ void foo14() {} */ void foo15() {} + /** + * Summary sentence on its own line. + *

+ * Another sentence that is not part of the summary, + * so this should not matter: fail-summary-fragment. + */ + void foo16() {} + /** * This is summary java doc. * diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect.java index 16a5d5570e0..167956afe5b 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect.java @@ -17,7 +17,7 @@ class InputSummaryJavadocIncorrect { /** - * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)} + * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)}. */ void foo3() {} // violation below 'Summary javadoc is missing.' diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect2.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect2.java index e5717e0314e..26aac282ea3 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect2.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect2.java @@ -16,7 +16,7 @@ class InputSummaryJavadocIncorrect2 { /** - * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)} + * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)}. */ void foo3() {} // violation below 'Summary javadoc is missing.' diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriod.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriod.java index 32a5aaa2dbd..95db32fbe4e 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriod.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriod.java @@ -1,7 +1,7 @@ /* SummaryJavadoc violateExecutionOnNonTightHtml = (default)false -forbiddenSummaryFragments = (default)^$ +forbiddenSummaryFragments = ^$|fail-summary-fragment period = _ @@ -37,4 +37,24 @@ void foo7(){} * {@summary An especially short bit of Javadoc} */ // violation above 'Summary .* missing an ending period.' void foo8() {} + + // violation below 'Forbidden summary fragment.' + /** + * Summary sentence containing default period mentioning version 1.1, then ending with correct + * period after disallowed words, fail-summary-fragment_ + */ + void foo9() {} + + // violation below 'First sentence .* missing an ending period.' + /** + * Summary sentence containing correct period mid_word, but not at the end + */ + void foo10() throws Exception {} + + // violation below 'Forbidden summary fragment.' + /** + * Summary sentence containing correct period mid_word, then ending with correct period after + * disallowed words, fail-summary-fragment_ + */ + void foo11() {} } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriodAtEnd.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriodAtEnd.java index e57120d3d7e..81bfa4e948f 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriodAtEnd.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocPeriodAtEnd.java @@ -12,7 +12,7 @@ public class InputSummaryJavadocPeriodAtEnd { /** - * JAXB 1.0 only default validation event handler + * JAXB 1.0 only default validation event handler. */ public static final byte NUL = 0; // violation below 'Summary javadoc is missing.' @@ -66,4 +66,9 @@ public void foo5(){ public void foo6() { } + // violation below 'First sentence .* missing an ending period.' + /** + * JAXB 1.0 missing end period + */ + public static final byte ONE = 1; }