Skip to content

Issue #17697: Add check for box comments#18986

Merged
romani merged 1 commit into
checkstyle:masterfrom
smita1078:BoxComments
May 19, 2026
Merged

Issue #17697: Add check for box comments#18986
romani merged 1 commit into
checkstyle:masterfrom
smita1078:BoxComments

Conversation

@smita1078

@smita1078 smita1078 commented Feb 22, 2026

Copy link
Copy Markdown
Contributor

#17697

Adds detection for comments enclosed in boxes drawn with special characters, providing partial coverage for Google Java Style Guide §4.8.6: "Comments are not enclosed in boxes drawn with asterisks or other characters."

Implementation
Uses the existing TodoComment check with a custom configuration:
Check ID: BoxComments
Regex Pattern: ^[ \t]*([*=#-])\1{8,}[ \t]*$
Detection: Lines with 9+ consecutive repeated characters (*, =, #)

Changes
Configuration
Modified: src/main/resources/google_checks.xml
Added TodoComment module with id="BoxComments"
Added suppressions for test files with intentional box-style comments

Documentation
Modified: src/xdocs/google_style.xml
Added row for §4.8.6.3 showing partial coverage
Modified: src/xdocs/checks/misc/todocomment.xml
Added Example 3 demonstrating box comment detection

Created: src/xdocs/resources/com/puppycrawl/tools/checkstyle/checks/todocomment/Example3.java
Example configuration and test cases

Tests
Created: src/it/java/com/google/checkstyle/test/chapter4formatting/rule4863boxcomments/BoxCommentsTest.java

Integration test for Google style
Created: src/it/resources/com/google/checkstyle/test/chapter4formatting/rule4863boxcomments/InputBoxComments.java

Test input with various box comment patterns
Modified: src/test/java/com/puppycrawl/tools/checkstyle/checks/TodoCommentCheckExamplesTest.java
Added testExample3() for box comment examples

Diff Regression config: https://raw.githubusercontent.com/smita1078/checkstyle/BoxComments/src/main/resources/google_checks.xml
Diff Regression projects: https://gist.githubusercontent.com/smita1078/f26521cfc945a2c966396eabd09bc1e8/raw/8d1de4376f538633f1d9f394494ba7315b40d81f/projects-google-style.properties
Report label: BoxComments check on Google Style projects

@smita1078 smita1078 force-pushed the BoxComments branch 3 times, most recently from 3bb84ad to 7e11c64 Compare February 22, 2026 11:12
@smita1078

Copy link
Copy Markdown
Contributor Author

github, generate site

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please run diff report on project: guava, comuda (they use google style)

items


void myFunc4() {
// #########
// violation above 'Comment should not be enclosed in a box.'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we cannot detect box reliably, we should change wording of violation to something like (wording can be improved)
Comment should not be repetitive usage of '*' , '=', '#' that usually used in box like comments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did try this violation message, but such errors were there..

examples\resources\com\puppycrawl\tools\checkstyle\checks\todocomment\Example3.java:8: Line is longer than 85 characters (found 88). [LineLength]
[ERROR] [checkstyle] [ERROR] C:\Users\Smita Prajapati\checkstyle\src\xdocs-examples\resources\com\puppycrawl\tools\checkstyle\checks\todocomment\Example3.java:17: Discouraged usage of escaped special regex symbols. 

Have changed it to Comment uses box-like repetitive character pattern. to better fit the use-case

@smita1078 smita1078 force-pushed the BoxComments branch 2 times, most recently from e054f0a to 9b3b087 Compare February 24, 2026 13:36
@smita1078

Copy link
Copy Markdown
Contributor Author

GitHub, generate report

@github-actions

Copy link
Copy Markdown
Contributor

Report generation failed on phase "make_report",
step "Prepare environment".
Link: https://github.com/checkstyle/checkstyle/actions/runs/22354819899

@smita1078

Copy link
Copy Markdown
Contributor Author

GitHub, generate report

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

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

@smita1078

Copy link
Copy Markdown
Contributor Author

@romani please review

@smita1078

smita1078 commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

@romani ping

@smita1078

Copy link
Copy Markdown
Contributor Author

@romani @Zopsss ping

Comment thread src/main/resources/google_checks.xml
Comment thread src/site/xdoc/checks/misc/todocomment.xml Outdated
@smita1078

Copy link
Copy Markdown
Contributor Author

@romani plz take a look

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

items:

Comment thread src/site/resources/styleguides/google-java-style-20250426/javaguide.html Outdated
Comment thread src/main/resources/google_checks.xml Outdated
@romani

romani commented May 17, 2026

Copy link
Copy Markdown
Member

CI is red

@smita1078 smita1078 force-pushed the BoxComments branch 4 times, most recently from 4247698 to 766d786 Compare May 17, 2026 21:40
@smita1078 smita1078 force-pushed the BoxComments branch 2 times, most recently from eaf3dd1 to 9d6fdbe Compare May 17, 2026 22:35
@smita1078

Copy link
Copy Markdown
Contributor Author

Done

"checks/regexp/regexpmultiline/Example5",
"checks/sizes/lambdabodylength/Example2",
"checks/sizes/linelength/Example6",
"checks/todocomment/Example3",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some incomplete idea.
What if we have two sections for examples. One section will be to show all properties in action and all code can be same. And another section to random useful use cases that we just share and code for it can be any, do not need to same as others.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way:

// xdoc section -- start
// Section 1: property showcase (consistent structure)
class ExampleN {
  // same fields, same methods across all examples
}
// xdoc section -- end

// xdoc section (use-case) -- start  
// Section 2: real-world use case 
class UseCaseN {
  // whatever makes sense for this specific config
}
// xdoc section (use-case) -- end

we can have this structure as I feel it would make examples to be more readable and complex unification wont be required much. but need to discuss for below :
will both sections appear in the generated xml?
will violation test run against both?

@romani romani May 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually through on section in xdoc/html, that reuse the same macroses, but fully excluded from validation on same non comment AST..

https://checkstyle.org/checks/misc/todocomment.html#Examples is for same non comment AST .

And under it new section. "Use cases".
With examples of any description and config and java codes. In our case it will be https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/7e11c64_20260222171827/checks/misc/todocomment.html#Example3-config

@smita1078 smita1078 May 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am not wrong is this what you are suggesting:

Section 1 — "Examples" (existing, same as now):
Same xdoc markers, same macros
All examples MUST have identical non-comment AST
XdocsExamplesAstConsistencyTest validates this section
Example: https://checkstyle.org/checks/misc/todocomment.html#Examples

Section 2 — "Use Cases" (new):
New section in the HTML page, below Examples
Uses same macros/rendering pipeline , just a new heading
BUT fully excluded from XdocsExamplesAstConsistencyTest
Any code structure, any config, no consistency requirement
Example: https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/7e11c64_20260222171827/checks/misc/todocomment.html#Example3-config

So from implementation perspective:
No new xdoc markers needed in Java files
New section heading in the .xml doc file
XdocsExamplesAstConsistencyTest gets a new set like USE_CASE_EXAMPLES that are excluded from validation
Same rendering, just organized differently on the page

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you got it.
Just new title of section, below it examples from real life.

If you agree and no problems you see, we can create separate issue and do it later.

This PR we will merge as is for now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can create a separate issue for this.

My only concern is whether we should first finish the long-running suppression issue #18435 and then introduce this new "Use Cases" section approach, or handle both in parallel since this would also reduce the need for many suppressions and AST consistency workarounds.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create new issue, as agreement point.
We will finish suppression list first, and we will keep in mind that some examples just need to be moved to "Use cases", so should not be forced on same AST

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example this example will be in that group for sure.
So as we proceed on examples we will move what we noticed to to new group of suppression under comment " until ((new issue))"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#18435
Done

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Items to think , see above

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main blocker #18986 (comment)

Items to fix

@smita1078 smita1078 force-pushed the BoxComments branch 5 times, most recently from 505d952 to cce6879 Compare May 18, 2026 13:44
@romani

romani commented May 19, 2026

Copy link
Copy Markdown
Member

GitHub, generate website

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, ok to merge

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Items

<span class="wrapper inline">
<img src="images/ok_green.png" alt="" />
</span>
BoxComments

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put here search link as others have

@smita1078 smita1078 May 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried adding the search link but it causes XdocsPagesTest to fail with "too many config links" for the 4.8.6.1 row. The test only allows one config link per module entry


[ERROR] Failures: 
[ERROR]   XdocsPagesTest.testAllStyleRules:1962->validateStyleModules:2094->validateChapterWiseTesting:2198 google_style.xml rule '4.8.6.1 Block comment style' module 'BoxComments' has too many config links
[INFO] 
[ERROR] Tests run: 6000, Failures: 1, Errors: 0, Skipped: 2
[IN. 

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, please revert back and create new issue on this nuance, to let us improve separately.

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot

@romani romani merged commit 629c2e9 into checkstyle:master May 19, 2026
127 of 128 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants