New Check: AnnotatedMethodNameCheck to check method names of annotated (test) methods

[AdoraMel]

I'd like to a request a new check that makes sure the method names of annotated methods adhere to a specific pattern.

How it works Now:

Check MethodName allows to check that method names conform to a specified pattern. However, it can not differentiate between annotated methods.

Is your feature request related to a problem? Please describe.

Some coding conventions, like the one used for my uni project, set rules for the naming of test methods, typically annotated with @Test.
The Google Conventions also shows an example of a specific pattern for test methods.
By implementing the proposed Check, those conventions could be enforced via Checkstyle.

Describe the solution you'd like

config.xml

<module name="Checker">
  <module name="TreeWalker">
    <module name="AnnotatedMethodNameCheck">
      <property name="annotations" value="@Test"/>
      <property name="methodPattern" value="^test.*$"/>
    </module>
  </module>
</module>

• annotations: All annotations that will trigger the check
• methodPattern: The regex pattern the name of an annotated method must adhere to

Test.java

import org.junit.Test;

public class Test {

  @Test
  public void additionTest() {}

  @Test
  public void testSubtract() {}
}

Expected Output

$ java -jar checkstyle-10.14.2-all.jar -c config.xml Test.java 
Starting audit...
[ERROR] C:\checkstyle testing\Test.java:6: Method name "additionTest" does not match pattern "^test.*$" [AnnotatedMethodName]
Audit done.
Checkstyle ends with 1 errors.

Additional context

Another Example shows how naming conventions for other testing Annotations can be enforced. Suppose @BeforeEach methods shall be named setUp():

config.xml

<module name="Checker">
  <module name="TreeWalker">
    <module name="AnnotatedMethodNameCheck">
      <property name="annotations" value="@BeforeEach"/>
      <property name="methodPattern" value="setUp$"/>
    </module>
  </module>
</module>

Example 1
The naming convention is followed.

import org.junit.Test;

public class Test1 {

  @BeforeEach
  public void setUp() {} //matches "setUp$"

  //test methods...
}

Example 2
The naming convention is not followed.

import org.junit.Test;

public class Test2 {

  @BeforeEach
  public void init() {} //violation, doesn't match "setUp$"

  //test methods...
}

Implementation Proposal

I have already implemented a possible solution for this problem. This may be useful as starting point for creating a solution for the Checkstyle project.

[MANISH-K-07]

The Google Conventions also suggest a specific pattern for test methods.
By implementing the proposed Check, those conventions could be enforced via Checkstyle.

Could you please explain a bit on where a specific pattern is mentioned for annotated test methods?
From what I know, google style guide only asks for the method names to follow lowerCamelCase.. am I wrong here?

does not match pattern "^test.*$"

I agree we may use this by convention (even in our code) but enforcing this on all users is a major debate.

Some coding conventions, like the one used for my uni project

Could you please share examples of such style schemes?

[AdoraMel]

@MANISH-K-07

Could you please explain a bit on where a specific pattern is mentioned for annotated test methods? From what I know, google style guide only asks for the method names to follow lowerCamelCase.. am I wrong here?

The Google Style Guide doesn't specify a pattern for test methods:

There is no One Correct Way to name test methods.

However it suggests that a pattern may be used:

One typical pattern is <methodUnderTest>_<state>, for example pop_emptyStack

---

does not match pattern "^test.*$"

I agree we may use this by convention (even in our code) but enforcing this on all users is a major debate.

Both the pattern and annotations are properties which can be adjusted to the user's needs.
I think those might be good default values tho. But we can totally change those. It's just the example that prompted me to make a Check like this.

---

Some coding conventions, like the one used for my uni project

Could you please share examples of such style schemes?

Test methods schould be named "test" + method name.

method that should be tested	test method
removeObject	testRemoveObject
removeKey	testRemoveKey

There are some special methods with specific annotations which should always be named the same:

method with annotation	name of the method
@BeforeEach	setUp()
@AfterEach	tearDown()
@BeforeAll	setUpAll()
@AfterAll	tearDownAll()

[MANISH-K-07]

Test methods schould be named "test" + method name.
There are some special methods with specific annotations which should always be named the same:

Some coding conventions, like the one used for my uni project, set rules for the naming of test methods,

I have understood your vision previously. Please provide sources/links to such code styles that support your statement above

However it suggests that a pattern may be used:

A bit of clarity here.. the new check wants to focus only on targeted methods ? (I don't think this is what google style implied)
Because we already have a property that allows this CS check to be customized by user with regex for method name identifier?

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/MethodNameCheck.java

Lines 68 to 70 in 4fe8118

	* Property {@code format} - Sets the pattern to match valid identifiers.
	* Type is {@code java.util.regex.Pattern}.
	* Default value is {@code "^[a-z][a-zA-Z0-9]*$"}.

[AdoraMel]

@MANISH-K-07

I want only certain annotated methods to be checked. The current MethodName Check doesn't allow that functionality. It will always check methods whether annotated or not.

Actually, the best thing to do would be to extend the current MethodName Check to be able to filter only certain annotated methods. Then a new Check won't be necessary. I suppose this issue could be changed to an extension for MethodName then?

Please provide sources/links to such code styles that support your statement above

I have provided a link to the Google conventions, which mentions a pattern. I cannot link to internal uni project conventions tho. But I am sure there are many conventions for test method out there. Regardless, I think this is a useful feature, allowing users more control.

[MANISH-K-07]

It will always check methods whether annotated or not.

This doesn't need a new check. We make use of certain properties to restrict/limit/enhance/extend existing check accordingly.

I have provided a link to the Google conventions, which mentions a pattern
But I am sure there are many conventions for test method out there

This is where we are stumbling around.. From what I know of the project, we don't introduce new functionalities based on assumptions or very few uni conventions. For a new functionality to be implemented, it needs to be backed up by a reliable code style scheme. I hope you understand what I mean, we can't force users to follow something that is limited to few unis or random conventions.

Regardless, I think this is a useful feature, allowing users more control.

This, however is not in my power to decide, let's wait for the maintainers' opinion.
If they decide it beneficial for the project, then a new property will suffice, new check is not required :)

[romani]

conceptually, after shallow review, I am good with proposal.

can this be a solution https://checkstyle.org/checks/coding/matchxpath.html#MatchXpath ?

you can find bunch of example in our issue tracker and discussions, like #12975 (comment)

[MANISH-K-07]

can this be a solution https://checkstyle.org/checks/coding/matchxpath.html#MatchXpath ?

Theoretically yes, we should be able to evaluate an xpath query that matches targeted node

[AdoraMel]

@romani

can this be a solution https://checkstyle.org/checks/coding/matchxpath.html#MatchXpath ?

I believe so, yes. I have not looked much into Xpaths, but they seem very powerful.

In that case, I think it might be useful to mention them in the documentation more explicitly. I had the impression I would have to change or add a new Check. This would keep users from implementing checks that Xpath could accomplish easily.

conceptually, after shallow review, I am good with proposal.

Does that mean the functionality I described should be added to Checkstlye? Or it should not, since Xpath could do the same?

[romani]

Try to make it by xpath first, such attempt usually shows and items that xpath is not able to do. So reason for new Check will be more explicit.