Question: how to best make this extension to CamelCaseMethodName

[JeroenDeDauw]

In my projects, camelCase is required for method names. There however is an exception: tests. This is also allowed: testGivenInvalidLanguage_exceptionIsThrown.

The rules for this are:

• only applies to methods starting with "test" (in classes that end on "Test")
• only a single _ is allowed (though not required)
• the character after the _ should be lowercase
• the _ should separate the "given" and/or "when" from the "then" clause
  • 3 or more words (including "test") before the _
  • 2 or more words after the _

I'd like to implement this. Any advice on how to do this? Shall I create a new standalone rule, one that extends CamelCaseMethodName, or add an option to CamelCaseMethodName? And for the former two, would that be a welcome addition to this repo or not?

[hanneskaeufler]

+1 Would definetely like this as well. I follow a similar naming pattern for tests.
The naming rules define allow to define an exception
https://github.com/phpmd/phpmd/blob/master/src/main/resources/rulesets/naming.xml#L22

Maybe an exception allowing a regexp pattern could be added to the CamelCaseMethodName rule?

<property name="exception" value="^test_"></property>

[JeroenDeDauw]

I've added an option in #257. It's a boolean option to allow for a single underscore in the method name of a test. It's not as strict as it can be - some regex wizard can probably make it better.

[JeroenDeDauw]

The basic option got merged now. Just came up with a regex that applies all rules I outlined (except the class context one):

/^test(([A-Z][a-z0-9]+)+|(([A-Z][a-z0-9]+){2,}_[a-z0-9]+([A-Z][a-z0-9]+)+))$/

Will submit a PR with that soon

[ravage84]

@JeroenDeDauw 🆒

[JeroenDeDauw]

I just noticed the 2.3 has already happened. That means updating the regex to be more restrictive would be a breaking change. Perhaps adding a new rule specifically for test methods would be good? Then you can enabled just that rule for your test methods, or also have the CamelCase one on with the already introduced allow-underscore-test option.

[ravage84]

According to SemVer it would, yes. But for this time, so shortly after release, I would give it a go, nonetheless. It was a new feature and it was merged just before the release, so we can assume very few people have it already implemented. But thanks for caring!

[JeroenDeDauw]

I have started implementation in #313. Still need to add the integration tests and make this rule not apply to methods starting with test in non test classes.

The approach to testing there is a bit different than what is done for the other rules, as I really did not want to create dozens of integration test files to test all edge cases of the regex.

[ravage84]

@JeroenDeDauw though I closed your PR, here some other thoughts on your issue:

I guess the actual "problem" is that you use no valid naming style. testGivenInvalidLanguage_exceptionIsThrown is simply not a widely accepted way of naming methods. Think of it, nobody gave it a name yet. It's not a standard, yet.

The fact, that PHPMD might also does stuff it's not 100 % made for (checking method names etc.) doesn't help either. PHPCS might be the better tool for that.

Anyway, a last idea that might help not only you:
What if we implement a ignore-in-tests property, which we can use in all PHPMD rules to disable the rule for test files/classes/methods? As for the definition of what a test file/class/method is, I would orient myself on PHPUnit's conventions.

[JeroenDeDauw]

Closed PR: #313

---

@ravage84 perhaps it's a bit premature to close this issue then?

IIRC we can already have the naming rule be ignored in tests, simply leaving us with no check for the names at all. Not been suing PHPMD for some time due to PHP 7 compat not being there, so I don't recall the details.

[ravage84]

Yes, one could simply exclude the test folder for some rules. So it's arguable if the ignore-in-tests property would of any use. That's why I did not create a new issue for it.

Feel free to re-open, though I think the question of this issue is quite specific. This specific that I think that it's not even a fitting task for PHPMD, but more one for PHPCS, because PHPCS is about style and - for me - your issue is a style question, not a mess to detect.

[ravage84]

By the way the PHP 7 compat should have been improved, as most issues were fixed in Pdepend.

[JeroenDeDauw]

I think it's weird to have some naming convention rules and then not accept additions cause the tool is not about naming conventions.

People not part of the org cannot re-open tickets

[ravage84]

I see your point. While I personally use the CamelCaseXyz rules myself and even extended some to add a missing feature, I feel we should not evolve much further into this territory with PHPMD, as I think there are more fitting tools, such as PHPCS. This is my personal view, not sure how @manuelpichler feels about this. Mainly it his project, I came late to the show, when all those rules existed already.

People not part of the org cannot re-open tickets

Oh, didn't know/realize. Shall I re-open then?

[JeroenDeDauw]

I am no longer using PHPMD in part because of this issue, so yeah, still relevant.