Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PEAR/FunctionDeclaration: examine arrow function declarations + fix fixer conflict #3661

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

PEAR/FunctionDeclaration: examine arrow function declarations + fix fixer conflict #3661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eced441
PEAR/FunctionDeclaration: examine arrow function declarations
jrfnl Sep 9, 2022
db548df
Squiz/MultiLineFunctionDeclaration: add tests for arrow function decl…
jrfnl Sep 11, 2022
d479811
PEAR/FunctionDeclaration: prevent fixer conflict
jrfnl Sep 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 86 additions & 50 deletions src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function register()
return [
T_FUNCTION,
T_CLOSURE,
T_FN,
];

}//end register()
Expand Down Expand Up @@ -75,7 +76,9 @@ public function process(File $phpcsFile, $stackPtr)
$openBracket = $tokens[$stackPtr]['parenthesis_opener'];
$closeBracket = $tokens[$stackPtr]['parenthesis_closer'];

if (strtolower($tokens[$stackPtr]['content']) === 'function') {
if (strtolower($tokens[$stackPtr]['content']) === 'function'
|| strtolower($tokens[$stackPtr]['content']) === 'fn'
) {
// Must be one space after the FUNCTION keyword.
if ($tokens[($stackPtr + 1)]['content'] === $phpcsFile->eolChar) {
$spaces = 'newline';
Expand All @@ -86,9 +89,13 @@ public function process(File $phpcsFile, $stackPtr)
}

if ($spaces !== 1) {
$error = 'Expected 1 space after FUNCTION keyword; %s found';
$data = [$spaces];
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpaceAfterFunction', $data);
$error = 'Expected 1 space after %s keyword; %s found';
$data = [
strtoupper($tokens[$stackPtr]['content']),
$spaces,
];

$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpaceAfterFunction', $data);
if ($fix === true) {
if ($spaces === 0) {
$phpcsFile->fixer->addContent($stackPtr, ' ');
Expand All @@ -99,9 +106,9 @@ public function process(File $phpcsFile, $stackPtr)
}
}//end if

// Must be no space before the opening parenthesis. For closures, this is
// enforced by the previous check because there is no content between the keywords
// and the opening parenthesis.
// Must be no space before the opening parenthesis. For closures and arrow functions,
// this is enforced by the previous check because there is no content between
// the keywords and the opening parenthesis.
// Unfinished closures are tokenized as T_FUNCTION however, and can be excluded
// by checking for the scope_opener.
if ($tokens[$stackPtr]['code'] === T_FUNCTION
Expand Down Expand Up @@ -257,6 +264,10 @@ public function isMultiLineDeclaration($phpcsFile, $stackPtr, $openBracket, $tok
*/
public function processSingleLineDeclaration($phpcsFile, $stackPtr, $tokens)
{
if ($tokens[$stackPtr]['code'] === T_FN) {
return;
}

if ($tokens[$stackPtr]['code'] === T_CLOSURE) {
$sniff = new OpeningFunctionBraceKernighanRitchieSniff();
} else {
Expand Down Expand Up @@ -297,61 +308,86 @@ public function processMultiLineDeclaration($phpcsFile, $stackPtr, $tokens)
return;
}

// The opening brace needs to be one space away from the closing parenthesis.
// The opening brace needs to be on the same line as the closing parenthesis.
// There should only be one space between the closing parenthesis - or the end of the
// return type - and the opening brace.
$opener = $tokens[$stackPtr]['scope_opener'];
if ($tokens[$opener]['line'] !== $tokens[$closeBracket]['line']) {
$error = 'The closing parenthesis and the opening brace of a multi-line function declaration must be on the same line';
$fix = $phpcsFile->addFixableError($error, $opener, 'NewlineBeforeOpenBrace');
if ($fix === true) {
$prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), $closeBracket, true);
$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContent($prev, ' {');
$error = 'The closing parenthesis and the %s of a multi-line function declaration must be on the same line';
$code = 'NewlineBeforeOpenBrace';
$data = ['opening brace'];

// If the opener is on a line by itself, removing it will create
// an empty line, so just remove the entire line instead.
$prev = $phpcsFile->findPrevious(T_WHITESPACE, ($opener - 1), $closeBracket, true);
$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);
if ($tokens[$stackPtr]['code'] === T_FN) {
$code = 'NewlineBeforeArrow';
$data = ['arrow'];
}

if ($tokens[$prev]['line'] < $tokens[$opener]['line']
&& $tokens[$next]['line'] > $tokens[$opener]['line']
) {
// Clear the whole line.
for ($i = ($prev + 1); $i < $next; $i++) {
if ($tokens[$i]['line'] === $tokens[$opener]['line']) {
$phpcsFile->fixer->replaceToken($i, '');
$prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), $closeBracket, true);
if ($tokens[$prev]['line'] === $tokens[$opener]['line']) {
// End of the return type is not on the same line as the close parenthesis.
$phpcsFile->addError($error, $opener, $code, $data);
} else {
$fix = $phpcsFile->addFixableError($error, $opener, $code, $data);
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContent($prev, ' '.$tokens[$opener]['content']);

// If the opener is on a line by itself, removing it will create
// an empty line, so just remove the entire line instead.
$prev = $phpcsFile->findPrevious(T_WHITESPACE, ($opener - 1), $closeBracket, true);
$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);

if ($tokens[$prev]['line'] < $tokens[$opener]['line']
&& $tokens[$next]['line'] > $tokens[$opener]['line']
) {
// Clear the whole line.
for ($i = ($prev + 1); $i < $next; $i++) {
if ($tokens[$i]['line'] === $tokens[$opener]['line']) {
$phpcsFile->fixer->replaceToken($i, '');
}
}
} else {
// Just remove the opener.
$phpcsFile->fixer->replaceToken($opener, '');
if ($tokens[$next]['line'] === $tokens[$opener]['line']) {
$phpcsFile->fixer->replaceToken(($opener + 1), '');
}
}
} else {
// Just remove the opener.
$phpcsFile->fixer->replaceToken($opener, '');
if ($tokens[$next]['line'] === $tokens[$opener]['line']) {
$phpcsFile->fixer->replaceToken(($opener + 1), '');
}
}

$phpcsFile->fixer->endChangeset();
$phpcsFile->fixer->endChangeset();
}//end if

return;
}//end if
}//end if

$prev = $tokens[($opener - 1)];
if ($prev['code'] !== T_WHITESPACE) {
$length = 0;
} else {
$prev = $tokens[($opener - 1)];
if ($prev['code'] !== T_WHITESPACE) {
$length = 0;
} else {
$length = strlen($prev['content']);
$length = strlen($prev['content']);
}

if ($length !== 1) {
$error = 'There must be a single space before the %s of a multi-line function declaration; found %s spaces';
$code = 'SpaceBeforeOpenBrace';
$data = ['opening brace'];

if ($tokens[$stackPtr]['code'] === T_FN) {
$code = 'SpaceBeforeArrow';
$data = ['arrow'];
}

if ($length !== 1) {
$error = 'There must be a single space between the closing parenthesis and the opening brace of a multi-line function declaration; found %s spaces';
$fix = $phpcsFile->addFixableError($error, ($opener - 1), 'SpaceBeforeOpenBrace', [$length]);
if ($fix === true) {
if ($length === 0) {
$phpcsFile->fixer->addContentBefore($opener, ' ');
} else {
$phpcsFile->fixer->replaceToken(($opener - 1), ' ');
}
}
$data[] = $length;

return;
}//end if
$fix = $phpcsFile->addFixableError($error, ($opener - 1), $code, $data);
if ($fix === true) {
if ($length === 0) {
$phpcsFile->fixer->addContentBefore($opener, ' ');
} else {
$phpcsFile->fixer->replaceToken(($opener - 1), ' ');
}
}
}//end if

}//end processMultiLineDeclaration()
Expand Down
35 changes: 35 additions & 0 deletions src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,3 +465,38 @@ new ExceptionMessage(),
) {
}
}

$arrowNoArgs = fn () => $retrievedfromscope;

$arrowSingleLineArgs = fn (Type $param1, int $param2, string $param3): \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1,
$longerVar2,
&...$muchLongerVar3
) => $longVar1;

$arrowNoArgs = fn( )
=> $retrievedfromscope;

$arrowSingleLineArgs = fn( Type $param1 , int $param2, string $param3
) : \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1, $longerVar2,

& ... $muchLongerVar3) => $longVar1;

// Prevent fixer conflict with itself.
function foo(
$param1,
)
: \SomeClass
{
}

function foo(
$param1,
$param2
) : // comment.
\Package\Sub\SomeClass {}
34 changes: 34 additions & 0 deletions src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,37 @@ new ExceptionMessage(),
) {
}
}

$arrowNoArgs = fn () => $retrievedfromscope;

$arrowSingleLineArgs = fn (Type $param1, int $param2, string $param3): \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1,
$longerVar2,
&...$muchLongerVar3
) => $longVar1;

$arrowNoArgs = fn ( )
=> $retrievedfromscope;

$arrowSingleLineArgs = fn ( Type $param1 , int $param2, string $param3
) : \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1, $longerVar2,
& ... $muchLongerVar3
) => $longVar1;

// Prevent fixer conflict with itself.
function foo(
$param1,
)
: \SomeClass {
}

function foo(
$param1,
$param2
) : // comment.
\Package\Sub\SomeClass {}
7 changes: 7 additions & 0 deletions src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ public function getErrorList($testFile='FunctionDeclarationUnitTest.inc')
371 => 1,
402 => 1,
406 => 1,
479 => 1,
482 => 1,
483 => 2,
487 => 1,
488 => 2,
495 => 1,
502 => 2,
];
} else {
$errors = [
Expand Down
21 changes: 21 additions & 0 deletions src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,24 @@ new ExceptionMessage(),
) {
}
}

$arrowNoArgs = fn () => $retrievedfromscope;

$arrowSingleLineArgs = fn (Type $param1, int $param2, string $param3): \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1,
$longerVar2,
&...$muchLongerVar3
) => $longVar1;

$arrowNoArgs = fn( )
=> $retrievedfromscope;

$arrowSingleLineArgs = fn( Type $param1 , int $param2, string $param3
) : \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1, $longerVar2,

& ... $muchLongerVar3) => $longVar1;
21 changes: 21 additions & 0 deletions src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,3 +314,24 @@ new ExceptionMessage(),
) {
}
}

$arrowNoArgs = fn () => $retrievedfromscope;

$arrowSingleLineArgs = fn (Type $param1, int $param2, string $param3): \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1,
$longerVar2,
&...$muchLongerVar3
) => $longVar1;

$arrowNoArgs = fn ( )
=> $retrievedfromscope;

$arrowSingleLineArgs = fn ( Type $param1 , int $param2, string $param3) : \ReturnType => $retrievedfromscope;

$arrowMultiLineArgs = fn (
$longVar1,
$longerVar2,
& ... $muchLongerVar3
) => $longVar1;
6 changes: 6 additions & 0 deletions src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ public function getErrorList($testFile='MultiLineFunctionDeclarationUnitTest.inc
252 => 1,
253 => 1,
254 => 1,
316 => 1,
319 => 1,
320 => 1,
323 => 1,
324 => 1,
325 => 2,
];
} else {
$errors = [
Expand Down