[apex] New Rule - Test Methods Must Be In Test Classes

[noerremark]

Before submitting a PR, please check that:

• The PR is submitted against master. The PMD team will merge back to support branches as needed.
• ./mvnw clean verify passes. This will build and test PMD, execute PMD and checkstyle rules. Check this for more info

PR Description:
The title pretty must says what this is for. It fixes #639.

[rsoesemann]

Sorry to be the bad cop here but I think this is a rule with really little value. Everybody should use the annotation and not the keyword for years. The annotation can only be added to @istest classes.

[pmd-test]

	1 Message
📖	No java rules are changed!

Generated by 🚫 Danger

[noerremark]

Sorry to be the bad cop here but I think this is a rule with really little value. Everybody should use the annotation and not the keyword for years.

I did this because #639 was open, and felt like something easy to start on. But we can close #639 and abandon this PR too, if you like.

I don't necessarily disagree with the bad cop in you, but I DO think that "the Salesforce"-way justifies a rule like this for the following reasons:

1. I don't get a warning in Illuminated Cloud if I remove @istest from a class with tests. Having this in place may mean that I get that without building.
2. Since building happens Salesforce-side, it will always be slower than running PMD on local

Re: testMethod is still heavily used in examples in documentation, and I wouldn't be surprised if lots of people out there still uses it.

[dstdia]

There are two sides to this rule (and the general idea): In current API versions, the Apex Compiler will reject test method annotations if the class itself is not annotated with @istest. So you won't need any PMD rule because the compiler will find this at compile time.

The other aspect is: i'm reviewing implementations or I'm taking over dated codebases from time to time, and one of the first things that I do is running Apex PMD against the codebase. Finding code that still uses ancient API versions that allow for @Testmethod within regular classes (or even "require" these inline tests) would be much easier if there was a rule pointing at these classes.

You may argue if this is the primary focus of Apex PMD, and you probably don't need such a rule in a basic / general ruleset, but I think it's handy for retro-analysis.

[adangel]

I'd be ok with adding this rule as long as we mention the following points in the description:

• new Apex API versions enforce this rule already at compile time
• The rule is mostly useful when dealing with legacy code

[noerremark]

@adangel / @dstdia / @rsoesemann - Better now?

[rsoesemann]

Let’s add it. It’s just a simple XPath rule so not much code overhead. This is not a rule we need to make pmd more attractive but it’s also doesn’t do any harm.

[noerremark]

@adangel - What is the rule, can I merge it, or will you?

[adangel]

@noerremark I'll merge it

[adangel]

Merged - this will be part of PMD 6.22.0