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

refactoring: RightCurlyCheck code expression #3764

Closed
romani opened this Issue Jan 27, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@romani
Member

romani commented Jan 27, 2017

https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/RightCurlyCheck.java#L174

    @Override
    public void visitToken(DetailAST ast) {
        final Details details = getDetails(ast);
        final DetailAST rcurly = details.rcurly;

        if (rcurly != null && rcurly.getType() == TokenTypes.RCURLY) {
        .....

Why rcurly variable could store non TokenTypes.RCURLY .
I might miss smth, but expected code is

       .....
        if (rcurly != null) {
       .....
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 27, 2017

Member

Tokens else and if aren't verifying that they have curlies or not so it is picking up whatever it finds.

nextToken = ast.findFirstToken(TokenTypes.LITERAL_ELSE);
if (nextToken == null) {
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
lcurly = ast.getLastChild();
rcurly = lcurly.getLastChild();
}
else {
lcurly = nextToken.getPreviousSibling();
rcurly = lcurly.getLastChild();
}

case TokenTypes.LITERAL_ELSE:
case TokenTypes.LITERAL_FINALLY:
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
lcurly = ast.getFirstChild();
rcurly = lcurly.getLastChild();

Member

rnveach commented Jan 27, 2017

Tokens else and if aren't verifying that they have curlies or not so it is picking up whatever it finds.

nextToken = ast.findFirstToken(TokenTypes.LITERAL_ELSE);
if (nextToken == null) {
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
lcurly = ast.getLastChild();
rcurly = lcurly.getLastChild();
}
else {
lcurly = nextToken.getPreviousSibling();
rcurly = lcurly.getLastChild();
}

case TokenTypes.LITERAL_ELSE:
case TokenTypes.LITERAL_FINALLY:
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
lcurly = ast.getFirstChild();
rcurly = lcurly.getLastChild();

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 27, 2017

Member

get details method should put in field only curly AST token

Member

romani commented Jan 27, 2017

get details method should put in field only curly AST token

romani added a commit that referenced this issue Jan 27, 2017

@romani romani added this to the 7.5 milestone Jan 27, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 27, 2017

Member

Fix is merged

Member

romani commented Jan 27, 2017

Fix is merged

@romani romani closed this Jan 27, 2017

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