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

[vivek-0509]

@Zopsss ping

[Zopsss]

GitHub, generate report

[Zopsss]

GitHub, generate site

[Zopsss]

@vivek-0509 sorry for the delay, please resolve conflicts

[vivek-0509]

@Zopsss Done, Could you please take a look at this comment and review the PR when you get a chance, Thanks

[vivek-0509]

GitHub, generate site

[vivek-0509]

GitHub, generate report

[vivek-0509]

I have generated the Diff report in comparison to the Method Name Check which was used earlier in google checks to clearly see new and removed violation .

[github-actions]

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

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

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

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

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

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

[github-actions]

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

[vivek-0509]

@Zopsss ping

[Zopsss]

Sorry for the delay, the diff report and documentation looks good! Thanks a lot for all the hard work!!!!

[vivek-0509]

@romani Updated the since to 13.2.0, and rebased the branch, PR is ready to merge, waiting for your approval.

[vivek-0509]

@romani Rebased the branch because recently a new check is merged, so need to update the XmlMetaReaderTest

[romani]

With such massive diff report, this update will be a bomb.
And we can can not reliably verify that there is no regression, false positives.

If will be awesome to find some project that uses Google style, and send them updates to test their reaction, before we pour this on heads of whole world.

Is there any ideas?

Maybe comunda?

This is also interesting case https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/b025006_2026062507/reports/diff/protonpack/index.html#A1
aggregate_on_bi_element_predicate Google style allow phrases to be combined, not a simple words. No capital letters hints us that there is single word.

Are we going to use this wording
https://google.github.io/styleguide/javaguide.html#s5.2.2-class-names

Underscores may appear in JUnit test method names to separate logical components of the name, with each component written in lowerCamelCase, for example transferMoney_deductsFromSource. There is no One Correct Way to name test methods.

[romani]

After thinking more on aggregate_on_bi_element_predicate , we should not allow this in first version of this Check. It is clear snake case naming.
Please add it to examples, even it test, no excuses.
We can change our mind based on user feedback.

[vivek-0509]

Are we going to use this wording
https://google.github.io/styleguide/javaguide.html#s5.2.2-class-names

@romani yes i was also worried about this type of cases while implementing this check due to the wording
I will update the test method regex to enforce that each segment must contain at least one uppercase letter (lowerCamelCase), which rejects snake_case patterns

Valid: testFoo, testCheck_loginFails, login (single segment without underscore)
Invalid: test_login, aggregate_on_bi_element_predicate

Regarding testing with Camunda, I checked their configuration at camunda/.checkstyle.xml and they use the default MethodNameCheck with the default regex, not google_checks.xml. Since they're not following Google style conventions for method naming, i think they would not be a good candidate to test this change

i found this project google-cloudevents-java that uses checkstyle google_checks.xml but it seems to be inactive
https://github.com/googleapis/google-cloudevents-java/blob/f23e048750ad5666a71b786da8f01952c9037de1/pom.xml#L92-L105

Do you know about any other project that enforces google_checks.xml ???

[Gaoyan1999]

Reply for comment #18804 (comment).
Using NullUtil.notNull(ast.findFirstToken(TokenTypes.IDENT)) here looks totally correct to me. A small nit: since this method only accepts METHOD_DEF, naming the parameter methodAst would be a bit more explicit. That said, ast is also fine—this is clear from getRequiredTokens, and other checkers do the same. Renaming or not renaming are both OK.

[romani]

Do you know about any other project that enforces google_checks.xml ?

No, users try to customize a bit config, and I understand it, demand for javadoc on all is too expensive.
Majority immediately remove all about javadoc. Javadoc is applicable only for libraries, so it is very low percentage from general java code base

[vivek-0509]

@romani
i think we need to update the violation message also to not confuse users

Test method name ''{0}'' is not valid. Each segment must start lowercase, contain at least one uppercase letter for multi-segment names, min 2 chars, avoid single lowercase letter followed by uppercase, and contain only letters, digits, or underscores.

does this work ??

[romani]

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

[vivek-0509]

GitHub, generate report

[vivek-0509]

GitHub, generate website