Issue #15503: Added support for violation of p tag if it precedes a block tag #15710

Zopsss · 2024-09-26T17:45:40Z

added support to check and violate block-tag if they're followed by p tag.

created a new violation message for it.

wrote new test cases.

took the HTML tags list from: https://checkstyle.sourceforge.io/checks/javadoc/javadocstyle.html#JavadocStyle

Check for allowed HTML tags. The list of allowed HTML tags is "a", "abbr", "acronym", "address", "area", "b", "bdo", "big", "blockquote", "br", "caption", "cite", "code", "colgroup", "dd", "del", "dfn", "div", "dl", "dt", "em", "fieldset", "font", "h1", "h2", "h3", "h4", "h5", "h6", "hr", "i", "img", "ins", "kbd", "li", "ol", "p", "pre", "q", "samp", "small", "span", "strong", "sub", "sup", "table", "tbody", "td", "tfoot", "th", "thead", "tr", "tt", "u", "ul", "var".

and filtered out block-level tags.

Diff Regression config: https://gist.githubusercontent.com/Zopsss/013f379d4c821f2d624be56212a64575/raw/5f17ec9186d4421396429ab38cbac3e4663bf4eb/javadocparagraph_config.xml
Diff Regression projects: https://raw.githubusercontent.com/checkstyle/contribution/master/checkstyle-tester/projects-to-test-on-for-github-action.properties

Zopsss · 2024-09-26T18:31:39Z

Github, generate report

github-actions · 2024-09-26T18:33:37Z

Report generation failed on phase "make_report",
step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/11058159106

Zopsss · 2024-09-26T18:58:32Z

Github, generate report

Zopsss · 2024-09-27T06:49:13Z

Github, generate report

Zopsss · 2024-09-27T06:50:38Z

...google/checkstyle/test/chapter7javadoc/rule712paragraphs/InputIncorrectJavadocParagraph.java

-  /* 4 lines below, no violation until #15503 */
+  /* 4 lines below, no violation until #15685 */


had to change issue number to #15685 because CI was complaining. These false-negatives will be covered as part of newly linked issue as they have their corresponding closing tag

github-actions · 2024-09-27T07:05:30Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/3aa7ee5_2024070511/reports/diff/index.html

Zopsss · 2024-09-27T09:39:28Z

Github, generate report

github-actions · 2024-09-27T10:06:38Z

Report generation failed on phase "make_report",
step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/11068301809

Zopsss · 2024-09-28T08:57:12Z

Github, generate report

github-actions · 2024-09-28T08:59:50Z

Report generation failed on phase "make_report",
step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/11082649268

Zopsss · 2024-09-28T09:07:15Z

Github, generate report

romani

items:

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

src/main/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/messages_ru.properties

src/main/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/messages.properties

romani

items:

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

...ycrawl/tools/checkstyle/checks/javadoc/javadocparagraph/InputJavadocParagraphIncorrect2.java

romani · 2024-09-29T04:56:13Z

Pitest should be fixed, we can not merge in red CI

romani

Items

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

romani · 2024-09-30T22:05:51Z

@Zopsss , please rebase

romani

item:

...eckstyle/test/chapter7javadoc/rule712paragraphs/InputFormattedIncorrectJavadocParagraph.java

romani

Items

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

Zopsss · 2024-10-02T16:59:17Z

github, generate report

Zopsss · 2024-10-02T17:48:11Z

ci passed 🎉

github-actions · 2024-10-02T17:56:46Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/index.html

Zopsss · 2024-10-02T18:12:32Z

the check isn't aware of comments, found a case like that at:

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/apache-struts/index.html#A5

* <p>
* <!-- START SNIPPET: params -->
*
* <ul>

this is good. This should still be considered as a violation

Zopsss · 2024-10-02T18:18:12Z

report looks good, no false-positives.

romani · 2024-10-02T23:07:34Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/pmd/index.html#A1

123     /**
124      * Parses a qualified name given in the format defined for this implementation. The format
125      * is specified by a regex pattern (see {@link #FORMAT}). Examples:
126      *
127      * <p>{@code com.company.MyClass$Nested#myMethod(String, int)}
128      * <ul>
129      * <li> Packages are separated by full stops;
130      * <li> Nested classes are separated by a dollar symbol;
131      * <li> The optional method suffix is separated from the class with a hashtag;
132      * <li> Method arguments are separated by a comma and a single space.
133      * </ul>
134      *
135      * <p>

There is content between P tag and UL tag, so it false-positive, no violation should be there.

romani · 2024-10-03T01:08:52Z

False positive
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/spring-framework/index.html#A8

*
88   * <p><b>Render Request:</b><p>
89   * <ol>

romani · 2024-10-03T01:40:49Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/apache-struts/index.html#A1

* <p>
55   * <!-- START SNIPPET: params -->
56   * <ul>
57   *      <li>id (String) - the id (if specified) to put the action under stack's context.
58   *      <li>name* (String) - name of the action to be executed (without the extension suffix eg. .action)</li>
59   *      <li>namespace (String) - default to the namespace where this action tag is invoked</li>
60   *      <li>executeResult (Boolean) -  default is false. Decides whether the result of this action is to be executed or not</li>
61   *      <li>ignoreContextParams (Boolean) - default to false. Decides whether the request parameters are to be included when the action is invoked</li>
62   * </ul>
63

Ideally this should continue to be violation , there are some content that is comment tag. We should skip it. Please add this case to Inputs , it should be violated

romani · 2024-10-03T01:46:46Z

False positive
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/openjdk21/index.html#A50

156  * See {@link ZoneInfo#transitions ZoneInfo.transitions} about the value.
157  *
158  * <p><strong>1.2 <code>offset_table</code> structure</strong><p>
159  * <blockquote>
160  * <pre>
161  *    offset_table {
162  *      u1      tag;              // 0x05 : constant
163  *      u2      length;           // byte length of whole values
164  *      s4      value[length/4];  // offset values in `int'
165  *    }
166  * </pre>
167  * </blockquote>

Zopsss · 2024-10-03T04:30:49Z

False positive
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/spring-framework/index.html#A8
*
88 * Render Request:
89 * <ol>

This isn't false-positive. There's a second opening  after  and in next line there's a block tag <ol>, so this should be a violation. If there was a closing paragraph tag instead of second opening paragraph tag then it wouldn't cause violation.

Zopsss · 2024-10-03T04:34:09Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/pmd/index.html#A1
123 /**
124 * Parses a qualified name given in the format defined for this implementation. The format
125 * is specified by a regex pattern (see {@link #FORMAT}). Examples:
126 *
127 * {@code com.company.MyClass$Nested#myMethod(String, int)}
128 * <ul>
129 * <li> Packages are separated by full stops;
130 * <li> Nested classes are separated by a dollar symbol;
131 * <li> The optional method suffix is separated from the class with a hashtag;
132 * <li> Method arguments are separated by a comma and a single space.
133 * </ul>
134 *
135 * 
There is content between P tag and UL tag, so it false-positive, no violation should be there.

Ummm {@code com.company.MyClass$Nested#myMethod(String, int)} does not have its corresponding closing tag, instead there's a block tag after it. If closing paragraph tag was present before <ul> then it would be fine, there would be no violation.

But I'll still look into this. Thanks for pointing this out

Zopsss · 2024-10-03T04:39:42Z

False positive
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/63facba_2024175613/reports/diff/openjdk21/index.html#A50
156 * See {@link ZoneInfo#transitions ZoneInfo.transitions} about the value.
157 *
158 * 1.2 <code>offset_table</code> structure
159 * <blockquote>
160 * <pre>
161 * offset_table {
162 * u1 tag; // 0x05 : constant
163 * u2 length; // byte length of whole values
164 * s4 value[length/4]; // offset values in `int'
165 * }
166 * </pre>
167 * </blockquote>

The violation is for

<p>
> 159  * <blockquote>

Not for

> 158  * <p><strong>1.2 <code>offset_table</code> structure</strong>

The blockquote tag is a block-tag and it is preceded by an opening tag so there should be violation for it.

Zopsss · 2024-10-03T04:42:01Z

I guess this check needs to have column number in its violation. Otherwise it can confuse users.

You can see in the above comments, javadocs have multiple opening paragraph tags on same line and on next line they have block-tag. So check gives violation for it but the violation message only contains line number not the column number so it can confuse users about which paragraph tag is causing the violation.

rdiachenko

lgtm

Zopsss · 2024-10-03T10:11:18Z

...ycrawl/tools/checkstyle/checks/javadoc/javadocparagraph/InputJavadocParagraphIncorrect5.java

+     * <p>
+     *  <!-- THIS COMMENT WILL GET IGNORED -->
+     *  <ul>
+     *      <li>Item 1</li>
+     *      <li>Item 2</li>
+     *      <li>Item 3</li>
+     *  </ul>


test case added #15710 (comment)

Zopsss · 2024-10-03T10:12:57Z

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

+        while (htmlElement != null && htmlElement.getType() != JavadocTokenTypes.HTML_ELEMENT) {
+            if ((htmlElement.getType() == JavadocTokenTypes.TEXT
+                    || htmlElement.getType() == JavadocTokenTypes.JAVADOC_INLINE_TAG)
+                    && !CommonUtil.isBlank(htmlElement.getText())) {
+                htmlElement = null;
+                break;
+            }


added new line: || htmlElement.getType() == JavadocTokenTypes.JAVADOC_INLINE_TAG) to fix the false-positive found at:

#15710 (comment)

…ecedes a block tag

Zopsss · 2024-10-03T10:16:33Z

...ycrawl/tools/checkstyle/checks/javadoc/javadocparagraph/InputJavadocParagraphIncorrect5.java

+    /**
+     * Some Summary.
+     *
+     * <p>{@code com.company.MyClass$Nested#myMethod(String, int)}
+     * <ul>
+     *     <li>Item 1</li>
+     *     <li>Item 2</li>
+     *     <li>Item 3</li>
+     * </ul>
+     */
+    private static final String NAME2 = "Jesse Pinkman";


test case added #15710 (comment)

romani · 2024-10-03T12:31:55Z

GitHub, generate report for configs in PR description

github-actions · 2024-10-03T13:21:37Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/6c9265fe31e17a7cce84e4bef58d96667f1c4138_2024132116/reports/diff/index.html

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch 3 times, most recently from b9d2992 to 3aa7ee5 Compare September 27, 2024 06:41

Zopsss commented Sep 27, 2024

View reviewed changes

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch from 3aa7ee5 to a9cec53 Compare September 27, 2024 09:37

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch from a9cec53 to 3eb627a Compare September 27, 2024 09:53

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch from 3eb627a to 19ad95a Compare September 27, 2024 15:33

romani requested changes Sep 28, 2024

View reviewed changes

romani self-assigned this Sep 28, 2024

romani requested changes Sep 28, 2024

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocParagraphCheck.java Outdated Show resolved Hide resolved

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocParagraphCheck.java Outdated Show resolved Hide resolved

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch from 19ad95a to 4315c39 Compare September 28, 2024 19:25

Zopsss commented Sep 28, 2024

View reviewed changes

...ycrawl/tools/checkstyle/checks/javadoc/javadocparagraph/InputJavadocParagraphIncorrect2.java Outdated Show resolved Hide resolved

romani requested changes Sep 29, 2024

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocParagraphCheck.java Outdated Show resolved Hide resolved

romani requested changes Oct 1, 2024

View reviewed changes

...eckstyle/test/chapter7javadoc/rule712paragraphs/InputFormattedIncorrectJavadocParagraph.java Outdated Show resolved Hide resolved

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch from 4315c39 to 70980bf Compare October 1, 2024 13:45

romani requested changes Oct 1, 2024

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocParagraphCheck.java Outdated Show resolved Hide resolved

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch 2 times, most recently from 03f5d5e to 5a5b1b4 Compare October 2, 2024 04:30

romani assigned rdiachenko and unassigned romani Oct 2, 2024

rdiachenko approved these changes Oct 3, 2024

View reviewed changes

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch 2 times, most recently from cef359b to 730221a Compare October 3, 2024 10:10

Zopsss commented Oct 3, 2024

View reviewed changes

Zopsss force-pushed the 15503-violate-for-extra-preceding-paragraph branch from 730221a to 6c9265f Compare October 3, 2024 10:15

Issue checkstyle#15503: Added support for violation of p tag if it pr…

6c9265f

…ecedes a block tag

Zopsss commented Oct 3, 2024

View reviewed changes

Zopsss mentioned this pull request Oct 3, 2024

minor: fixed P tag content's placement in CauseParameterInExceptionCheck.java sevntu-checkstyle/sevntu.checkstyle#1037

Merged

This was referenced Oct 3, 2024

JavadocParagraph: validation with comment inside P tag #15732

Closed

JavadocParagraph: report violation with column #15733

Closed

romani merged commit aeaf4b0 into checkstyle:master Oct 3, 2024
112 checks passed

Zopsss deleted the 15503-violate-for-extra-preceding-paragraph branch October 3, 2024 14:04

		/* 4 lines below, no violation until #15503 */
		/* 4 lines below, no violation until #15685 */

Issue #15503: Added support for violation of p tag if it precedes a block tag #15710

Issue #15503: Added support for violation of p tag if it precedes a block tag #15710

Conversation

Zopsss commented Sep 26, 2024 • edited Loading

Zopsss commented Sep 26, 2024

github-actions bot commented Sep 26, 2024

Zopsss commented Sep 26, 2024

Zopsss commented Sep 27, 2024

Zopsss Sep 27, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 27, 2024

Zopsss commented Sep 27, 2024

github-actions bot commented Sep 27, 2024

Zopsss commented Sep 28, 2024

github-actions bot commented Sep 28, 2024

Zopsss commented Sep 28, 2024

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani commented Sep 29, 2024

romani left a comment

Choose a reason for hiding this comment

romani commented Sep 30, 2024

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

Zopsss commented Oct 2, 2024

Zopsss commented Oct 2, 2024

github-actions bot commented Oct 2, 2024

Zopsss commented Oct 2, 2024 • edited Loading

Zopsss commented Oct 2, 2024

romani commented Oct 2, 2024

romani commented Oct 3, 2024

romani commented Oct 3, 2024

romani commented Oct 3, 2024

Zopsss commented Oct 3, 2024

Zopsss commented Oct 3, 2024

Zopsss commented Oct 3, 2024

Zopsss commented Oct 3, 2024 • edited Loading

rdiachenko left a comment

Choose a reason for hiding this comment

Zopsss Oct 3, 2024

Choose a reason for hiding this comment

Zopsss Oct 3, 2024

Choose a reason for hiding this comment

Zopsss Oct 3, 2024

Choose a reason for hiding this comment

romani commented Oct 3, 2024

github-actions bot commented Oct 3, 2024

Zopsss commented Sep 26, 2024 •

edited

Loading

Zopsss commented Oct 2, 2024 •

edited

Loading

Zopsss commented Oct 3, 2024 •

edited

Loading