New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] ClassNamingConventions is too ambitious on finding utility classes #1096

Closed
asarkar opened this Issue May 11, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@asarkar

asarkar commented May 11, 2018

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

@asarkar asarkar changed the title from ClassNamingConventions is too ambitious on finding utility classes to [java] ClassNamingConventions is too ambitious on finding utility classes May 11, 2018

@jsotuyod

This comment has been minimized.

Member

jsotuyod commented May 12, 2018

@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

This comment has been minimized.

asarkar commented May 12, 2018

@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

This comment has been minimized.

Member

jsotuyod commented May 12, 2018

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

This comment has been minimized.

Member

adangel commented May 16, 2018

See also #1102

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment