Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Allow simple if statement without else to have no braces in codestyle

I use checkstyle to check if my java code respects the guidelines of our project.

However, we have one guideline that I cannot figure out how to check with this tool. We want to allow simple if (understand if with no else and no other conditional structure in it) to have no brace, like in this example :

// valid
if(condition) callFunction();

// invalid
if(condition) for(int i = 0; i < someValue; i++) callFunction(i);

// valid
if(condition) {
    for(int i = 0; i < someValue; i++) {
        callFunction(i);
    }
}

// invalid
if(condition) callFunction();
else callOtherFunction();

This convention can be discussed, but this is the one we chose. It allows a reduced if syntax for very trivial cases, but ensures we have good indentation and block delimitation for more complex structures.

Any help with that would be really appreciated.

I'm also ready to do some code to perform this check if nothing is available, but really don't know where to start. In last ressort, some tips about that will be appreciated too.

like image 632
deadalnix Avatar asked May 26 '11 16:05

deadalnix


2 Answers

At the end, I did implement a custom check for checkstyle. Here is the source code if someone else is interested in it :

import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;

public class IfBracesCheck extends Check {

    @Override
    public int[] getDefaultTokens() {
        return new int[] {
            TokenTypes.LITERAL_ELSE,
            TokenTypes.LITERAL_IF,
        };
    }

    @Override
    public void visitToken(DetailAST aAST) {
        final DetailAST slistAST = aAST.findFirstToken(TokenTypes.SLIST);

        if(aAST.getType() == TokenTypes.LITERAL_ELSE) {
            // If we have an else, it must have braces, except it is an "else if" (then the if must have braces).
            DetailAST ifToken = aAST.findFirstToken(TokenTypes.LITERAL_IF);

            if(ifToken == null) {
                // This is an simple else, it must have brace.
                if(slistAST == null) {
                    log(aAST.getLineNo(), "ifBracesElse", aAST.getText());
                }
            } else {
                // This is an "else if", the if must have braces.
                if(ifToken.findFirstToken(TokenTypes.SLIST) == null) {
                    log(aAST.getLineNo(), "ifBracesConditional", ifToken.getText(), aAST.getText() + " " + ifToken.getText());
                }
            }
        } else if(aAST.getType() == TokenTypes.LITERAL_IF) {
            // If the if uses braces, nothing as to be checked.
            if (slistAST != null) {
                return;
            }

            // We have an if, we need to check if it has no conditionnal structure as direct child.
            final int[] conditionals = {
                TokenTypes.LITERAL_DO,
                TokenTypes.LITERAL_ELSE,
                TokenTypes.LITERAL_FOR,
                TokenTypes.LITERAL_IF,
                TokenTypes.LITERAL_WHILE,
                TokenTypes.LITERAL_SWITCH,
            };

            for(int conditional : conditionals) {
                DetailAST conditionalAST = aAST.findFirstToken(conditional);

                if (conditionalAST != null) {
                    log(aAST.getLineNo(), "ifBracesConditional", aAST.getText(), conditionalAST.getText());

                    // Let's trigger this only once.
                    return;
                }
            }
        }
    }
}
like image 181
deadalnix Avatar answered Oct 19 '22 06:10

deadalnix


Just want to add that now checkstyle supports 'allowSingleLineIf' property which covers some cases.

    <module name="NeedBraces">
        <property name="allowSingleLineIf" value="true"/>
    </module>
like image 42
4ndrew Avatar answered Oct 19 '22 07:10

4ndrew