Pmd: [java] ClassNamingConventions is too ambitious on finding utility classes

[asarkar]

Affects PMD Version:
6.3.0

Rule:
ClassNamingConventions

Description:
ClassNamingConventions is too draconian on finding utility classes. For example, it would fail TestHelper as "doesn't match '[A-Z][a-zA-Z]+Util". I don't want to disable the rule because it also checks other classes and interfaces that I find useful. However, no one said every class with a private constructor must end with Util or the world would burn down.

Code Sample demonstrating the issue:
N/A

Running PMD through: Gradle

[jsotuyod]

@asarkar thanks for the report. This however seems to not be an issue with PMD itself...

The rule defines properties to allow users to enforce whatever conventions they see fit. You can simply change the value to whatever fits your project and organization very easily:

<rule ref="category/java/codestyle.xml/ClassNamingConventions">
  <properties>
    <property name="utilityClassPattern" value="[A-Z][a-zA-Z]+(Utils?|Helper)"/> <!-- allow *Utils. *Util and *Helper -->
  </properties>
</rule>

We can discuss default values for theses properties, but I would never call draconian a rule that is designed for customization.

[asarkar]

@jsotuyod Thanks for your response. I don't think there is an established pattern for what PMD considers utility class. Following are some examples of perfectly acceptable class names that the rule in question would fail:

1. ApplicationConstants
2. BlahHelper
3. ThrowableAnalyzer
4. MyApp (class with main method)
5. BlahBuilder
6. This one is classic - a single s fails the check TestUtils

I understand that I can customize the property, but I shouldn't have to; none of the classnames shown above are out of the ordinary.

I'm of the opinion the utility classname check creates more problems than it solves and should be removed.

[jsotuyod]

I see your point, but I'm not sure to share your opinion.

To remove this just because you personally don't need it seems rather harsh... why should we refrain from letting users enforce a convention if they want to?

You can effectively "disable" this check by setting it to:

<rule ref="category/java/codestyle.xml/ClassNamingConventions">
  <properties>
    <property name="utilityClassPattern" value="[A-Z][a-zA-Z0-9]+"/>
  </properties>
</rule>

I understand you may have an issue with the default values, and we may be open to discuss them (see #1070) but removing functionality is a different matter altogether

[adangel]

See also #1102

[DanySK]

Is there any plan to discuss the defaults for the rule? I believe it would be a very useful one, but its default configuration is not convenient right now.

I agree with @asarkar when he says that the rule default behaviour is inconvenient. Configurability is good and I believe the rule should stay, but its default configuration should account at least for several endings that are currently ignored:

• Factory (by the way I disagree on Builder @asarkar, as the GoF pattern does not imply using an utility class, if you have it, it's a collection of factory methods and I would say you should name it accordingly)
• App(lication)?
• Ending s (as per Collections, Arrays...)

I would argue that the default configuration should not include names ending in er: they seem to me to imply the construction of an instance that can do something (Builder, Helper, Analyzer...), for which a different phrasing could be more appropriate (Factory, Utilities, Analysis...).

[jsotuyod]

@DanySK we are more than open to do so, as we already did in plenty other cases (some even cited on this thread).

Please, open a new issue with the changes you would like to see in place with proper support for the case (showing this is indeed a common enough naming convention to make it a default).

For one, I totally support allowing for FooFactory.
Classes ending in s may be a little too broad… your examples are just fine, but that alone doesn't imply a pluralization. Otherwhise BazAnalysis would be automatically ok. Maybe a plural detection lib would be needed, but that's somewhat harder, and would probably assume all code is written in English...