Add test case for issue found in the CMS with closures (#250)

Author: wilsonge

Joomla/Sniffs/ControlStructures/ControlStructuresBracketsSniff.php

@@ -173,18 +173,50 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
 				}
 			}
 
-			$nested = 0;
+			$baseLevel = $tokens[$stackPtr]['level'];
 
 			/**
 			 * Take into account any nested parenthesis that don't contribute to the level (often required for
 			 * closures and anonymous classes
 			 */
 			if (array_key_exists('nested_parenthesis', $tokens[$stackPtr]) === true)
 			{
-				$nested = count($tokens[$stackPtr]['nested_parenthesis']);
+				$nestedStructures = $tokens[$stackPtr]['nested_parenthesis'];
+				$nestedCount = 0;
+
+				foreach ($nestedStructures as $start => $end)
+				{
+					/**
+					 * Crude way of checking for a chained method which requires an extra indent. We navigate to the open
+					 * parenthesis of the nested structure. The element before that is the function name. Before that we
+					 * check for an operator (->) and a whitespace before it (which makes it a chained method on a new line)
+					 * TODO: Is there a better way to check for a chained method? This feels very dirty!
+					 */
+					if ($tokens[$start - 2]['type'] === 'T_OBJECT_OPERATOR' && $tokens[$start - 3]['type'] === 'T_WHITESPACE')
+					{
+						/**
+						 * If we have an anonymous function/class on the same line as our chained method then we
+						 * balance out so only increase the count by 1. Else by 2.
+						 */
+						if ($tokens[$start + 1]['type'] === 'T_CLOSURE' || $tokens[$start + 1]['type'] === 'T_ANON_CLASS')
+						{
+							$nestedCount++;
+						}
+						else
+						{
+							$nestedCount += 2;
+						}
+					}
+					else
+					{
+						$nestedCount++;
+					}
+				}
+
+				$baseLevel += $nestedCount;
 			}
 
-			$expected = ($tokens[$stackPtr]['level'] + $nested) * $this->indent;
+			$expected = $baseLevel * $this->indent;
 
 			// We need to divide by 4 here since there is a space vs tab intent in the check vs token
 			$expected /= $this->indent;

Joomla/Tests/ControlStructures/ControlStructuresBracketsUnitTest.inc

@@ -248,3 +248,61 @@ $this->attachRule(
 		}
 	}
 );
+
+class Application
+{
+	public function doThings($container)
+	{
+		$container->alias(MyClass::class, 'MyClass')
+			->share(
+				'MyClass',
+				function (Container $container) {
+					doThing();
+
+					if ($thing)
+						{
+						bar();
+					}
+
+					return foo();
+				},
+				true
+			);
+	}
+}
+
+class Application
+{
+	public function doThings($container)
+	{
+		$container->share(
+			'MyClass',
+			function (Container $container) {
+				doThing();
+
+				if ($thing)
+		{
+					bar();
+				}
+				else {
+					something();
+
+					if (somethingelse())
+		{
+						bar();
+					}
+
+					anotherFunction()
+						->withNested(function() {
+							if ($foo){
+								bar();
+							}
+						});
+
+					return foo();
+				}
+			},
+			true
+		);
+	}
+}

Joomla/Tests/ControlStructures/ControlStructuresBracketsUnitTest.inc.fixed

@@ -247,3 +247,63 @@ $this->attachRule(
 		}
 	}
 );
+
+class Application
+{
+	public function doThings($container)
+	{
+		$container->alias(MyClass::class, 'MyClass')
+			->share(
+				'MyClass',
+				function (Container $container) {
+					doThing();
+
+					if ($thing)
+					{
+						bar();
+					}
+
+					return foo();
+				},
+				true
+			);
+	}
+}
+
+class Application
+{
+	public function doThings($container)
+	{
+		$container->share(
+			'MyClass',
+			function (Container $container) {
+				doThing();
+
+				if ($thing)
+				{
+					bar();
+				}
+				else
+				{
+					something();
+
+					if (somethingelse())
+					{
+						bar();
+					}
+
+					anotherFunction()
+						->withNested(function() {
+							if ($foo)
+							{
+								bar();
+							}
+						});
+
+					return foo();
+				}
+			},
+			true
+		);
+	}
+}

Joomla/Tests/ControlStructures/ControlStructuresBracketsUnitTest.php

@@ -46,6 +46,11 @@ public function getErrorList()
 				210 => 1,
 				226 => 1,
 				235 => 1,
+				263 => 1,
+				284 => 1,
+				287 => 1,
+				291 => 1,
+				297 => 1,
 			   );
 	}