[2.x] Indention Issues With Closures

[mbabker]

See https://travis-ci.org/joomla/jissues/jobs/273897032 for reference

Basically, indents for control structures aren't being handled correctly in Closures

[photodude]

possibly related to this issue https://github.com/squizlabs/PHP_CodeSniffer/issues/1580

[photodude]

Here is a case that is failing from the CMS administrator\components\com_installer\models\languages.php

The sniff that is at issue is Joomla.ControlStructures.ControlStructuresBrackets.SpaceBeforeBrace

expected

// Sort the array by value of subarray
usort(
    $languages,
    function($a, $b) use ($that)
    {
        $ordering = $that->getState('list.ordering');

        if (strtolower($that->getState('list.direction')) === 'asc')
        {
            return StringHelper::strcmp($a->$ordering, $b->$ordering);
        }
        else
        {
            return StringHelper::strcmp($b->$ordering, $a->$ordering);
        }
    }
);

Current result of the sniff

// Sort the array by value of subarray
usort(
    $languages,
    function($a, $b) use ($that)
    {
        $ordering = $that->getState('list.ordering');

        if (strtolower($that->getState('list.direction')) === 'asc')
    {
            return StringHelper::strcmp($a->$ordering, $b->$ordering);
        }
        else
    {
            return StringHelper::strcmp($b->$ordering, $a->$ordering);
        }
    }
);

[photodude]

Another CMS case from plugins\system\sef\sef.php where Joomla.ControlStructures.ControlStructuresBrackets.SpaceBeforeBrace fails on closures

expected

$buffer = preg_replace_callback(
    $regex,
    function ($match) use ($base, $protocols)
    {
        preg_match_all('#(?:[^\s]+)\s*(?:[\d\.]+[wx])?(?:\,\s*)?#i', $match[1], $matches);

        foreach ($matches[0] as &$src)
        {
            $src = preg_replace('#^(?!/|' . $protocols . '|\#|\')(.+)#', $base . '$1', $src);
        }

        return ' srcset="' . implode($matches[0]) . '"';
    },
    $buffer
);

Current result from sniff

$buffer = preg_replace_callback(
    $regex,
    function ($match) use ($base, $protocols)
    {
        preg_match_all('#(?:[^\s]+)\s*(?:[\d\.]+[wx])?(?:\,\s*)?#i', $match[1], $matches);

        foreach ($matches[0] as &$src)
    {
            $src = preg_replace('#^(?!/|' . $protocols . '|\#|\')(.+)#', $base . '$1', $src);
        }

        return ' srcset="' . implode($matches[0]) . '"';
    },
    $buffer
);

[photodude]

I've done a little digging here. seems the issue might stem from our reliance on $tokens[$stackPtr]['level'] at line 176 to determine the expected indent for the scope opening brace.

I propose we remove this portion of our rule and rely on the following parts of the ruleset that also cover this indenting

Generic.WhiteSpace.ScopeIndent.IncorrectExact Generic.WhiteSpace.ScopeIndent.Incorrect Squiz.WhiteSpace.ScopeClosingBrace.Indent PEAR.Functions.FunctionCallSignature.Indent

note: Generic.WhiteSpace.ScopeIndent reports in tabs as expected.

I wonder what the overlap is and if we can eliminate some sniffs so there is no duplication in the checks.

[photodude]

218 now fixes this issue and has a test to verify that it fixes the issue.

[mbabker]

This might be covered by the same, but will this also cover PHP 7 anonymous classes, i.e. from com_code?

[photodude]

Yes, I believe it should as I included T_ANON_CLASS for Anonymous classes in the conditional for the fix.

[photodude]

Just did a test locally, seems to be good. I'll throw a test together.

[photodude]

Close via #218