Skip to content

Issue #18064: added ArrayBracketNoWhitespace check - #18240

Open
M-SaaD-H wants to merge 1 commit into
checkstyle:masterfrom
M-SaaD-H:18064
Open

Issue #18064: added ArrayBracketNoWhitespace check#18240
M-SaaD-H wants to merge 1 commit into
checkstyle:masterfrom
M-SaaD-H:18064

Conversation

@M-SaaD-H

@M-SaaD-H M-SaaD-H commented Dec 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

@romani

romani commented Dec 5, 2025

Copy link
Copy Markdown
Member

Please read and watch videos at Starting_Development.
Please make CI green.

@M-SaaD-H

M-SaaD-H commented Dec 5, 2025

Copy link
Copy Markdown
Contributor Author

Hi @romani ,
I have tested the check locally before raising the PR, but overlooked the XpathRegression tests. I am sorry for this, but I am facing some issues in passing these tests. I have asked this in the discord channel but didn't got the solution from there too. It will be really helpful if you could help me in understanding and solving this issue.
Tagged you in the related thread on discord.

@romani

romani commented Dec 5, 2025

Copy link
Copy Markdown
Member

Just debug test and trace all lines to understand why there is no violation. All is debug-able.

@SURYANSHUAGRAWAL2006

SURYANSHUAGRAWAL2006 commented Dec 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

@saad please make sure commit should be only one so, please squash all commits into single one.

@mohitsatr

mohitsatr commented Dec 7, 2025

Copy link
Copy Markdown
Member

@M-SaaD-H please push the changes so we can see what's wrong with XpathRegression tests. It's little hard to understand from images.

@M-SaaD-H

M-SaaD-H commented Dec 7, 2025

Copy link
Copy Markdown
Contributor Author

@mohitsatr I have pushed the code.
Please look into it.

mohitsatr
mohitsatr reviewed Dec 8, 2025
View reviewed changes
Comment thread ...ckstyle/checks/whitespace/arraybracketnowhitespace/InputArrayBracketNoWhitespaceDefault.java Outdated
Comment thread ...ckstyle/checks/whitespace/arraybracketnowhitespace/InputArrayBracketNoWhitespaceDefault.java Outdated
@M-SaaD-H

M-SaaD-H commented Dec 11, 2025

Copy link
Copy Markdown
Contributor Author

Hi @mohitsatr can you please look into the CIs, I can't really get why they are failing.

@SURYANSHUAGRAWAL2006

SURYANSHUAGRAWAL2006 commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Hello Saad my advice is that you have to be run the command mvn clean verify again and run this command again and again until BUILD SUCCESS will not encountered because same error also came in my checks so I simply did to ran command mvn clean verify and when BUILD SUCCESS showing then I push my code and all checks passed.
so, please go through this.

@M-SaaD-H

M-SaaD-H commented Dec 11, 2025

Copy link
Copy Markdown
Contributor Author

Hi @SURYANSHUAGRAWAL2006 ,
Thanks for your advice. I have already verified that build is success locally. I am getting BUILD SUCCESS message when I run mvn clean verify. This error is only showing when I push my changes. Moreover, I can't find the actual reason for the failure of these CI pipelines.

@vivek-0509

vivek-0509 commented Dec 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

For the run-inspections failure open the ci failes they have given this link
https://app.circleci.com/pipelines/github/checkstyle/checkstyle/38573/workflows/43e71112-f6c4-4ca9-8d52-4a0ed937de87/jobs/1167874/artifacts

you will se the inspections result open those links and fix those issues

Similarly for other CI failures check the logs

@SURYANSHUAGRAWAL2006

SURYANSHUAGRAWAL2006 commented Dec 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hello Saad I saw your build failure this is occurring not because of your code or check style it is because of maven problem so, Kindly rerun all CI best way close the pr then reopen the pr then all CI will run again and then you check all CI is passing or not but I am sure this is not your code problem rather than it is problem of maven internal.

@M-SaaD-H

M-SaaD-H commented Dec 12, 2025

Copy link
Copy Markdown
Contributor Author

Hi @romani
Can you please look into it?

@M-SaaD-H
M-SaaD-H force-pushed the 18064 branch 2 times, most recently from ce465be to b2c32cd Compare December 16, 2025 06:27
@M-SaaD-H
M-SaaD-H force-pushed the 18064 branch 2 times, most recently from bf67dc9 to 8bb25c4 Compare December 16, 2025 13:57
@mohitsatr

mohitsatr commented Dec 16, 2025

Copy link
Copy Markdown
Member

@M-SaaD-H Green CI is a good sign. Take to run regression.

Please go through the section Executing generation using github action and try to generate the report.

Example PR: #17329

@M-SaaD-H
M-SaaD-H force-pushed the 18064 branch 4 times, most recently from 47b8d0f to 4d1133d Compare December 16, 2025 17:23
@M-SaaD-H

M-SaaD-H commented Dec 16, 2025

Copy link
Copy Markdown
Contributor Author

GitHub, generate report

@github-actions

github-actions Bot commented Dec 16, 2025

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Dec 16, 2025

Copy link
Copy Markdown
Contributor

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

@mohitsatr

mohitsatr commented Dec 17, 2025

Copy link
Copy Markdown
Member

@M-SaaD-H Config in the Pr's description is not valid. It has to be like: https://gist.githubusercontent.com/mohitsatr/0e04070f847926ea44bf7eda88706807/raw/f9470db307d730a651e9c93cad776d6b592839e8/textblockgooglestyleformatting.xml. Replace Textb...Check with your check

@M-SaaD-H
M-SaaD-H force-pushed the 18064 branch 3 times, most recently from d4d3846 to 0dad8c0 Compare June 29, 2026 07:42
@M-SaaD-H

M-SaaD-H commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@romani
Please take a look when you get the chance.

@M-SaaD-H

M-SaaD-H commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

@romani ,
I've rebased the changes on the latest master. Please take a look.

@M-SaaD-H
M-SaaD-H force-pushed the 18064 branch 2 times, most recently from 200e597 to 007f23b Compare July 12, 2026 10:46
@romani

romani commented Jul 12, 2026

Copy link
Copy Markdown
Member

@mahfouz72 , please help to review.

@Atharv3221 Atharv3221 mentioned this pull request Jul 16, 2026
Draft
@vivek-0509 vivek-0509 mentioned this pull request Jul 17, 2026
Open
@romani

romani commented Jul 22, 2026

Copy link
Copy Markdown
Member

@M-SaaD-H , please rebase to resove conflict, while we are waiting for review.

@M-SaaD-H

M-SaaD-H commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Hi @mahfouz72 , I've rebased the PR on the latest master and resolved the conflicts. It's ready for another review.

@romani

romani commented Jul 24, 2026

Copy link
Copy Markdown
Member

GitHub, generate report

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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

@M-SaaD-H

M-SaaD-H commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Found one false positive here for String[]... param
Everything else looks good.

@M-SaaD-H

M-SaaD-H commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

GitHub, generate report

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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

@M-SaaD-H

M-SaaD-H commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@romani ,
I've fixed the previous false positive. Everything in the report looks good. Please take a look

@M-SaaD-H

M-SaaD-H commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@romani , gentle ping on this. Please let me know if there's anything else I need to address.

@mahfouz72

mahfouz72 commented Jul 29, 2026

Copy link
Copy Markdown
Member

Github, generate site

mahfouz72
mahfouz72 reviewed Jul 29, 2026
View reviewed changes

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

Looks good.
first pass:

Comment thread config/checkstyle-checks.xml

@mahfouz72 mahfouz72 Jul 29, 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.

Should we add this check in google-style config also and it tests? is google formatter fixes this?

@Atharv3221 Atharv3221 Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, but not in this PR

Comment thread config/checkstyle-resources-suppressions.xml Outdated
Comment on lines +120 to +123
<suppress id="noTrailingWhitespace"
files="/InputArrayBracketNoWhitespaceDefault\.java"/>
<suppress id="noTrailingWhitespace"
files="/InputJavadocParagraphCorrect\.java"/>

@mahfouz72 mahfouz72 Jul 29, 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.

Why do we have those, I don't see where are these input files

@M-SaaD-H M-SaaD-H Jul 30, 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.

done

Comment thread ...kstyle/checks/whitespace/arraybracketnowhitespace/InputArrayBracketNoWhitespaceCreation.java

@mahfouz72 mahfouz72 Jul 29, 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.

#18240 (comment)

Please add this case if not added already

@M-SaaD-H M-SaaD-H Jul 30, 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.

already added

@M-SaaD-H M-SaaD-H Jul 30, 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.

done

@mahfouz72

mahfouz72 commented Jul 29, 2026

Copy link
Copy Markdown
Member

GitHub, generate report

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/b446256_20260729195755/checks.html#Standard_Checks

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/b446256_20260729195755/checks/whitespace/arraybracketnowhitespace.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/b446256_20260729195755/checks/whitespace/index.html#Whitespace_Checks

@romani

romani commented Jul 30, 2026

Copy link
Copy Markdown
Member

uhh, this insane , I only now noticed that this is two different PRs for same issue.

here is second that also close to merge state: #19541

@M-SaaD-H

M-SaaD-H commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Failing CI is unrelated.

@romani

romani commented Jul 31, 2026

Copy link
Copy Markdown
Member

@mahfouz72 , please evaluate this PR #19541 also, we need to take best from it to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mahfouz72 mahfouz72 mahfouz72 left review comments
@romani romani romani requested changes
@mohitsatr mohitsatr Awaiting requested review from mohitsatr
@Zopsss Zopsss Awaiting requested review from Zopsss
+1 more reviewer
@Atharv3221 Atharv3221 Atharv3221 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@mahfouz72 mahfouz72

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

New check: ArrayBracketWhitespace

8 participants

@M-SaaD-H @romani @SURYANSHUAGRAWAL2006 @mohitsatr @vivek-0509 @Atharv3221 @Zopsss @mahfouz72