[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 squizlabs/PHP_CodeSniffer#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