Skip to content

Allow array unpacking in ArrayDeclaration multiline Sniff #3843

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

Merged
jrfnl merged 10 commits into from
Jul 11, 2023
Merged

Allow array unpacking in ArrayDeclaration multiline Sniff #3843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
dbc9f4f
Allow array unpacking in ArrayDeclaration multiline Sniff
edorian Jun 20, 2023
922c6c3
Test the absence of NoKeySpecified error for array unpacking
edorian Jun 20, 2023
bd9d7b9
Test cases to also cover 'KeySpecified' errors
edorian Jun 21, 2023
2e2d7d2
Remove inline code example
edorian Jun 21, 2023
3d43052
Ignoring array unpacking for array shape tracking
edorian Jun 21, 2023
28b6dc8
Fix spacing
edorian Jun 21, 2023
c2bd659
Improve description
edorian Jun 21, 2023
0d7b3a0
Esnure errors still occur in mixed arrays
edorian Jun 21, 2023
088c922
Ensure errors still occur in mixed arrays
edorian Jun 21, 2023
41f6803
Adapt to suggested comment
edorian Jun 21, 2023
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
23 changes: 18 additions & 5 deletions src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,13 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array
}

if ($keyUsed === true && $tokens[$lastToken]['code'] === T_COMMA) {
$error = 'No key specified for array entry; first entry specifies key';
$phpcsFile->addError($error, $nextToken, 'NoKeySpecified');
return;
$nextToken = $phpcsFile->findNext(Tokens::$emptyTokens, ($lastToken + 1), null, true);
// Allow for PHP 7.4+ array unpacking within an array declaration.
if ($tokens[$nextToken]['code'] !== T_ELLIPSIS) {
$error = 'No key specified for array entry; first entry specifies key';
$phpcsFile->addError($error, $nextToken, 'NoKeySpecified');
return;
}
}

if ($keyUsed === false) {
Expand Down Expand Up @@ -470,8 +474,17 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array
true
);

$indices[] = ['value' => $valueContent];
$singleUsed = true;
$indices[] = ['value' => $valueContent];
$usesArrayUnpacking = $phpcsFile->findPrevious(
Tokens::$emptyTokens,
($nextToken - 2),
null,
true
);
if ($tokens[$usesArrayUnpacking]['code'] !== T_ELLIPSIS) {
// Don't decide if an array is key => value indexed or not when PHP 7.4+ array unpacking is used.
$singleUsed = true;
}
}//end if

$lastToken = $nextToken;
Expand Down
47 changes: 45 additions & 2 deletions src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,14 +475,57 @@ yield array(
static fn () : string => '',
);

$foo = [
$foo = array(
'foo' => match ($anything) {
'foo' => 'bar',
default => null,
},
];
);

// Intentional syntax error.
$a = array(
'a' =>
);

// Safeguard correct errors for key/no key when PHP 7.4+ array unpacking is encountered.
$x = array(
...$a,
'foo' => 'bar',
);

$x = array(
'foo' => 'bar',
...$a,
);

$x = array(
'foo' => 'bar',
...$a,
'baz' => 'bar',
);

$x = array(
...$a,
'foo' => 'bar', // OK.
'bar', // NoKeySpecified Error (based on second entry).
);

$x = array(
...$a,
'bar', // OK.
'foo' => 'bar', // KeySpecified Error (based on second entry).
);

$x = array(
'foo' => 'bar',
...$a,
'baz' => 'bar',
'bar', // NoKeySpecified Error (based on first entry).
);

$x = array(
'bar',
...$a,
'bar',
'baz' => 'bar', // KeySpecified (based on first entry).
);
47 changes: 45 additions & 2 deletions src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,14 +511,57 @@ yield array(
static fn () : string => '',
);

$foo = [
$foo = array(
'foo' => match ($anything) {
'foo' => 'bar',
default => null,
},
];
);

// Intentional syntax error.
$a = array(
'a' =>
);

// Safeguard correct errors for key/no key when PHP 7.4+ array unpacking is encountered.
$x = array(
...$a,
'foo' => 'bar',
);

$x = array(
'foo' => 'bar',
...$a,
);

$x = array(
'foo' => 'bar',
...$a,
'baz' => 'bar',
);

$x = array(
...$a,
'foo' => 'bar', // OK.
'bar', // NoKeySpecified Error (based on second entry).
);

$x = array(
...$a,
'bar', // OK.
'foo' => 'bar', // KeySpecified Error (based on second entry).
);

$x = array(
'foo' => 'bar',
...$a,
'baz' => 'bar',
'bar', // NoKeySpecified Error (based on first entry).
);

$x = array(
'bar',
...$a,
'bar',
'baz' => 'bar', // KeySpecified (based on first entry).
);
50 changes: 50 additions & 0 deletions src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,57 @@ yield [
static fn () : string => '',
];

$foo = [
'foo' => match ($anything) {
'foo' => 'bar',
default => null,
},
];

Copy link
Contributor Author

@edorian edorian Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is aligning 1.inc and 2.inc

// Intentional syntax error.
$a = [
'a' =>
];

// Safeguard correct errors for key/no key when PHP 7.4+ array unpacking is encountered.
$x = [
...$a,
'foo' => 'bar',
];

$x = [
'foo' => 'bar',
...$a,
];

$x = [
'foo' => 'bar',
...$a,
'baz' => 'bar',
];

$x = [
...$a,
'foo' => 'bar', // OK.
'bar', // NoKeySpecified Error (based on second entry).
];

$x = [
...$a,
'bar', // OK.
'foo' => 'bar', // KeySpecified Error (based on second entry).
];

$x = [
'foo' => 'bar',
...$a,
'baz' => 'bar',
'bar', // NoKeySpecified Error (based on first entry).
];

$x = [
'bar',
...$a,
'bar',
'baz' => 'bar', // KeySpecified (based on first entry).
];
50 changes: 50 additions & 0 deletions src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,57 @@ yield [
static fn () : string => '',
];

$foo = [
'foo' => match ($anything) {
'foo' => 'bar',
default => null,
},
];

// Intentional syntax error.
$a = [
'a' =>
];

// Safeguard correct errors for key/no key when PHP 7.4+ array unpacking is encountered.
$x = [
...$a,
'foo' => 'bar',
];

$x = [
'foo' => 'bar',
...$a,
];

$x = [
'foo' => 'bar',
...$a,
'baz' => 'bar',
];

$x = [
...$a,
'foo' => 'bar', // OK.
'bar', // NoKeySpecified Error (based on second entry).
];

$x = [
...$a,
'bar', // OK.
'foo' => 'bar', // KeySpecified Error (based on second entry).
];

$x = [
'foo' => 'bar',
...$a,
'baz' => 'bar',
'bar', // NoKeySpecified Error (based on first entry).
];

$x = [
'bar',
...$a,
'bar',
'baz' => 'bar', // KeySpecified (based on first entry).
];
8 changes: 8 additions & 0 deletions src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ public function getErrorList($testFile='')
467 => 1,
471 => 1,
472 => 1,
510 => 1,
516 => 1,
523 => 1,
530 => 1,
];
case 'ArrayDeclarationUnitTest.2.inc':
return [
Expand Down Expand Up @@ -210,6 +214,10 @@ public function getErrorList($testFile='')
456 => 1,
460 => 1,
461 => 1,
499 => 1,
505 => 1,
512 => 1,
519 => 1,
];
default:
return [];
Expand Down