Verifying tokens in checkstyle config handles default tokens wrong #4119

Closed
rnveach opened this Issue Mar 29, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@rnveach
Member

rnveach commented Mar 29, 2017

Found in #4117 (comment) , and confirmed by looking in our configuration file

<module name="RightCurly">
<property name="tokens" value="METHOD_DEF"/>
<property name="tokens" value="CTOR_DEF"/>
<property name="tokens" value="CLASS_DEF"/>
<property name="tokens" value="INSTANCE_INIT"/>
<property name="tokens" value="LITERAL_FOR"/>
<property name="tokens" value="STATIC_INIT"/>
<property name="tokens" value="LITERAL_WHILE"/>
<property name="option" value="alone"/>
</module>
<module name="RightCurly">
<property name="tokens" value="LITERAL_DO"/>
<property name="option" value="same"/>
</module>
.

We do not validate all tokens in RightCurly. We are missing LITERAL_IF and LITERAL_ELSE and others.

Problem is with AllChecksTest.validateAllCheckTokensAreReferencedInConfigFile and how we handle default tokens in

configTokens.addAll(CheckUtil.getTokenNameSet(check.getDefaultTokens()));
.

Once fixed, we will have to re-confirm all new tokens found, if they should be added to our config or not.
This will also affect google config.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 29, 2017

Member

each change in google config should be proved by quote from style guide.

Member

romani commented Mar 29, 2017

each change in google config should be proved by quote from style guide.

rnveach added a commit to rnveach/checkstyle that referenced this issue Mar 29, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 9, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 9, 2017

romani added a commit that referenced this issue Apr 12, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Apr 12, 2017

Member

Fix was merged

Member

rnveach commented Apr 12, 2017

Fix was merged

@rnveach rnveach closed this Apr 12, 2017

@rnveach rnveach added this to the 7.7 milestone Apr 12, 2017

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