MethodNameCasing poorly implemented

[delanym]

Essentially the implementation of the MethodNameCasing recipe is inadequate. All the documentation https://docs.openrewrite.org/recipes/staticanalysis/methodnamecasing can tell me is Method names should comply with a naming convention. Well what is that convention and how do I change it? There are no options for that.

A tag is listed: https://sonarsource.github.io/rspec/#/rspec/S100 Which includes the text For example, with the default provided regular expression ^([A-Z0-9_]*|[a-z0-9_]*)$, the method...

If this was in fact the default regular expression then why did it convert a method name get_CHAR_NullableOutputList to getCHARNullableOutputList?

A further issue is that it did not convert the method name get_LongInputList since there was already a getLongInputList method, and yet it did not cancel the recipe with an error/warning.

I'm logging this as a bug not a feature because it applied the recipe and broke the build.

[timtebeek]

Thanks for the feedback! I can understand the confusion; up to recently that recipe linked to an older source at Sonar that's not been maintained; they'd pointed that out and we've since updated the links.

The old link was a little more descriptive in what is and is not compliant, in particular using a Java example: https://rules.sonarsource.com/java/RSPEC-100/

Shared naming conventions allow teams to collaborate efficiently.

This rule raises an issue when a method name does not match a provided regular expression.

For example, with the default provided regular expression ^[a-z][a-zA-Z0-9]*$, the method:

public int DoSomething(){...} // Noncompliant

should be renamed to

public int doSomething(){...}

Exceptions Overriding methods are excluded.

@Override
public int Do_Something(){...} // Compliant by exception

I guess it would help to update our recipe description now that the external source isn't helping understand the changes made. Perhaps working back from the tests we have to document the changes. https://github.com/openrewrite/rewrite-static-analysis/blob/1ed9c44b1413a388a7d436e8a7ca5bcacc40fc94/src/test/java/org/openrewrite/staticanalysis/MethodNameCasingTest.java#L85-L108

@mike-solomon is this one you could take a look at to update the displayed name and description? https://github.com/openrewrite/rewrite-static-analysis/blob/1ed9c44b1413a388a7d436e8a7ca5bcacc40fc94/src/main/java/org/openrewrite/staticanalysis/MethodNameCasing.java#L58-L64

Beyond that we can then see how what the recipe does might conflict with what you were expecting @delanym ; how did things break specifically? A before/after example would be best, just to be sure we fix the bug as well that prompted you to log it here.

[timtebeek]

Mike was kind enough to update the display name and description already in

• 292

Would you feel we need to make further adjustments still? Perhaps an example with a before/after as seen in our tests would be best for anything not correctly handled right now. Making the examples runnable in that sense also helps rule out any outside influences, like Lombok leading to missing type information.