Issue #14689: Fix Javadoc summary period handling

src/it/java/com/google/checkstyle/test/chapter7javadoc/rule72thesummaryfragment/SummaryJavadocTest.java

@@ -54,6 +54,7 @@ public void testIncorrect() throws Exception {
  "summary.javaDoc.missing");
 
  final String[] expected = {
+ "9: " + msgFirstSentence,
  "14: " + msgMissingDoc,
  "32: " + msgMissingDoc,
  "37: " + msgFirstSentence,

src/it/resources/com/google/checkstyle/test/chapter7javadoc/rule72thesummaryfragment/InputIncorrectSummaryJavaDocCheck.java

@@ -6,7 +6,7 @@
  */
 class InputIncorrectSummaryJavaDocCheck {
 
- /**
+/*warn*//**
  * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)}
  */
  void foo3() {}

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheck.java

@@ -19,10 +19,15 @@
 
 package com.puppycrawl.tools.checkstyle.checks.javadoc;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.BitSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 import com.puppycrawl.tools.checkstyle.StatelessCheck;
 import com.puppycrawl.tools.checkstyle.api.DetailNode;
@@ -48,7 +53,10 @@
  * Default value is {@code "^$"}.
  * </li>
  * <li>
- * Property {@code period} - Specify the period symbol at the end of first javadoc sentence.
+ * Property {@code period} - Specify the period symbol. Used to check the first sentence ends with a
+ * period. Periods that are not followed by a whitespace character are ignored (eg. the period in
+ * v1.0). Because some periods include whitespace built into the character, if this is set to a
+ * non-default value any period will end the sentence, whether it is followed by whitespace or not.
  * Type is {@code java.lang.String}.
  * Default value is {@code "."}.
  * </li>
@@ -154,7 +162,10 @@ public class SummaryJavadocCheck extends AbstractJavadocCheck {
  private Pattern forbiddenSummaryFragments = CommonUtil.createPattern("^$");
 
  /**
- * Specify the period symbol at the end of first javadoc sentence.
+ * Specify the period symbol. Used to check the first sentence ends with a period. Periods that
+ * are not followed by a whitespace character are ignored (eg. the period in v1.0). Because some
+ * periods include whitespace built into the character, if this is set to a non-default value
+ * any period will end the sentence, whether it is followed by whitespace or not.
  */
  private String period = DEFAULT_PERIOD;
 
@@ -169,7 +180,11 @@ public void setForbiddenSummaryFragments(Pattern pattern) {
  }
 
  /**
- * Setter to specify the period symbol at the end of first javadoc sentence.
+ * Setter to specify the period symbol. Used to check the first sentence ends with a period.
+ * Periods that are not followed by a whitespace character are ignored (eg. the period in v1.0).
+ * Because some periods include whitespace built into the character, if this is set to a
+ * non-default value any period will end the sentence, whether it is followed by whitespace or
+ * not.
  *
  * @param period period's value.
  * @since 6.2
@@ -218,14 +233,17 @@ 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)) {
- log(ast.getLineNumber(), MSG_SUMMARY_FIRST_SENTENCE);
+ if (summaryDoc.contains(period)) {
+ final String firstSentence = getFirstSentenceOrNull(ast, period);
+ if (firstSentence == null) {
+ log(ast.getLineNumber(), MSG_SUMMARY_FIRST_SENTENCE);
+ }
+ else if (containsForbiddenFragment(firstSentence)) {
+ log(ast.getLineNumber(), MSG_SUMMARY_JAVADOC);
+ }
  }
- if (endOfSentence != -1
- && containsForbiddenFragment(firstSentence.substring(0, endOfSentence))) {
- log(ast.getLineNumber(), MSG_SUMMARY_JAVADOC);
+ else {
+ log(ast.getLineNumber(), MSG_SUMMARY_FIRST_SENTENCE);
  }
  }
  }
@@ -576,31 +594,74 @@ private static String getStringInsideTag(String result, DetailNode detailNode) {
  }
 
  /**
- * Finds and returns first sentence.
+ * Finds and returns the first sentence.
  *
- * @param ast Javadoc root node.
- * @return first sentence.
+ * @param ast The Javadoc root node.
+ * @param period The configured period symbol.
+ * @return The first sentence up to and excluding the period, or null if no ending was found.
  */
- private static String getFirstSentence(DetailNode ast) {
- 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();
+ @Nullable private static String getFirstSentenceOrNull(DetailNode ast, String period) {
+ final List<String> sentenceParts = new ArrayList<>();
+ String sentence = null;
+ for (String text : (Iterable<String>) streamTextParts(ast)::iterator) {
+ final String sentenceEnding = findSentenceEndingOrNull(text, period);
+ if (sentenceEnding != null) {
+ sentenceParts.add(sentenceEnding);
+ sentence = String.join("", sentenceParts);
+ break;
  }
  else {
- text = getFirstSentence(child);
+ sentenceParts.add(text);
  }
+ }
+ return sentence;
+ }
 
- if (text.contains(periodSuffix)) {
- result.append(text, 0, text.indexOf(periodSuffix) + 1);
+ /**
+ * Streams through all the text under the given node.
+ *
+ * @param node The Javadoc node to examine.
+ * @return All the text in all nodes that have no child nodes.
+ */
+ private static Stream<String> streamTextParts(DetailNode node) {
+ final Stream<String> stream;
+ if (node.getChildren().length == 0) {
+ stream = Stream.of(node.getText());
+ }
+ else {
+ stream = Stream.of(node.getChildren())
+ .flatMap(SummaryJavadocCheck::streamTextParts);
+ }
+ return stream;
+ }
+
+ /**
+ * Finds the end of a sentence. If a sentence ending period was found, returns the whole string
+ * up to and excluding that period. The end of sentence detection here could be replaced in the
+ * future by Java's built-in BreakIterator class.
+ *
+ * @param text The string to search.
+ * @param period The period character to find.
+ * @return The string up to and excluding the period, or null if no ending was found.
+ */
+ @Nullable private static String findSentenceEndingOrNull(String text, String period) {
+ int periodIndex = text.indexOf(period);
+ String sentenceEnding = null;
+ while (periodIndex >= 0) {
+ final int afterPeriodIndex = periodIndex + period.length();
+
+ // Handle western period separately as it is only the end of a sentence if followed
+ // by whitespace. Other period characters often include whitespace in the character.
+ if (!DEFAULT_PERIOD.equals(period)
+ || afterPeriodIndex >= text.length()
+ || Character.isWhitespace(text.charAt(afterPeriodIndex))) {
+ sentenceEnding = text.substring(0, periodIndex);
  break;
  }
-
- result.append(text);
+ else {
+ periodIndex = text.indexOf(period, afterPeriodIndex);
+ }
  }
- return result.toString();
+ return sentenceEnding;
  }
-
 }

src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/javadoc/SummaryJavadocCheck.xml

@@ -20,7 +20,10 @@
  <description>Specify the regexp for forbidden summary fragments.</description>
  </property>
  <property default-value="." name="period" type="java.lang.String">
- <description>Specify the period symbol at the end of first javadoc sentence.</description>
+ <description>Specify the period symbol. Used to check the first sentence ends with a
+ period. Periods that are not followed by a whitespace character are ignored (eg. the period in
+ v1.0). Because some periods include whitespace built into the character, if this is set to a
+ non-default value any period will end the sentence, whether it is followed by whitespace or not.</description>
  </property>
  <property default-value="false"
  name="violateExecutionOnNonTightHtml"

src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/SummaryJavadocCheckTest.java

@@ -68,21 +68,22 @@ public void testInlineCorrect() throws Exception {
  @Test
  public void testIncorrect() throws Exception {
  final String[] expected = {
- "24: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "42: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "47: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
- "57: " + getCheckMessage(MSG_SUMMARY_JAVADOC),
- "63: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "68: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "79: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "93: " + getCheckMessage(MSG_SUMMARY_JAVADOC),
- "113: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "126: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
- "131: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "136: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "142: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
- "147: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "150: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "20: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "25: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "43: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "48: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "58: " + getCheckMessage(MSG_SUMMARY_JAVADOC),
+ "64: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "69: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "80: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "94: " + getCheckMessage(MSG_SUMMARY_JAVADOC),
+ "114: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "127: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "132: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "137: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "143: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "148: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "151: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
  };
  verifyWithInlineConfigParser(
  getPath("InputSummaryJavadocIncorrect.java"), expected);
@@ -130,19 +131,20 @@ public void testNoPeriod() throws Exception {
  @Test
  public void testDefaultConfiguration() throws Exception {
  final String[] expected = {
- "23: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "41: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "46: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
- "62: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "67: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "78: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "112: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "125: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
- "130: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "135: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "141: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
- "146: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
- "149: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "19: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "24: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "42: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "47: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "63: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "68: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "79: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "113: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "126: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "131: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "136: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "142: " + getCheckMessage(MSG_SUMMARY_FIRST_SENTENCE),
+ "147: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
+ "150: " + getCheckMessage(MSG_SUMMARY_JAVADOC_MISSING),
  };
 
  verifyWithInlineConfigParser(
@@ -230,12 +232,31 @@ 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(
  getPath("InputSummaryJavadocPeriodAtEnd.java"), expected);
  }
 
+ @Test
+ public void testForbiddenFragmentRelativeToPeriod() throws Exception {
+ final String[] expected = {
+ "23: " + getCheckMessage(MSG_SUMMARY_JAVADOC),
+ };
+
+ verifyWithInlineConfigParser(
+ getPath("InputSummaryJavadocForbiddenFragmentRelativeToPeriod.java"), expected);
+ }
+
+ @Test
+ public void testJapanesePeriod() throws Exception {
+ final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
+
+ verifyWithInlineConfigParser(
+ getPath("InputSummaryJavadocJapanesePeriod.java"), expected);
+ }
+
  @Test
  public void testHtmlFormatSummary() throws Exception {
  final String[] expected = {

src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocForbiddenFragmentRelativeToPeriod.java

@@ -0,0 +1,28 @@
+/*
+SummaryJavadoc
+violateExecutionOnNonTightHtml = (default)false
+forbiddenSummaryFragments = ^$|fail-summary-fragment
+period = 。
+
+
+*/
+
+package com.puppycrawl.tools.checkstyle.checks.javadoc.summaryjavadoc;
+
+public class InputSummaryJavadocForbiddenFragmentRelativeToPeriod {
+
+ /**
+ * 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 foo1() {}
+
+ // 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 foo2() {}
+}

src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect.java

@@ -16,6 +16,7 @@
  */
 class InputSummaryJavadocIncorrect {
 
+ // violation below 'First sentence .* missing an ending period.'
  /**
  * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)}
  */

src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocIncorrect2.java

@@ -15,6 +15,7 @@
  */
 class InputSummaryJavadocIncorrect2 {
 
+ // violation below 'First sentence .* missing an ending period.'
  /**
  * As of JDK 1.1, replaced by {@link #setBounds(int,int,int,int)}
  */

src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/summaryjavadoc/InputSummaryJavadocJapanesePeriod.java

@@ -0,0 +1,24 @@
+/*
+SummaryJavadoc
+violateExecutionOnNonTightHtml = (default)false
+forbiddenSummaryFragments = (default)^$
+period = 。
+
+
+*/
+
+package com.puppycrawl.tools.checkstyle.checks.javadoc.summaryjavadoc;
+
+public class InputSummaryJavadocJapanesePeriod {
+
+ /**
+ * Summary sentence ending with correct period and no following whitespace。The Japanese
+ * period has whitespace built in!
+ */
+ void foo1() {}
+
+ /**
+ * 要約文。別の文。
+ */
+ void foo2() {}
+}