JS: line indentation error when using setTimeout() inline function

[BoboTiG]

Using PHP_CodeSniffer 2.6.2, there is no issue. But with the latest 2.7.1, this snippet ends on an error:

function a()
{
    return;
}

class TestOk
{
    destroy()
    {
        setTimeout(a, 1000);

        if (typeof self.callbackOnClose === "function") {
            self.callbackOnClose();
        }
    }
}

class TestBad
{
    destroy()
    {
        setTimeout(function () {
            return;
        }, 1000);

        if (typeof self.callbackOnClose === "function") {
            self.callbackOnClose();
        }
    }
}

The error:

$ php phpcs-2.7.1.phar --standard=PSR2 test.js

----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 26 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
    |       |     8
 28 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
    |       |     8

phpcbf does fix the identation for phpcs 2.7.1, but it is not correct:

function a()
{
    return;
}

class TestOk
{
    destroy()
    {
        setTimeout(a, 1000);

        if (typeof self.callbackOnClose === "function") {
            self.callbackOnClose();
        }
    }
}

class TestBad
{
    destroy()
    {
        setTimeout(function () {
            return;
        }, 1000);

    if (typeof self.callbackOnClose === "function") {
        self.callbackOnClose();
    }
    }
}

Now, it is phpcs 2.6.2 who cries, and he is right:

$ php phpcs-2.6.2.phar --standard=PSR2 test.js

----------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
 26 | ERROR | [x] Line indented incorrectly; expected 8 spaces, found
    |       |     4
 27 | ERROR | [x] Line indented incorrectly; expected at least 12
    |       |     spaces, found 8
 28 | ERROR | [x] Line indented incorrectly; expected 8 spaces, found

Thanks for your time :)

[gsherwood]

The problem occurs because the class methods are not prefixed with function and PHPCS is tokenizing them as objects. The scope indent sniff uses objects differently to functions/methods when calculating indents. I need to look into the tokenizer more.

[gsherwood]

Given that there is no keyword to latch onto, the tokenizer would need to repurpose the name of the method (normally a T_STRING) and make it a custom token type. But this would stop all sniffs that listen for T_FUNCTION (including, but definitely not limited to, the scope indent sniff) recognising this structure as a standard function. Plus, they all assume the next token after the word function is a T_STRING with the name of the function in there, so they would need to change that assumption as well.

So this is a pretty big change that's going to take a while. I'm not entirely sure of the extent of the change, but it feels like a big BC break, so it might not be done for a while, and will probably only be available in 3.0.

The fact it worked in older version is due to the scope indent sniff having bugs where it wasn't applying rules correctly. It just so happened that the incorrect tokenizing wasn't affecting the indent because it wasn't being calculated correctly anyway. So I can't just revert back a change.

[aik099]

Plus, they all assume the next token after the word function is a T_STRING with the name of the function in there, so they would need to change that assumption as well.

For anonymous functions next token after function keyword isn't a T_STRING with a function name. At least not for a PHP sniffs. So the anonymous functions in JavaScript are not supported by any JS sniffs?

[gsherwood]

For anonymous functions next token after function keyword isn't a T_STRING with a function name. At least not for a PHP sniffs. So the anonymous functions in JavaScript are not supported by any JS sniffs?

Yes, they are supported. I was talking about class methods, but used "function" too broadly in my comment. For both methods, global functions, and closures, the function keyword is not currently optional in either the PHP or JS tokenizers.

[aik099]

Ah, I've just noticed that ES6 function/method declaration syntax was used. This is surely major change and needs to go into 3.0 version.

Maybe a tokenizer can translate ES6 syntax into tokens for ES5 syntax, e.g.

(param) => {
   ...
}

should be tokenized as if it was this code

function (param) {
   ...
}

[BoboTiG]

Thanks @gsherwood and @aik099, take your time for this issue, I can use the 2.6.1 version for the time being :)

[gsherwood]

I've committed a change to the 3.0 branch that fixes this specific issue by tokenizing ES6 methods as T_FUNCTION structures. I've changed internal sniffs to support this as well. It turns out that most don't really care about the function name and jump straight to the braces, which I support in the core tokenizer for this syntax, so they don't need changing.

I haven't added support for any other type of new syntax because I just don't have the time. But hopefully someone will let me know if something isn't working.

If you want to give this a go, you'll need to grab PHPCS directly from the 3.0 branch. Easiest way for testing is just to do this:

git clone https://github.com/squizlabs/PHP_CodeSniffer.git
cd PHP_CodeSniffer
php bin/phpcs ...