Issue #14689: Resolve Javadoc summary false positives

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() {}
 

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

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. */

src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ModifiedControlVariableCheck.java

@@ -408,7 +408,7 @@ private static Set<String> 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

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: "\\\\.". */

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;
  }
 
 }

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. */

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(

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.
+ * <p>
+ * Another sentence that is not part of the summary,
+ * so this should not matter: fail-summary-fragment.
+ */
+ void foo16() {}
+
  /**
  * This is summary java doc.
  * <a href="mailto:[email protected]"/>

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.'

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.'

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() {}
 }

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;
 }