Skip to content

Conversation

Anmol202005
Copy link
Contributor

@Anmol202005 Anmol202005 commented Mar 6, 2025

@Anmol202005
Copy link
Contributor Author

Anmol202005 commented Mar 6, 2025

@romani
I'm stuck with an xpathRegression failure. Could you take a look and let me know if I'm missing something?

unexpected output: C:\Users\anmol\IdeaProjects\checkstyle3\src\it\resources\
org\checkstyle\suppressionxpathfilter\unnecessarynullwithinstanceof\
InputXpathUnnecessaryNullCheckWithInstanceOf.java:5:13: Unnecessary nullity check with instanceof operator.
expected: 0
but was : 1
Expected :0
Actual   :1
<Click to see difference>

The Xpath test is failing, do i need to configure something related to the new check .

It seems like the XPath query isn't suppressing the violation, causing it to be flagged at:

verify(treeWalkerConfigWithXpath, fileToProcess.getPath(), CommonUtil.EMPTY_STRING_ARRAY);

please have a look.

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:

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.

please keep resolving all CI redness.

items:

@Anmol202005
Copy link
Contributor Author

@romani all CI is green, please review
please help me with how to mention violations in message.properties for languages that are not supported !
like for ja, zh

@nrmancuso
Copy link
Member

@romani all CI is green, please review please help me with how to mention violations in message.properties for languages that are not supported ! like for ja, zh

Just use google, we do not have any maintainers that can read or write these

@Anmol202005
Copy link
Contributor Author

Anmol202005 commented Mar 16, 2025

Just use google, we do not have any maintainers that can read or write these

thanks @nrmancuso
the issue was actually that IDEA was supporting these languages but no issue, i configured the file encoding to UTF-8.

@Anmol202005 Anmol202005 force-pushed the InstanceOfCheck branch 2 times, most recently from e8b4fb3 to f324a11 Compare March 16, 2025 12:34
@romani
Copy link
Member

romani commented Mar 20, 2025

@mahfouz72 , as this is your issue please be welcome to review first.

@romani
Copy link
Member

romani commented Mar 20, 2025

@Anmol202005 , please read on how to do regression testing for new modules
https://github.com/checkstyle/checkstyle/tree/master/.github/workflows#diff-report-by-configuration-in-pull-request-description

we need report of how it works in wild reallife code.

@mahfouz72
Copy link
Member

github, generate site

Copy link
Member

@mahfouz72 mahfouz72 left a comment

Choose a reason for hiding this comment

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

Looks good.
Few items:

@Anmol202005 Anmol202005 force-pushed the InstanceOfCheck branch 3 times, most recently from 806dd59 to e99d147 Compare March 21, 2025 16:13
@Anmol202005
Copy link
Contributor Author

Github, generate report

Copy link
Contributor

@romani
Copy link
Member

romani commented May 18, 2025

GitHub, generate website

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

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

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

@Anmol202005 Anmol202005 force-pushed the InstanceOfCheck branch 3 times, most recently from f2be459 to 1abfd1e Compare May 20, 2025 08:21
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 May 20, 2025

based on #16489 (comment)

please send PRs to following projects (you can send updates to any other projects too):
Orekit
camunda

to prove that people are welcoming such updates, and validations is correct. and eventual auto-fix will be well welcomed also.

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.

last:

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.

last:

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.

very last:

@romani romani removed the request for review from nrmancuso May 25, 2025 14:15
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.

great job!

we will merge after CI pass.

@romani romani merged commit 17169c1 into checkstyle:master May 25, 2025
116 of 117 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.

Add Check Support for Java 21 Record Pattern : New Check UnnecessaryNullCheckWithInstanceof
5 participants