Checkstyle: New Check NoEnumTrailingComma

Created on 12 Aug 2019  路  8Comments  路  Source: checkstyle/checkstyle

Followup 2 to #6469

Imho it looks strange to have a trailing comma, so I'd rather like to be able to check that no trailing comma is ever used. This will be directly competing with EnumTrailingComma.

_Examples:_

enum Foo1 {
  FOO, // OK
  BAR, // violation
  ;
}
enum Foo2 {
  FOO, // OK
  BAR,; // violation
}
enum Foo3 { FOO, BAR,; } // violation
enum Foo4 {
  FOO, // OK
  BAR, // violation
}
enum Foo5 { FOO, BAR, } // violation
enum Foo6 {
  FOO, // OK
  BAR // OK
  ;
}
enum Foo7 {
  FOO, // OK
  BAR; // OK
}
enum Foo8 { FOO, BAR; } // OK
enum Foo9 {
  FOO, // OK
  BAR // OK
}
enum Foo10 { FOO, BAR } // OK

_Current Work-Around only for enums:_

<module name="IllegalToken">
    <property name="id" value="trailingEnumComma"/>
    <property name="tokens" value="COMMA"/>
    <message key="illegal.token" value="Do not use trailing enum comma."/>
</module>
<module name="SuppressionXpathSingleFilter">
    <property name="id" value="trailingEnumComma"/>
    <property name="query" value="(
                                      //ENUM_DEF/OBJBLOCK/*[self::COMMA and not(position() = (last() - 1))],
                                      //COMMA[not(parent::OBJBLOCK/parent::ENUM_DEF)]
                                  )"/>
</module>

_Current Work-Around for arrays and enums:_

<module name="IllegalToken">
    <property name="id" value="trailingComma"/>
    <property name="tokens" value="COMMA"/>
    <message key="illegal.token" value="Do not use trailing comma."/>
</module>
<module name="SuppressionXpathSingleFilter">
    <property name="id" value="trailingComma"/>
    <property name="query" value="(
                                      //ARRAY_INIT/*[self::COMMA and not(position() = (last() - 1))],
                                      //ENUM_DEF/OBJBLOCK/*[self::COMMA and not(position() = (last() - 1))],
                                      //COMMA[not(parent::ARRAY_INIT or parent::OBJBLOCK/parent::ENUM_DEF)]
                                  )"/>
</module>
approved new feature new module

Most helpful comment

I wasn't a big fan of dangling commas for aesthetic reasons. This changed my mind :

https://medium.com/@nikgraf/why-you-should-enforce-dangling-commas-for-multiline-statements-d034c98e36f8

All 8 comments

I wasn't a big fan of dangling commas for aesthetic reasons. This changed my mind :

https://medium.com/@nikgraf/why-you-should-enforce-dangling-commas-for-multiline-statements-d034c98e36f8

I know these arguments, but they don't change my mind. :-)

I removed all unrelated discussions ...
Style is something that hard to discuss, user should be able to make your own style, even it is not very proven practice.
I am ok with such check, we will not use it, but in general idea is OK, especially if @Vampire will provide implementation of it.

@rnveach , please 2nd review of this idea and put label to unblock implementation.

@romani I am fine with the check. I just had 1 thing to point out for our own purposes.

BAR, // violation
BAR,; // violation
enum Foo3 { FOO, BAR,; } // violation

For our own repo, I could see us wanting a violation on the 2nd and 3rd example but not the first.
What do you think of adding an option for this?

I am ok to have such option, users will like it for sure. with such option activated we can enable such Check over our code that is very good approach to show that we use what we produce.

@rnveach I would like to work on this.

@sd1998 , thanks a lot for help.
If you see approved issue, no activity in issue, simply place comment " I am on it" or something like this and you are welcome with PR.

Fix was merged.

Was this page helpful?
0 / 5 - 0 ratings