Google java style method naming checks

48 views
Skip to first unread message

Pavel Baranchikov

unread,
Oct 15, 2015, 4:34:11 AM10/15/15
to checkstyle-devel
Hello.

According to https://google.github.io/styleguide/javaguide.html#s5.2.3-method-names

> Underscores may appear in JUnit test method names to separate logical components of the name. One typical pattern is test<MethodUnderTest>_<state>, for example testPop_emptyStack.

I found, that checkstyle instead allow this method naming (using underscore) in google checks for all the methods.

What's your opinion to add special name format for methods, marked as @Test in some way? In general, I can propose the following:
- restrict method name for the specified pattern if method is marked by one of the specified annotation
- -''- if the method is not marked by one of the specified annotation.

So, the result checkstyle configuration should look like the following:

<module name="MethodName">
<property name="format" value="[a-z][a-zA-Z0-9]*"/> // camelCase
<property name="checkAnnotatedMethods" value="false"/>
<property name="annotationCanonicalNames" value="org.junit.Test"/>
</module>
<module name="MethodName">
<property name="format" value="[a-z][a-zA-Z0-9]*(_?[a-z][a-zA-Z0-9]*)*"/> // camelCase_withOptions
<property name="checkAnnotatedMethods" value="true"/>
<property name="annotationCanonicalNames" value="org.junit.Test"/>
</module>

The default values should be checkAnnotatedMethods = "false", annotationCannonicalNames = ""

We already have the similar approach in VisibilityModifier in order to allow @Rule annotated fields to be declared as public.

Best regards,
Pavel.

Pavel Baranchikov

unread,
Oct 19, 2015, 3:30:43 PM10/19/15
to checkstyle-devel
Any objections here?

Should I start implementing?

Roman Ivanov

unread,
Oct 20, 2015, 1:08:43 AM10/20/15
to Pavel Baranchikov, checkstyle-devel
Hi Pavel,

your post is still in my TODO list, I will reply to you for sure, sorry, too much project activity on me ... .

We had somewhere in mail list discussion about this and reasons why we did that decision, I just need a bit more time to answer your with details.

​thanks,
Roman Ivanov

Roman Ivanov

unread,
Oct 23, 2015, 10:07:19 AM10/23/15
to Pavel Baranchikov, checkstyle-devel
Hi Pavel,

> What's your opinion to add special name format for methods, marked as @Test in some way? 


I found, that checkstyle instead allow this method naming (using underscore) in google checks for all the methods

Here is brief explanation: http://checkstyle.sourceforge.net/google_style.html#a5.2.3 if you find problems in Style interpretation, this page will answer it most likely.

Here is what I found for this Check update and configuration during Google style guide implementation:

So reasons:
 
1) test code is not that simple to detect, Junit is not the only test framework in Java. If we can not do this reliably - let not do this. User can use suppression 
and Check configuration will cover the main part of code.

2) Google's Style (config) is not used in Google :) - proven by practice and reviewing code of Google in github and googlecode. Each team use its own slightly-medium changed version of this Style. So this config serving the whole Java community as main target.

3) I also used to have a habit to have "_" in test names, but after we removed them from, I never regret :). I hope the same will happen in Google style - C style naming will go away. Google already confirmed that its Style is not exact and several items need to be changed. Details: https://github.com/google/guava/issues/1891

4) Test code is always different from production, not all Checks are applicable to it. See our own suppression - https://github.com/checkstyle/checkstyle/blob/master/config/suppressions.xml
So usage of suppression in encouraged.
By suppressing the whole Check in test folder, user will not loose that much control as Java does not allow anything in names. The only symbols are left: "$", unicode .Usage of that symbols is so rear so I would not make a effort to cover them.


thanks,
Roman Ivanov
Reply all
Reply to author
Forward
0 new messages