Skip to content

Issue #18064: added ArrayBracketNoWhitespace check#18240

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

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

Conversation

@M-SaaD-H
Copy link
Contributor

@M-SaaD-H M-SaaD-H commented Dec 3, 2025

@romani
Copy link
Member

romani commented Dec 5, 2025

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

@M-SaaD-H
Copy link
Contributor Author

M-SaaD-H commented Dec 5, 2025

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
Copy link
Member

romani commented Dec 5, 2025

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

@SURYANSHUAGRAWAL2006
Copy link

SURYANSHUAGRAWAL2006 commented Dec 5, 2025

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

@mohitsatr
Copy link
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
Copy link
Contributor Author

M-SaaD-H commented Dec 7, 2025

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

@M-SaaD-H
Copy link
Contributor Author

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

@SURYANSHUAGRAWAL2006
Copy link

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
Copy link
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
Copy link
Contributor

vivek-0509 commented Dec 11, 2025

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
Copy link

SURYANSHUAGRAWAL2006 commented Dec 11, 2025

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
Copy link
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
Copy link
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
Copy link
Contributor Author

GitHub, generate report

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@mohitsatr
Copy link
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 5 times, most recently from e252fbf to 4d67faa Compare January 12, 2026 19:16
@M-SaaD-H
Copy link
Contributor Author

Hi @romani ,
I’ve addressed all the review comments from earlier.
Please let me know if anything else is needed from my side.
Thanks!

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

items:

@romani
Copy link
Member

romani commented Jan 30, 2026

please rebase on most latest code.

@romani
Copy link
Member

romani commented Jan 30, 2026

GitHub, generate website

@romani
Copy link
Member

romani commented Jan 30, 2026

@M-SaaD-H , can you explain this violation
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/4d1133d_2025073844/reports/diff/guava/index.html#A1 I did not get what is expected code ?

can you google formatter onthis snippet by CLI and share output.

@M-SaaD-H
Copy link
Contributor Author

M-SaaD-H commented Jan 30, 2026

@romani , the violation that you pointed out is
byte[] expected = {bytes[4], bytes[5]};
and the expected (according to the current implementation) is
byte[] expected = { bytes[4], bytes[5] };

However, ArrayBracketNoWhitespaceCheck should not require whitespace between ] and } here, so the existing format looks correct to me. If you want this enforced, I can update the test, please confirm.

@M-SaaD-H
Copy link
Contributor Author

@romani , I ran google-java-format on the file via CLI. The formatter does not change this line, so it doesn’t consider it a formatting issue.

@M-SaaD-H M-SaaD-H force-pushed the 18064 branch 2 times, most recently from a20f5bf to f17ae09 Compare January 30, 2026 12:18
Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

items:

@romani
Copy link
Member

romani commented Jan 30, 2026

However, ArrayBracketNoWhitespaceCheck should not require whitespace between ] and } here, so the existing format looks correct to me. If you want this enforced, I can update the test, please confirm.

this is false positive, please fix.

@romani
Copy link
Member

romani commented Jan 30, 2026

GitHub, generate website

@romani
Copy link
Member

romani commented Jan 30, 2026

GitHub, generate website

@M-SaaD-H
Copy link
Contributor Author

GitHub, generate website

@M-SaaD-H M-SaaD-H requested a review from romani January 31, 2026 05:15
@vivek-0509
Copy link
Contributor

@M-SaaD-H
There are a lot of surviving mutants please eliminate them
https://github.com/checkstyle/checkstyle/actions/runs/21519356239/job/62005978578?pr=18240

@M-SaaD-H
Copy link
Contributor Author

M-SaaD-H commented Feb 1, 2026

Hi @vivek-0509
I have killed most of the muatations, but there still exists one.
And for it to be killed the VARIABLE_DEF token need to be the closest valid token after ] on the same line (something like []int) which is not a valid java syntax. Is there a way to suppress this mutation? or how else should I proceed?

@vivek-0509
Copy link
Contributor

vivek-0509 commented Feb 1, 2026

@M-SaaD-H This means the VARIABLE_DEF token does nothing and is effectively dead code since a VARIABLE_DEF token can never appear closest after a ]. We can safely remove it and verify that everything works correctly without it.

@M-SaaD-H
Copy link
Contributor Author

M-SaaD-H commented Feb 2, 2026

@vivek-0509
I tried that, but that causes much more mutations. I didn't had much time yesterday due to some problems but I'll be working on it.

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.

New check: ArrayBracketWhitespace

5 participants