diff --git a/src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php b/src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php index 70828bb991..8613f34a97 100644 --- a/src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php +++ b/src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php @@ -114,30 +114,64 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) }//end if }//end if - if ($propertyInfo['scope_specified'] === false) { + if ($propertyInfo['scope_specified'] === false && $propertyInfo['set_scope'] === false) { $error = 'Visibility must be declared on property "%s"'; $data = [$tokens[$stackPtr]['content']]; $phpcsFile->addError($error, $stackPtr, 'ScopeMissing', $data); } /* - * Note: per PSR-PER section 4.6, the order should be: + * Note: per PSR-PER section 4.6 v 2.1/3.0, the order should be: * - Inheritance modifier: `abstract` or `final`. * - Visibility modifier: `public`, `protected`, or `private`. + * - Set-visibility modifier: `public(set)`, `protected(set)`, or `private(set)` * - Scope modifier: `static`. * - Mutation modifier: `readonly`. * - Type declaration. * - Name. * - * Ref: https://www.php-fig.org/per/coding-style/#46-modifier-keywords + * Ref: + * - https://www.php-fig.org/per/coding-style/#46-modifier-keywords + * - https://github.com/php-fig/per-coding-style/pull/99 * * The `static` and `readonly` modifiers are mutually exclusive and cannot be used together. * * Based on that, the below modifier keyword order checks are sufficient (for now). */ - if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_final'] === true) { - $scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1)); + $hasVisibilityModifier = ($propertyInfo['scope_specified'] === true || $propertyInfo['set_scope'] !== false); + $lastVisibilityModifier = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1)); + $firstVisibilityModifier = $lastVisibilityModifier; + + if ($propertyInfo['scope_specified'] === true && $propertyInfo['set_scope'] !== false) { + $scopePtr = $phpcsFile->findPrevious([T_PUBLIC, T_PROTECTED, T_PRIVATE], ($stackPtr - 1)); + $setScopePtr = $phpcsFile->findPrevious([T_PUBLIC_SET, T_PROTECTED_SET, T_PRIVATE_SET], ($stackPtr - 1)); + if ($scopePtr > $setScopePtr) { + $error = 'The "read"-visibility must come before the "write"-visibility'; + $fix = $phpcsFile->addFixableError($error, $stackPtr, 'AvizKeywordOrder'); + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + + for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) { + if ($tokens[$i]['code'] !== T_WHITESPACE) { + break; + } + + $phpcsFile->fixer->replaceToken($i, ''); + } + + $phpcsFile->fixer->replaceToken($scopePtr, ''); + $phpcsFile->fixer->addContentBefore($setScopePtr, $tokens[$scopePtr]['content'].' '); + + $phpcsFile->fixer->endChangeset(); + } + } + + $firstVisibilityModifier = min($scopePtr, $setScopePtr); + }//end if + + if ($hasVisibilityModifier === true && $propertyInfo['is_final'] === true) { + $scopePtr = $firstVisibilityModifier; $finalPtr = $phpcsFile->findPrevious(T_FINAL, ($stackPtr - 1)); if ($finalPtr > $scopePtr) { $error = 'The final declaration must come before the visibility declaration'; @@ -161,8 +195,8 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) } }//end if - if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_static'] === true) { - $scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1)); + if ($hasVisibilityModifier === true && $propertyInfo['is_static'] === true) { + $scopePtr = $lastVisibilityModifier; $staticPtr = $phpcsFile->findPrevious(T_STATIC, ($stackPtr - 1)); if ($scopePtr > $staticPtr) { $error = 'The static declaration must come after the visibility declaration'; @@ -170,7 +204,7 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) if ($fix === true) { $phpcsFile->fixer->beginChangeset(); - for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) { + for ($i = ($staticPtr + 1); $staticPtr < $stackPtr; $i++) { if ($tokens[$i]['code'] !== T_WHITESPACE) { break; } @@ -178,16 +212,16 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) $phpcsFile->fixer->replaceToken($i, ''); } - $phpcsFile->fixer->replaceToken($scopePtr, ''); - $phpcsFile->fixer->addContentBefore($staticPtr, $propertyInfo['scope'].' '); + $phpcsFile->fixer->replaceToken($staticPtr, ''); + $phpcsFile->fixer->addContent($scopePtr, ' '.$tokens[$staticPtr]['content']); $phpcsFile->fixer->endChangeset(); } } }//end if - if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_readonly'] === true) { - $scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1)); + if ($hasVisibilityModifier === true && $propertyInfo['is_readonly'] === true) { + $scopePtr = $lastVisibilityModifier; $readonlyPtr = $phpcsFile->findPrevious(T_READONLY, ($stackPtr - 1)); if ($scopePtr > $readonlyPtr) { $error = 'The readonly declaration must come after the visibility declaration'; @@ -195,7 +229,7 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) if ($fix === true) { $phpcsFile->fixer->beginChangeset(); - for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) { + for ($i = ($readonlyPtr + 1); $readonlyPtr < $stackPtr; $i++) { if ($tokens[$i]['code'] !== T_WHITESPACE) { break; } @@ -203,8 +237,8 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) $phpcsFile->fixer->replaceToken($i, ''); } - $phpcsFile->fixer->replaceToken($scopePtr, ''); - $phpcsFile->fixer->addContentBefore($readonlyPtr, $propertyInfo['scope'].' '); + $phpcsFile->fixer->replaceToken($readonlyPtr, ''); + $phpcsFile->fixer->addContent($scopePtr, ' '.$tokens[$readonlyPtr]['content']); $phpcsFile->fixer->endChangeset(); } diff --git a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc index 6aa9ffb799..2e1785a204 100644 --- a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc @@ -109,4 +109,20 @@ class AsymmetricVisibility { protected(set) public int $wrongOrder1; private(set) protected ?string $wrongOrder2; + + final protected private(set) static bool $correctOrder; + + private(set) static final protected bool $wrongOrder3; + private(set) static protected final bool $wrongOrder4; + final protected(set) static public bool $wrongOrder5; + static public(set) final public bool $wrongOrder6; + + protected private(set) static final bool $wrongOrder7; + protected final private(set) static bool $wrongOrder8; + static public final protected(set) bool $wrongOrder9; + public static public(set) final bool $wrongOrder10; + + private(set) static final bool $wrongOrder11; + final static protected(set) bool $wrongOrder12; + static public(set) final bool $wrongOrder13; } diff --git a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed index efb6a0da00..b398e2fe78 100644 --- a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed +++ b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed @@ -37,8 +37,8 @@ class ABC { private static $propC; public static $propD; protected static - $propE; - private static /*comment*/ $propF; + $propE; + private static /*comment*/ $propF; } class MyClass @@ -103,7 +103,23 @@ class AsymmetricVisibility { protected(set) array $unfixed; - protected(set) public int $wrongOrder1; + public protected(set) int $wrongOrder1; - private(set) protected ?string $wrongOrder2; + protected private(set) ?string $wrongOrder2; + + final protected private(set) static bool $correctOrder; + + final protected private(set) static bool $wrongOrder3; + final protected private(set) static bool $wrongOrder4; + final public protected(set) static bool $wrongOrder5; + final public public(set) static bool $wrongOrder6; + + final protected private(set) static bool $wrongOrder7; + final protected private(set) static bool $wrongOrder8; + final public protected(set) static bool $wrongOrder9; + final public public(set) static bool $wrongOrder10; + + final private(set) static bool $wrongOrder11; + final protected(set) static bool $wrongOrder12; + final public(set) static bool $wrongOrder13; } diff --git a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php index 41b3626afb..bce3d9f1df 100644 --- a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php +++ b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php @@ -61,9 +61,21 @@ public function getErrorList() 95 => 1, 96 => 1, 97 => 2, - 101 => 2, + 101 => 1, 105 => 1, - 107 => 1, + 109 => 1, + 111 => 1, + 115 => 3, + 116 => 3, + 117 => 2, + 118 => 3, + 120 => 1, + 121 => 1, + 122 => 2, + 123 => 2, + 125 => 1, + 126 => 1, + 127 => 2, ]; }//end getErrorList()