From fae43ae5eba23a49ef053f06f9d6ea4437cf94f6 Mon Sep 17 00:00:00 2001 From: Greg Sherwood Date: Sat, 6 Feb 2016 09:51:35 +1100 Subject: [PATCH] PEAR FunctionCallSignatureSniff (and the Squiz and PSR2 sniffs that use it) now correctly check the first argument (ref bug #698) --- .../Functions/FunctionCallSignatureSniff.php | 134 ++++++++++-------- .../FunctionCallSignatureUnitTest.inc | 7 + .../FunctionCallSignatureUnitTest.inc.fixed | 9 ++ .../FunctionCallSignatureUnitTest.php | 2 + package.xml | 2 + 5 files changed, 94 insertions(+), 60 deletions(-) diff --git a/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionCallSignatureSniff.php b/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionCallSignatureSniff.php index 1c44eeae2d..4c2c03a778 100644 --- a/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionCallSignatureSniff.php +++ b/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionCallSignatureSniff.php @@ -353,11 +353,21 @@ public function processMultiLineCall(PHP_CodeSniffer_File $phpcsFile, $stackPtr, } // Each line between the parenthesis should be indented n spaces. - $lastLine = $tokens[$openBracket]['line']; + $lastLine = ($tokens[$openBracket]['line'] - 1); $argStart = null; $argEnd = null; $inArg = false; - for ($i = ($openBracket + 1); $i < $closeBracket; $i++) { + + // Start processing at the first argument. + $i = $phpcsFile->findNext(T_WHITESPACE, ($openBracket + 1), null, true); + if ($tokens[($i - 1)]['code'] === T_WHITESPACE + && $tokens[($i - 1)]['line'] === $tokens[$i]['line'] + ) { + // Make sure we check the indent. + $i--; + } + + for ($i; $i < $closeBracket; $i++) { if ($i > $argStart && $i < $argEnd) { $inArg = true; } else { @@ -384,76 +394,80 @@ public function processMultiLineCall(PHP_CodeSniffer_File $phpcsFile, $stackPtr, continue; } - // We changed lines, so this should be a whitespace indent token, but first make - // sure it isn't a blank line because we don't need to check indent unless there - // is actually some code to indent. - if ($tokens[$i]['code'] === T_WHITESPACE) { - $nextCode = $phpcsFile->findNext(T_WHITESPACE, ($i + 1), ($closeBracket + 1), true); - if ($tokens[$nextCode]['line'] !== $lastLine) { - if ($inArg === false) { - $error = 'Empty lines are not allowed in multi-line function calls'; - $fix = $phpcsFile->addFixableError($error, $i, 'EmptyLine'); - if ($fix === true) { - $phpcsFile->fixer->replaceToken($i, ''); + if ($tokens[$i]['line'] !== $tokens[$openBracket]['line']) { + // We changed lines, so this should be a whitespace indent token, but first make + // sure it isn't a blank line because we don't need to check indent unless there + // is actually some code to indent. + if ($tokens[$i]['code'] === T_WHITESPACE) { + $nextCode = $phpcsFile->findNext(T_WHITESPACE, ($i + 1), ($closeBracket + 1), true); + if ($tokens[$nextCode]['line'] !== $lastLine) { + if ($inArg === false) { + $error = 'Empty lines are not allowed in multi-line function calls'; + $fix = $phpcsFile->addFixableError($error, $i, 'EmptyLine'); + if ($fix === true) { + $phpcsFile->fixer->replaceToken($i, ''); + } } - } - continue; + continue; + } + } else { + $nextCode = $i; } - } else { - $nextCode = $i; - } - if ($tokens[$nextCode]['line'] === $tokens[$closeBracket]['line']) { - // Closing brace needs to be indented to the same level - // as the function call. - $inArg = false; - $expectedIndent = $functionIndent; - } else { - $expectedIndent = ($functionIndent + $this->indent); - } + if ($tokens[$nextCode]['line'] === $tokens[$closeBracket]['line']) { + // Closing brace needs to be indented to the same level + // as the function call. + $inArg = false; + $expectedIndent = $functionIndent; + } else { + $expectedIndent = ($functionIndent + $this->indent); + } - if ($tokens[$i]['code'] !== T_WHITESPACE - && $tokens[$i]['code'] !== T_DOC_COMMENT_WHITESPACE - ) { - // Just check if it is a multi-line block comment. If so, we can - // calculate the indent from the whitespace before the content. - if ($tokens[$i]['code'] === T_COMMENT - && $tokens[($i - 1)]['code'] === T_COMMENT + if ($tokens[$i]['code'] !== T_WHITESPACE + && $tokens[$i]['code'] !== T_DOC_COMMENT_WHITESPACE ) { - $trimmed = ltrim($tokens[$i]['content']); - $foundIndent = (strlen($tokens[$i]['content']) - strlen($trimmed)); + // Just check if it is a multi-line block comment. If so, we can + // calculate the indent from the whitespace before the content. + if ($tokens[$i]['code'] === T_COMMENT + && $tokens[($i - 1)]['code'] === T_COMMENT + ) { + $trimmed = ltrim($tokens[$i]['content']); + $foundIndent = (strlen($tokens[$i]['content']) - strlen($trimmed)); + } else { + $foundIndent = 0; + } } else { - $foundIndent = 0; + $foundIndent = strlen($tokens[$i]['content']); } - } else { - $foundIndent = strlen($tokens[$i]['content']); - } - if ($foundIndent < $expectedIndent - || ($inArg === false - && $expectedIndent !== $foundIndent) - ) { - $error = 'Multi-line function call not indented correctly; expected %s spaces but found %s'; - $data = array( - $expectedIndent, - $foundIndent, - ); - - $fix = $phpcsFile->addFixableError($error, $i, 'Indent', $data); - if ($fix === true) { - $padding = str_repeat(' ', $expectedIndent); - if ($foundIndent === 0) { - $phpcsFile->fixer->addContentBefore($i, $padding); - } else { - if ($tokens[$i]['code'] === T_COMMENT) { - $comment = $padding.ltrim($tokens[$i]['content']); - $phpcsFile->fixer->replaceToken($i, $comment); + if ($foundIndent < $expectedIndent + || ($inArg === false + && $expectedIndent !== $foundIndent) + ) { + $error = 'Multi-line function call not indented correctly; expected %s spaces but found %s'; + $data = array( + $expectedIndent, + $foundIndent, + ); + + $fix = $phpcsFile->addFixableError($error, $i, 'Indent', $data); + if ($fix === true) { + $padding = str_repeat(' ', $expectedIndent); + if ($foundIndent === 0) { + $phpcsFile->fixer->addContentBefore($i, $padding); } else { - $phpcsFile->fixer->replaceToken($i, $padding); + if ($tokens[$i]['code'] === T_COMMENT) { + $comment = $padding.ltrim($tokens[$i]['content']); + $phpcsFile->fixer->replaceToken($i, $comment); + } else { + $phpcsFile->fixer->replaceToken($i, $padding); + } } } - } + }//end if + } else { + $nextCode = $i; }//end if if ($inArg === false) { diff --git a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc index 3ab982aa01..7a424685fe 100644 --- a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc +++ b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc @@ -296,3 +296,10 @@ array_filter( return $i === 0; } ); + +foo(array( + 'callback' => function () { + $foo = 'foo'; + return; + }, +)); diff --git a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc.fixed b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc.fixed index 2d8e48df22..0851637590 100644 --- a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc.fixed +++ b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.inc.fixed @@ -301,3 +301,12 @@ array_filter( return $i === 0; } ); + +foo( + array( + 'callback' => function () { + $foo = 'foo'; + return; + }, + ) +); diff --git a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.php b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.php index 3ae7dd1b3e..184b4eaf77 100644 --- a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.php +++ b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionCallSignatureUnitTest.php @@ -100,6 +100,8 @@ public function getErrorList($testFile='FunctionCallSignatureUnitTest.inc') 215 => 2, 274 => 1, 275 => 1, + 300 => 1, + 305 => 1, ); }//end getErrorList() diff --git a/package.xml b/package.xml index ef2ad41b2c..9dcfd7e019 100644 --- a/package.xml +++ b/package.xml @@ -56,6 +56,8 @@ http://pear.php.net/dtd/package-2.0.xsd"> - Squiz CSS IndentationSniff no longer assumes the class opening brace is at the end of a line - Squiz FunctionCommentThrowTagSniff now ignores non-docblock comments - Squiz ComparisonOperatorUsageSniff now allows conditions like while(true) + - PEAR FunctionCallSignatureSniff (and the Squiz and PSR2 sniffs that use it) now correctly check the first argument + -- Further fix for bug #698 - Fixed bug #872 : Incorrect detection of blank lines between CSS class names - Fixed bug #879 : Generic InlineControlStructureSniff can create parse error when case/if/elseif/else have mixed brace and braceless definitions - Fixed bug #883 : PSR2 is not checking for blank lines at the start and end of control structures