Issue #14765: fix OverloadMethodsDeclarationOrderCheck

[Zopsss]

Issue #14765

• Fixes OverloadMethodsDelcarationCheck. Now the module gives error when there is variable, interface, class, etc between overloaded methods.
• Added new test cases
• Updated Documentation

---

Diff Regression config: https://gist.githubusercontent.com/Zopsss/80b98438b46ad1111cf47c520e8ea669/raw/d98d1c801e2c5abd3e6e8834b749dbc95769db53/OverloadMethodsDeclarationOrderCheck.xml

Diff Regression projects: https://gist.githubusercontent.com/Zopsss/22adadb570e4deb95296917244c580b3/raw/29ea756eada8915637b76678db4f46048c198808/projects-to-test-on-for-github-action.properties

[Zopsss]

GitHub, generate site

[github-actions]

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/ccc4bf1_2024055104/checks/coding/overloadmethodsdeclarationorder.html#Example2-code

[Zopsss]

GitHub, generate report

[github-actions]

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

[Zopsss]

GitHub, generate report

[github-actions]

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

[Zopsss]

GitHub, generate report

[github-actions]

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

[Zopsss]

The website & report looks good to me.

@romani can you please review the PR?

[romani]

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/ccc4bf1_2024074227/reports/diff/apache-ant/index.html#A1

All overloaded methods should be placed next to each other. Placing non-overloaded methods in between overloaded methods with the same type is a violation. Previous overloaded method located at line '1,234'.

Question to maintainers: Should we change message ?

---

@Zopsss , this looks like false positive - https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/ccc4bf1_2024074227/reports/diff/apache-struts/index.html#A5 , if that is true, please fix and regerate report and check report yourself first.

item:

[romani]

GitHub, generate website

[github-actions]

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/44b86fb_2024135129/checks/coding/overloadmethodsdeclarationorder.html#Example1-code

[romani]

Ok to merge.

[romani]

GitHub, generate report

[github-actions]

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

[rnveach]

@romani @nrmancuso @Zopsss
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/1f7b5dc_2024204006/reports/diff/apache-struts/index.html#A1

Placing non-overloaded methods in between overloaded methods with the same type is a violation.

Shouldn't this verbiage be updated to something like Placing anything in between overloaded methods?

[Zopsss]

Placing anything in between overloaded methods

I guess following message will be a better option as we ignore the comments in this check:

Placing anything in between overloaded methods, expect comments

[Zopsss]

Why linkcheck-plugin is failing? Even though I changed only the input file?

https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25181/workflows/01a7d70f-631b-4419-9430-902834a1d7e5/jobs/574997/steps

@rnveach

[romani]

Failed CI is restarted

[Zopsss]

Failed CI is restarted

Thanks now all CIs are passing.

I've made the changes according to your suggestions, is there anything else left to do? @rnveach

[Zopsss]

Does this check have logic based on return type?

No the check does not care about the return type, it only cares about the method name.

Perhaps a better message would be Overloaded method declarations should be grouped together.

Can it be something like this?

Overloaded method declarations should be grouped together. Placing anything in between them is a violation, expect comment. Previous overloaded method is located at line '0'.

This message is shorter than the current error message and also simple & easy to understand.

[rnveach]

@Zopsss We have come to a decision on new message.

Original message is All overloaded methods should be placed next to each other. Placing non-overloaded methods in between overloaded methods with the same type is a violation. Previous overloaded method located at line X.

New message is we trim message to just All overloaded methods should be placed next to each other. Previous overloaded method located at line X. So we basically remove the existing 2nd sentence.

Context of the 2nd sentence coming into existence:

bb2d8c1
#4921

It doesn't look like we will revert that issue with the proposed change. Original confusion was on the word "split" and we still don't use that in new change.

[rnveach]

Last change.

[rnveach]

I am good with code and regression besides my last item.

[Zopsss]

My IntelliJ automatically added unicodes to the translated messages.. fixing this asap. Idk why this happened. So sorry for this

[Zopsss]

Updated the xdoc examples and resolved the failing CIs.

PR is now ready to be merged. Sorry it took a little longer, my IntelliJ was not working properly idk why that happened.

[Zopsss]

@rnveach

[MANISH-K-07]

My IntelliJ automatically added unicodes to the translated messages.. fixing this asap. Idk why this happened.

PR is now ready to be merged. Sorry it took a little longer, my IntelliJ was not working properly idk why that happened.

@Zopsss , not just IntelliJ, IDEs in general are not automatically configured to detect these.
It is better to use primitive editors for properties files to avoid such unwanted changes..

[Zopsss]

@Zopsss , not just IntelliJ, IDEs in general are not automatically configured to detect these. It is better to use primitive editors for properties files to avoid such unwanted changes..

Okkay got it! Thanks for the help.

[Zopsss]

@nrmancuso can you merge the PR please? Other maintainers have approved the changes.

[nrmancuso]

Good stuff @Zopsss !