phpcheckstyle icon indicating copy to clipboard operation
phpcheckstyle copied to clipboard

The statement if must contain its code within a {} block is reported incorrectly

Open jbrooksuk opened this issue 11 years ago • 5 comments

This code:

App::error(function(Exception $exception, $code) {
    Log::error($exception);
    if (Config::get('app.debug') === FALSE && $code <> 404) {
        switch ($code) {
            case 401: $errorTitle = 'Unauthorized'; break;
            case 403: $errorTitle = 'Forbidden'; break;
            // case 404: $errorTitle = 'Not Found'; break;
            case 408: $errorTitle = 'Request Timeout'; break;
            case 500: $errorTitle = 'Internal Server Error'; break;
            case 502: $errorTitle = 'Gateway Timeout'; break;
            case 503: $errorTitle = 'Service Unavailable'; break;
            case 508: $errorTitle = 'Loop detected'; break;
            default: $errorTitle  = 'Unknown Error';
        }

        $hasView = View::exists('errors.' . $code);

        return Response::view('errors.' . ($hasView ? $code : 500), [
            'errorCode'  => $code,
            'exception'  => $exception,
            'errorTitle' => $errorTitle
        ], $code);
    }
});

Is being reported as needing the if condition to be in its own block. The problem is that it already is.

I don't think that I've broken this?

jbrooksuk avatar Sep 02 '14 09:09 jbrooksuk

@tchule I've narrowed down the issue to being anything which calls a function within the condition itself. See this test file I've just added.

jbrooksuk avatar Sep 02 '14 13:09 jbrooksuk

If you remove the App::error closure and only leave the condition, it's fine.

jbrooksuk avatar Sep 02 '14 13:09 jbrooksuk

The offending code is:

if (!$this->tokenizer->checkNextValidToken(T_BRACES_OPEN)) {
    $msg = $this->_getMessage('NEED_BRACES', $stmt);
    $this->_writeError('needBraces', $msg);
}

It's too strict. The code only needs to be concerned with the fact that it ends with a {.

That, or we need a way of knowing if a control statement contains a body. This is where an AST would be useful.

jbrooksuk avatar Sep 02 '14 20:09 jbrooksuk

Problem occurs because the closing parenthese after debug') is considered as the end of the IF, nextValidToken is then === instead of {

Should count the opening and closing parentheses.

tchule avatar Feb 10 '17 14:02 tchule

Humm, ok, we already count the opening/closing.

Problem is in "_processParenthesisClose()", this part of the code existed before we used a stack. We use flags to know if we are in a function call or a control statement. But we fail to recognize the anonymous function structure. These flags should be replaced by a proper use of the statement stack, but it impacts a lot of code.

tchule avatar Feb 13 '17 15:02 tchule