Class name length rule

[frankdekker]

Type: feature
Issue: Resolves #763
Breaking change: no

Added LongClassName Rule
Detects when classes or interfaces are declared with excessively long names.

Properties:

• maximum: default 20
• subtract-suffixes: default '', comma separated suffixes removed from the length calculation

Added ShortClassName Rule
Detects when classes or interfaces have a very short name.

• minimum: default 3
• exceptions: default '', comma separated list of class names that are excluded

Added /PHPMD/Support/Strings class
::length, calculates the length of a string with optionally subtracted suffixes
::split, splits a string on a specific separator, trimming parts, and removing empty strings

Updated LongVariable Rule
removed function getStringLength and replaced usage with Strings::length
cleaned up getSubtractSuffixList

• used the getStringProperty with default param
• used the Strings::split method to split the suffixes.

Added test cases for all changes
Updated documentation for Long- and ShortClassName

Note:
"since" needs to be updated in naming.xml

Unsure about the priority for the two rules. I've set them to 3.
Unsure about the namespace for the Strings class. I took Laravels "Support/Str" namespace/class as example. I'm open for suggestions.

[frankdekker]

Any tips how to solve:
https://travis-ci.org/github/phpmd/phpmd/jobs/683292013

It seems that build.xml runs phpmd.phar on the PHPMD/Rule directory. This now triggers the LongClassName rule as there are some classes that exceed the 20 characters mark.

Should I add a phpmd.xml that will be used by build.xml with a higher allowed class name length or choose a different directory for the test?

[kylekatarnls]

I will check why the PHAR build fails on Travis, but everything else is fine for me.

[ravage84]

@MarkVaughn adressed #765 (comment) in 7d8212c

[ravage84]

@frankdekker

Unsure about the priority for the two rules. I've set them to 3.

That's fine, I guess.

Unsure about the namespace for the Strings class. I took Laravels "Support/Str" namespace/class as example. I'm open for suggestions.

Updated it to Utility.

[ravage84]

@frankdekker thank you very much for your great work 😎

I updated your PR with lots of small improvements. Also see my comments.

[ravage84]

Any tips how to solve:
https://travis-ci.org/github/phpmd/phpmd/jobs/683292013

It seems that build.xml runs phpmd.phar on the PHPMD/Rule directory. This now triggers the LongClassName rule as there are some classes that exceed the 20 characters mark.

Should I add a phpmd.xml that will be used by build.xml with a higher allowed class name length or choose a different directory for the test?

Fixed by bumping the maximum threshold to 40. 😆

[ravage84]

@frankdekker

And very nice to see the changes. It more closer how I would have programmed it. It was kinda hard to decide if I should stick to the code style that I found in the other rules, and didn't improve as much a I wanted.

That is fine. To be honest, the code base is really old (some stuff dates back to 2008 or at least to 2012. In the past the code was maintained by a single person. Since last year a small team is maintaining the code and we slowly improve what we find.

Next to the coversDefaultClass I usually add @Covers ::function to each test method aswell. to explicitly specificy what the test covers. I didn't find that anywhere.

Funny, I do too, normally. But it was too late at night for me to add these, too.
Feel free to do that in a follow up PR.

I kinda dislike the name change of lengthWithoutSuffixes. The second parameter is a configuration parameter for calculating the length. If I would add more configurations options i would have to rename the function aswell.

My idea was to make the code more self expressive. Also, one should be cautious to add functionality through configuration parameters. This could lead to code that does too much or too many different things. It might be wiser to add a second method, instead.

I dislike the switching of the arguments of splitToList. My expectation would be separator / string. As I would compare it to implode and explode.

My idea was in the sense of "Split string using separator to get a list of it"

Just my ideas. Nothing blocking, nice changes :)

Your opinion is welcomed.

If you want, you can talk to us on our Gitter channel.

[frankdekker]

@kylekatarnls Thanks the approval. Just a heads up, i think there needs to be done an extra step to update the documentation on the website. Is not entirely up-to-date now.

See:
https://phpmd.org/rules/index.html#naming-rules and the the LongClassName link.

[kylekatarnls]

Indeed. Could you add the missing paragraph to https://github.com/phpmd/phpmd/blob/master/src/site/rst/rules/naming.rst following the example of ShortVariable?

Thanks,