Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #15685: fixed JavadocParagraph to detect paragraph tag when they have their corresponding closing tag #15686

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci/validation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ no-error-pgjdbc)
checkout_from https://github.com/pgjdbc/pgjdbc.git
cd .ci-temp/pgjdbc
# pgjdbc easily damage build, we should use stable versions
git checkout "cf7afb6c210b6841a232ef27591230fc78a""fa6a5"
git checkout "fcc13e70e6b6bb64b848df4b4ba6b3566b5""e95a3"
./gradlew --no-parallel --no-daemon checkstyleAll \
-PenableMavenLocal -Pcheckstyle.version="${CS_POM_VERSION}"
cd ../
Expand Down
12 changes: 12 additions & 0 deletions config/checkstyle-checks.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@
<property name="files"
value=".*[\\/]src[\\/](test|it|xdocs-examples)[\\/].*(?&lt;!Support)\.java"/>
</module>
<!--
Until we fully support opening-closing paragraph tag
https://github.com/checkstyle/checkstyle/issues/15763
-->
<module name="SuppressionSingleFilter">
<property name="checks" value="JavadocParagraph"/>
<property name="message" value="tag should be preceded with an empty line."/>
</module>
<module name="SuppressionSingleFilter">
<property name="checks" value="JavadocParagraph"/>
<property name="message" value="Redundant .* tag."/>
</module>
<module name="SuppressWarningsFilter"/>
<module name="SuppressWithPlainTextCommentFilter">
<!--
Expand Down
2 changes: 2 additions & 0 deletions config/jsoref-spellchecker/whitelist.words
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ detailnodetreestringprinter
devops
Dexec
Dexpression
df
dfa
DFFF
Dfile
Expand Down Expand Up @@ -404,6 +405,7 @@ Fannotation
favicon
Fblocks
FCBL
fcc
FCCD
Fchecks
Fcheckstyle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class InputIncorrectJavadocParagraph {
*
* <p>Some Javadoc.
*
* @see <a href="example.com">
* Documentation about GWT emulated source</a>
* @see <a href="example.com">Documentation about GWT emulated source</a>
*/
boolean emulated() {
return false;
Expand Down Expand Up @@ -82,13 +81,12 @@ class InnerInputCorrectJavaDocParagraphCheck {
*
* <p>
* Some Javadoc.<p>
* @see <a href="example.com">
* Documentation about GWT emulated source</a>
* @see <a href="example.com">Documentation about GWT emulated source</a>
*/
// 2 violations 4 lines above:
// 2 violations 3 lines above:
// '<p> tag should be placed immediately before the first word'
// '<p> tag should be preceded with an empty line.'
// violation 6 lines above 'Javadoc tag '@see' should be preceded with an empty line.'
// violation 5 lines above 'Javadoc tag '@see' should be preceded with an empty line.'
boolean emulated() {
return false;
}
Expand Down Expand Up @@ -116,8 +114,7 @@ boolean emulated() {
*
* <p> Some Javadoc.
*
* @see <a href="example.com">
* Documentation about <p> GWT emulated source</a>
* @see <a href="example.com">Documentation about <p> GWT emulated source</a>
*/
// 2 violations 2 lines above:
// '<p> tag should be placed immediately before the first word'
Expand All @@ -127,14 +124,16 @@ boolean emulated() {
}
};

/* 4 lines below, no violation until #15685 */
/**
* Some summary.
*
* <p><h1>Testing...</h1></p>
*/
// violation 2 lines above '<p> tag should not precede HTML block-tag '<h1>''
class InnerPrecedingPtag {
/* 5 lines below, no violation until #15685 */
// 2 violations 6 lines below:
// '<p> tag should be placed immediately before the first word'
// '<p> tag should not precede HTML block-tag '<ul>''
/**
* Some summary.
*
Expand All @@ -148,7 +147,9 @@ class InnerPrecedingPtag {
*/
public void foo() {}
romani marked this conversation as resolved.
Show resolved Hide resolved

/* 5 lines below, no violation until #15685 */
// 2 violations 6 lines below:
// '<p> tag should be placed immediately before the first word'
// '<p> tag should not precede HTML block-tag '<table>''
/**
* Some summary.
*
Expand All @@ -159,7 +160,9 @@ public void foo() {}
*/
public void fooo() {}

/* 5 lines below, no violation until #15685 */
// 2 violations 6 lines below:
// '<p> tag should be placed immediately before the first word'
// '<p> tag should not precede HTML block-tag '<pre>''
/**
* Some summary.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,7 @@ public final class TokenTypes {
* <pre>
* new ArrayList(50);
* </pre>
* <p> parses as:</p>
* <p>parses as:</p>
* <pre>
* |--EXPR -&gt; EXPR
* | `--LITERAL_NEW -&gt; new
Expand Down Expand Up @@ -5184,7 +5184,7 @@ public final class TokenTypes {

/**
* The type that refers to all types. This node has no children.
* <p> For example: </p>
* <p>For example: </p>
* <pre>
*
* List&lt;?&gt; list;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
* <a href="https://www.w3schools.com/html/html_blocks.asp">HTML block-tag</a>,
* nested paragraph tags are allowed to do that.</li>
* </ul>
* <p><b>ATTENTION:</b></p>
* <p>This Check ignores HTML comments.</p>
* <p>The Check ignores all the nested paragraph tags,
* it will not give any kind of violation if the paragraph tag is nested.</p>
* <ul>
* <li>
* Property {@code allowNewlineParagraph} - Control whether the &lt;p&gt; tag
Expand Down Expand Up @@ -174,7 +178,8 @@ public void visitJavadocToken(DetailNode ast) {
checkEmptyLine(ast);
}
else if (ast.getType() == JavadocTokenTypes.HTML_ELEMENT
&& JavadocUtil.getFirstChild(ast).getType() == JavadocTokenTypes.P_TAG_START) {
&& (JavadocUtil.getFirstChild(ast).getType() == JavadocTokenTypes.P_TAG_START
|| JavadocUtil.getFirstChild(ast).getType() == JavadocTokenTypes.PARAGRAPH)) {
checkParagraphTag(ast);
}
}
Expand All @@ -198,25 +203,49 @@ private void checkEmptyLine(DetailNode newline) {
* @param tag html tag.
*/
private void checkParagraphTag(DetailNode tag) {
final DetailNode newLine = getNearestEmptyLine(tag);
if (isFirstParagraph(tag)) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_REDUNDANT_PARAGRAPH);
}
else if (newLine == null || tag.getLineNumber() - newLine.getLineNumber() != 1) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_LINE_BEFORE);
}
if (!isNestedParagraph(tag)) {
final DetailNode newLine = getNearestEmptyLine(tag);
if (isFirstParagraph(tag)) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_REDUNDANT_PARAGRAPH);
}
else if (newLine == null || tag.getLineNumber() - newLine.getLineNumber() != 1) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_LINE_BEFORE);
}

final String blockTagName = findFollowedBlockTagName(tag);
if (blockTagName != null) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_PRECEDED_BLOCK_TAG, blockTagName);
}
final String blockTagName = findFollowedBlockTagName(tag);
if (blockTagName != null) {
log(tag.getLineNumber(), tag.getColumnNumber(),
MSG_PRECEDED_BLOCK_TAG, blockTagName);
}

if (!allowNewlineParagraph && isImmediatelyFollowedByNewLine(tag)) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_MISPLACED_TAG);
if (!allowNewlineParagraph && isImmediatelyFollowedByNewLine(tag)) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_MISPLACED_TAG);
}
if (isImmediatelyFollowedByText(tag)) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_MISPLACED_TAG);
}
}
if (isImmediatelyFollowedByText(tag)) {
log(tag.getLineNumber(), tag.getColumnNumber(), MSG_MISPLACED_TAG);
}

/**
* Determines whether the paragraph tag is nested.
*
* @param tag html tag.
* @return true, if the paragraph tag is nested.
*/
private static boolean isNestedParagraph(DetailNode tag) {
boolean nested = false;
DetailNode parent = tag;

while (parent != null) {
if (parent.getType() == JavadocTokenTypes.PARAGRAPH) {
nested = true;
break;
}
parent = parent.getParent();
}

return nested;
}

/**
Expand Down Expand Up @@ -245,9 +274,11 @@ private static String findFollowedBlockTagName(DetailNode tag) {
*/
@Nullable
private static DetailNode findFirstHtmlElementAfter(DetailNode tag) {
DetailNode htmlElement = JavadocUtil.getNextSibling(tag);
DetailNode htmlElement = getNextSibling(tag);

while (htmlElement != null && htmlElement.getType() != JavadocTokenTypes.HTML_ELEMENT) {
while (htmlElement != null
&& htmlElement.getType() != JavadocTokenTypes.HTML_ELEMENT
&& htmlElement.getType() != JavadocTokenTypes.HTML_TAG) {
if ((htmlElement.getType() == JavadocTokenTypes.TEXT
|| htmlElement.getType() == JavadocTokenTypes.JAVADOC_INLINE_TAG)
&& !CommonUtil.isBlank(htmlElement.getText())) {
Expand All @@ -268,7 +299,13 @@ private static DetailNode findFirstHtmlElementAfter(DetailNode tag) {
*/
@Nullable
private static String getHtmlElementName(DetailNode htmlElement) {
final DetailNode htmlTag = JavadocUtil.getFirstChild(htmlElement);
final DetailNode htmlTag;
if (htmlElement.getType() == JavadocTokenTypes.HTML_TAG) {
htmlTag = htmlElement;
}
else {
htmlTag = JavadocUtil.getFirstChild(htmlElement);
}
final DetailNode htmlTagFirstChild = JavadocUtil.getFirstChild(htmlTag);
final DetailNode htmlTagName =
JavadocUtil.findFirstToken(htmlTagFirstChild, JavadocTokenTypes.HTML_TAG_NAME);
Expand Down Expand Up @@ -364,7 +401,8 @@ private static DetailNode getNearestEmptyLine(DetailNode node) {
* @return true, if the paragraph tag is immediately followed by the text.
*/
private static boolean isImmediatelyFollowedByText(DetailNode tag) {
final DetailNode nextSibling = JavadocUtil.getNextSibling(tag);
final DetailNode nextSibling = getNextSibling(tag);

return nextSibling.getType() == JavadocTokenTypes.EOF
|| nextSibling.getText().startsWith(" ");
}
Expand All @@ -373,10 +411,35 @@ private static boolean isImmediatelyFollowedByText(DetailNode tag) {
* Tests whether the paragraph tag is immediately followed by the new line.
*
* @param tag html tag.
* @return true, if the paragraph tag is immediately followed by the text.
* @return true, if the paragraph tag is immediately followed by the new line.
*/
private static boolean isImmediatelyFollowedByNewLine(DetailNode tag) {
final DetailNode nextSibling = JavadocUtil.getNextSibling(tag);
return nextSibling.getType() == JavadocTokenTypes.NEWLINE;
return getNextSibling(tag).getType() == JavadocTokenTypes.NEWLINE;
}

/**
* Custom getNextSibling method to handle different types of paragraph tag.
* It works for both {@code <p>} and {@code <p></p>} tags.
*
* @param tag HTML_ELEMENT tag.
* @return next sibling of the tag.
*/
private static DetailNode getNextSibling(DetailNode tag) {
DetailNode nextSibling;

if (JavadocUtil.getFirstChild(tag).getType() == JavadocTokenTypes.PARAGRAPH) {
final DetailNode paragraphToken = JavadocUtil.getFirstChild(tag);
final DetailNode paragraphStartTagToken = JavadocUtil.getFirstChild(paragraphToken);
nextSibling = JavadocUtil.getNextSibling(paragraphStartTagToken);
}
else {
nextSibling = JavadocUtil.getNextSibling(tag);
}

if (nextSibling.getType() == JavadocTokenTypes.HTML_COMMENT) {
nextSibling = JavadocUtil.getNextSibling(nextSibling);
}

return nextSibling;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
&lt;li&gt;First paragraph tag should not precede
&lt;a href="https://www.w3schools.com/html/html_blocks.asp"&gt;HTML block-tag&lt;/a&gt;,
nested paragraph tags are allowed to do that.&lt;/li&gt;
&lt;/ul&gt;</description>
&lt;/ul&gt;
&lt;p&gt;&lt;b&gt;ATTENTION:&lt;/b&gt;&lt;/p&gt;
&lt;p&gt;This Check ignores HTML comments.&lt;/p&gt;
&lt;p&gt;The Check ignores all the nested paragraph tags,
it will not give any kind of violation if the paragraph tag is nested.&lt;/p&gt;</description>
<properties>
<property default-value="true" name="allowNewlineParagraph" type="boolean">
<description>Control whether the &amp;lt;p&amp;gt; tag
Expand Down
Loading
Loading