Skip to content

Issue #18435: Fix xdocs Examples AST Consistency Test - parenpad#19983

Merged
romani merged 1 commit into
checkstyle:masterfrom
smita1078:AST13_GSOC
Jun 2, 2026
Merged

Issue #18435: Fix xdocs Examples AST Consistency Test - parenpad#19983
romani merged 1 commit into
checkstyle:masterfrom
smita1078:AST13_GSOC

Conversation

@smita1078

Copy link
Copy Markdown
Contributor

#18435

parenpad

Example1.java -> default config
Example2.java -> token property (unique)

Both in section1.

@smita1078

Copy link
Copy Markdown
Contributor Author

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

Items

catch(Exception e ) {} // violation 'not followed by whitespace'
for ( int i = 0; i < x; i++ ) {
// ok
// ok

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.

@smita1078 , please check why there is no violations on file with simple // ok comment?

I thought we forbid it in all inputs. It should be always with details on why is ok and removed.

In your case it should be removed and empty line states.

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.

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.

Sure

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 checked the configuration and noticed that the Regexp module is currently placed under TreeWalker.
https://github.com/checkstyle/checkstyle/blob/d4c29c07597995ca1cb4d87227a62984cbd1f338/config/checkstyle-input-checks.xml

<module name="TreeWalker">
    ...
    <module name="Regexp">

Since Regexp is a Checker-level check and not a TreeWalker check, it looks like the configuration is not being applied for these comments currently. That is likely why existing plain // ok comments (for example in ParenPad inputs) are not reporting violations.

For this case, I will remove the unnecessary comments and keep empty line states instead.

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.

Also, should I create a separate issue/PR to move this Regexp check to the Checker level so these comments are properly validated repository wide?

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.

Separate Issue and PR.
We need to prevent this bad pattern in examples. Thanks a lot for investigation

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.

@smita1078 smita1078 force-pushed the AST13_GSOC branch 2 times, most recently from fefebf3 to 8250dfd Compare May 28, 2026 17:12
@smita1078

Copy link
Copy Markdown
Contributor Author

@smita1078

Copy link
Copy Markdown
Contributor Author

Github, generate site

@romani

romani commented May 29, 2026

Copy link
Copy Markdown
Member

From html page:

The following cases are not checked

This looks like use case without config. If you agree, please create issue will update it as we deal with Use cases.


All good, as two issues created we can merge.

@smita1078

smita1078 commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

From html page:

The following cases are not checked

This looks like use case without config. If you agree, please create issue will update it as we deal with Use cases.

Will we create examples for such case under use case sections?? or just move this under use case sections?

#19994
Done

@smita1078

smita1078 commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

All good, as two issues created we can merge.

Issues and PR #19998 is created, I guess we can merge this!

@stoyanK7 stoyanK7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks cleaner now 👍

@romani

romani commented Jun 2, 2026

Copy link
Copy Markdown
Member

GitHub, generate website

@romani romani merged commit fd5afe3 into checkstyle:master Jun 2, 2026
126 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