Issue #17841: Add GoogleMethodNameCheck to fix method name false nega…

[vivek-0509]

part of Issue: #17841
Issue: #18420

Description

Introduces a new GoogleMethodName check that properly validates method names according to the Google Java Style Guide, fixing false-negatives where underscores between letters and digits were not being flagged.

Problem

The previous MethodName check failed to detect invalid names like:

• gradle_9_5_1() - underscore between letter 'e' and digit '9'
• jdk_9_0_392() - underscore between letter 'k' and digit '9'

Solution

Created a dedicated GoogleMethodName check that:

• Has no configurable properties
• Uses multi-pattern validation instead of one complex regex
• Handles @Test methods separately (underscores allowed)
• Properly validates numbering suffixes (guava33_4_5 is valid)

Validation Rules

Regular Methods:

• Must be in lowerCamelCase
• Underscores only between adjacent digits (_4_5)
• Not between letter and digit (gradle_9 invalid)

Test Methods (@Test, @ParameterizedTest, @RepeatedTest):

• Underscores allowed to separate components
• Each segment must be lowerCamelCase
• Each segment must be at least 2 characters
• Segments cannot start with single lowercase followed by uppercase (e.g., fO)

Override Methods:

• Methods with @Override annotation are skipped (no validation)

Breaking Changes

None. This check is backward compatible with the previous MethodName regex behavior.

Diff Regression config: https://gist.githubusercontent.com/vivek-0509/3c0bc74332fbdb3a6ce4aef1c43d2f6b/raw/a7d36ba756226bac0e01e9148dc8bd7f3016ec96/base_config.xml

Diff Regression patch config: https://gist.githubusercontent.com/vivek-0509/a2777cd3e2a86dc84c549268d6651aa0/raw/6490530b3efb4213b7a2421dace6f157c7b264a2/patch_config.xml

Contirbution repo PR: checkstyle/contribution#1000

[vivek-0509]

The CI Failures are unrelated

[vivek-0509]

Github, generate report for GoogleMethodName/all-examples-in-one

[github-actions]

Failed to download or process the specified configuration(s).
Error details:

Please ensure you've used one of the following commands correctly:

• 'GitHub, generate report for <config_bundle_path>'
• 'GitHub, generate report by config from <file_path>'
• 'GitHub, generate report for configs in PR description'
  
  And check that all specified configs or files exist and are valid.
  
  Link: https://github.com/checkstyle/checkstyle/actions/runs/20267426773

[vivek-0509]

GitHub, generate report

[vivek-0509]

GitHub, generate report

[vivek-0509]

GitHub, generate report

[github-actions]

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

[vivek-0509]

GitHub, generate report

[github-actions]

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

[vivek-0509]

Hey @romani,
Here is the analysis of the report

Bug Found (Will Fix)

Issue: Test methods with numbering suffix like test3_1 are incorrectly rejected.

Cause: The implementation doesn't strip numbering suffix before validating test methods (unlike regular methods).

Examples:

• test3_1 → Rejected (Should be valid - 3_1 = adjacent numbers)
• testFoo_1_2 → Rejected (correctly invalid)

Fix: Will add numbering suffix stripping + INVALID_UNDERSCORE_PATTERN check for test methods.

---

Design Questions - Need Your Input

1. Minimum 2-character segment requirement for test methods

Currently rejects single lowercase character segments:

Examples:

• t_name → Rejected (segment t = 1 char)
• test_a → Rejected (segment a = 1 char)
• test_foo → Valid (both segments ≥ 2 chars)

Real example:

@Test
void can_be_used_in_a_collector() {} //  Rejected (segment a fails)

Question: Should we keep the current 2-character minimum requirement, or should we make it stricter by requiring proper lowerCamelCase (at least 2 words joined with capitalization) for each segment in test method names?

2. Test methods without @test annotation

Examples:

• void test_ctor_shouldThrow_whenPidIsMissing() → REJECTED (no annotation)
• @Test void test_ctor_shouldThrow_whenPidIsMissing() → VALID

Supported annotations: @Test, @ParameterizedTest, @RepeatedTest

Question: Should test-style method names without annotations be rejected, as Google Style only mentions JUnit tests?

[vivek-0509]

@romani ping

[romani]

@Zopsss , can you help me to review this PR?
while I am busy with general stream of PRs.

[vivek-0509]

Will fix the CI asap , failures caused due to recent pr merge method access level for getPackageLocation() changed to pubilc

[vivek-0509]

Hi @Zopsss ,

Before you review the code, I'd like to clarify my understanding of the expected rules for GoogleMethodNameCheck , as Google Style Guide is somewhat ambiguous in places.

For regular methods (without @Test), my implementation requires lowerCamelCase names where underscores are only allowed between adjacent numbers (like guava33_4_5). Names like gradle_9_5_1 with letter_digit underscores are flagged as invalid. Single letter names like f and patterns like fO are also rejected.

For test methods (with @Test, @ParameterizedTest, @RepeatedTest), underscores are allowed to separate logical components, with each component being lowerCamelCase. So transferMoney_deductsFromSource is valid, but test_1 is invalid because 1 is not lowerCamelCase and also it violates the no underscore between just number and digit. The numbering suffix rule also applies, so test3_1 is valid (adjacent numbers), but testFoo_1_2 is invalid (letter_digit pattern). For test methods like solve6x6_returnsTrue and solve6x6_noSolution_returnsFalse, my implementation treats them as valid since each component (solve6x6, returnsTrue, noSolution, returnsFalse) is lowerCamelCase and the underscore separates these components rather than being between a digit and letter within a component.

One thing I'm unclear about: Google Style says test components should be "lowerCamelCase". Strictly speaking, lowerCamelCase means multiple words joined with capitalization (like transferMoney). Should single-word components like test_foo be valid, or should we require multi-word lowerCamelCase like transferMoney_deductsFromSource?

If any of my expectations don't align with what's actually required, please let me know and I'll adjust my implementation before you do a full code review. This should save time .

[Zopsss]

this message does not explain why the method name is invalid. Instead we should keep different messages for the different conditions we hit to easily explain reason behind violation to the user.

[Zopsss]

please create separate messages for all the logs which should explain the reason behind the violation.

[vivek-0509]

For the separate violation messages, I'm planning to add 4 messages (one per log point):

google.method.name.underscore.regular=Method name ''{0}'' has invalid underscore usage, underscores only allowed between adjacent digits.

google.method.name.underscore.test=Method name ''{0}'' has invalid underscore usage, underscore only allowed between letters or between digits.

google.method.name.format.regular=Method name ''{0}'' must be lowerCamelCase: start with lowercase, at least 2 characters, and avoid single lowercase followed by uppercase.

google.method.name.format.test=Method name ''{0}'' is not valid: each segment must be lowerCamelCase: start lowercase, min 2 chars, avoid single lowercase followed by uppercase.

Does this approach work for you?

[Zopsss]

google.method.name.underscore.test=Method name ''{0}'' has invalid underscore usage, underscore only allowed between letters or between digits.

this one is for the test methods, so it should mention that it is for test methods.

google.method.name.format.regular=Method name ''{0}'' must be lowerCamelCase: start with lowercase, at least 2 characters, and avoid single lowercase followed by uppercase.

google.method.name.format.test=Method name ''{0}'' is not valid: each segment must be lowerCamelCase: start lowercase, min 2 chars, avoid single lowercase followed by uppercase.

it should end with: avoid single lowercase followed by uppercase or underscore.

[vivek-0509]

it should end with: avoid single lowercase followed by uppercase or underscore.

but this still falls under invalid underscore usage, for example, f_ is not permitted because valid underscore usage is restricted to letter–letter or digit–digit only.
and for this case test_a message already mentions the min 2-chars required for each segment.

[vivek-0509]

Hi @Zopsss, thanks for the review
I’ve created an issue to clarify the expected scope and rules for this check: #18420

Would it be acceptable if I apply all the requested changes once we align on what exactly this check should cover and the set of rules to be enforced for this check?

[romani]

GitHub, generate report

[github-actions]

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

[romani]

GitHub, generate report

[github-actions]

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

[github-actions]

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

[lrozenblyum]

@lrozenblyum , I know you use Google style. Can you do us favor to provide feedback on this Check implementation? We can release it, without activation in Google style config and we can probably us you to test it of your codebase. If you confirm that no bad side effects appears, we can update config and be more confident in success.

Hello @romani. Yes I think I can support this. Does that mean I need to use some nightly build of Checkstyle or will there be a stable release with this rule?

[vivek-0509]

@romani Please guide how should we proceed.

[vivek-0509]

GitHub, generate report

[vivek-0509]

GitHub, generate website

[vivek-0509]

@romani CI failures are unrelated

[github-actions]

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

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/227d5fd_20260225203343/checks/naming/googlemethodname.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/227d5fd_20260225203343/checks/naming/index.html#Naming_Conventions_Checks

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/227d5fd_20260225203343/checks/naming/methodname.html#Example_of_Usage

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/227d5fd_20260225203343/google_style.html#Coverage_table

[github-actions]

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

[vivek-0509]

@romani Updated the example for this also to keep only the relevant part of the violation message

[vivek-0509]

@romani Updated the XpathRegressionTest for this also to have atleast 3 tests with different structure..

[vivek-0509]

Rebased and resolved all the merge conflicts, no other changes
CI Failures are unrelated

[vivek-0509]

Updated the version to 13.4.0, i was getting the spell check ci failure with the diff to add args , but my PR does not add args anywhere, i have added to satisfy the CI

[romani]

please read https://github.com/checkstyle/checkstyle/pull/18312/changes#r2868309055

[vivek-0509]

@romani ping https://github.com/checkstyle/checkstyle/pull/18312/changes#r2868462900

[vivek-0509]

@Zopsss @romani the PR is related to Google Style project..
Closing this as discussed with Mahfouz
@Atharv3221 You can reuse the work since the PR is in very good state just few tweaks and ready to merge