New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config: forbid assert token by Illegal token Check #3751

Closed
romani opened this Issue Jan 24, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@romani
Member

romani commented Jan 24, 2017

https://github.com/checkstyle/checkstyle/blob/master/config/checkstyle_checks.xml#L223

Assert token should be added.
Why we have two lines for tokens ? It is override. Should be forbidden by XML schema.

@romani romani added the approved label Jan 24, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 24, 2017

Member

@romani

Why we have two lines for tokens ? It is override. Should be forbidden by XML schema.

DefaultConfiguration merges multiple properties together like it was one line. Some were already like this in the config, so I thought it was acceptable for history viewing to clearly show removes/adds.
There is an old commit where tokens were purposely made like this. Most of those changes and documentation are still around.
Ex: http://checkstyle.sourceforge.net/property_types.html#stringSet

this property can be supplied multiple times which is equivalent to a set of comma separated strings.

Member

rnveach commented Jan 24, 2017

@romani

Why we have two lines for tokens ? It is override. Should be forbidden by XML schema.

DefaultConfiguration merges multiple properties together like it was one line. Some were already like this in the config, so I thought it was acceptable for history viewing to clearly show removes/adds.
There is an old commit where tokens were purposely made like this. Most of those changes and documentation are still around.
Ex: http://checkstyle.sourceforge.net/property_types.html#stringSet

this property can be supplied multiple times which is equivalent to a set of comma separated strings.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Feb 16, 2017

Member

fix for config is merged.

Member

romani commented Feb 16, 2017

fix for config is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment