Skip to content

Commit 8cdc122

Browse files
committed
PHP 8.1 | PSR2/PropertyDeclaration: add check for visibility before readonly keyword
The `PSR2.Classes.PropertyDeclaration` sniff includes a check to verify that the `static` keyword is _after_ the visibility in a property declaration. PHP 8.1 introduced the `readonly` keyword for properties. [PSR PER 2.0.0](https://www.php-fig.org/per/coding-style/#46-modifier-keywords) dictates that visibility is declared before the `readonly` keyword. All code samples in both the RFC as well as the PHP manual also use the `visibility - readonly` modifier keyword order, so it is likely that this will become the standard modifier keyword order. A search for PHP projects which have started to use the `readonly` keyword, also shows these predominantly use the `visibility - readonly` modifier keyword order. With that in mind, I'm proposing to add a check for this order to the `PSR2.Classes.PropertyDeclaration` sniff - this being the only PHPCS native sniff which checks the modifier keyword order for properties. As this sniff is included in PSR2 and PSR12, the new check will automatically apply the PSR-PER property modifier keyword order rules to PSR2/PSR12. Includes unit tests. Ref: * https://wiki.php.net/rfc/readonly_properties_v2 * https://www.php.net/manual/en/language.oop5.properties.php#language.oop5.properties.readonly-properties * https://sourcegraph.com/search?q=context:global+%5Cs%28public%7Cprotected%7Cprivate%29%5Cs%2Breadonly%5Cs%2B+lang:php+-file:%28%5E%7C/%29%28vendor%7Ctests%3F%29/+fork:no+archived:no&patternType=regexp (searching `visibility - readonly` order - > 500 results) * https://sourcegraph.com/search?q=context:global+%5Csreadonly%5Cs%2B%28public%7Cprotected%7Cprivate%29%5Cs%2B+lang:php+-file:%28%5E%7C/%29%28vendor%7Ctests%3F%29/+fork:no+archived:no&patternType=regexp (searching `readonly - visibility` order - 41 (non false positive) results)
1 parent c85ac6a commit 8cdc122

File tree

4 files changed

+64
-14
lines changed

4 files changed

+64
-14
lines changed

src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -118,30 +118,70 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
118118
$phpcsFile->addError($error, $stackPtr, 'ScopeMissing', $data);
119119
}
120120

121+
/*
122+
* Note: per PSR-PER section 4.6, the order should be:
123+
* - Inheritance modifier: `abstract` or `final`.
124+
* - Visibility modifier: `public`, `protected`, or `private`.
125+
* - Scope modifier: `static`.
126+
* - Mutation modifier: `readonly`.
127+
* - Type declaration.
128+
* - Name.
129+
*
130+
* Ref: https://www.php-fig.org/per/coding-style/#46-modifier-keywords
131+
*
132+
* At this time (PHP 8.2), inheritance modifiers cannot be applied to properties and
133+
* the `static` and `readonly` modifiers are mutually exclusive and cannot be used together.
134+
*
135+
* Based on that, the below modifier keyword order checks are sufficient (for now).
136+
*/
137+
121138
if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_static'] === true) {
122139
$scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
123140
$staticPtr = $phpcsFile->findPrevious(T_STATIC, ($stackPtr - 1));
124-
if ($scopePtr < $staticPtr) {
125-
return;
126-
}
141+
if ($scopePtr > $staticPtr) {
142+
$error = 'The static declaration must come after the visibility declaration';
143+
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'StaticBeforeVisibility');
144+
if ($fix === true) {
145+
$phpcsFile->fixer->beginChangeset();
127146

128-
$error = 'The static declaration must come after the visibility declaration';
129-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'StaticBeforeVisibility');
130-
if ($fix === true) {
131-
$phpcsFile->fixer->beginChangeset();
147+
for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
148+
if ($tokens[$i]['code'] !== T_WHITESPACE) {
149+
break;
150+
}
132151

133-
for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
134-
if ($tokens[$i]['code'] !== T_WHITESPACE) {
135-
break;
152+
$phpcsFile->fixer->replaceToken($i, '');
136153
}
137154

138-
$phpcsFile->fixer->replaceToken($i, '');
155+
$phpcsFile->fixer->replaceToken($scopePtr, '');
156+
$phpcsFile->fixer->addContentBefore($staticPtr, $propertyInfo['scope'].' ');
157+
158+
$phpcsFile->fixer->endChangeset();
139159
}
160+
}
161+
}//end if
162+
163+
if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_readonly'] === true) {
164+
$scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
165+
$readonlyPtr = $phpcsFile->findPrevious(T_READONLY, ($stackPtr - 1));
166+
if ($scopePtr > $readonlyPtr) {
167+
$error = 'The readonly declaration must come after the visibility declaration';
168+
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ReadonlyBeforeVisibility');
169+
if ($fix === true) {
170+
$phpcsFile->fixer->beginChangeset();
171+
172+
for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
173+
if ($tokens[$i]['code'] !== T_WHITESPACE) {
174+
break;
175+
}
176+
177+
$phpcsFile->fixer->replaceToken($i, '');
178+
}
140179

141-
$phpcsFile->fixer->replaceToken($scopePtr, '');
142-
$phpcsFile->fixer->addContentBefore($staticPtr, $propertyInfo['scope'].' ');
180+
$phpcsFile->fixer->replaceToken($scopePtr, '');
181+
$phpcsFile->fixer->addContentBefore($readonlyPtr, $propertyInfo['scope'].' ');
143182

144-
$phpcsFile->fixer->endChangeset();
183+
$phpcsFile->fixer->endChangeset();
184+
}
145185
}
146186
}//end if
147187

src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,8 @@ class ReadOnlyProp {
8080
protected readonly ?string $foo;
8181

8282
readonly array $foo;
83+
84+
readonly public int $wrongOrder1;
85+
86+
readonly protected ?string $wrongOrder2;
8387
}

src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,8 @@ class ReadOnlyProp {
7777
protected readonly ?string $foo;
7878

7979
readonly array $foo;
80+
81+
public readonly int $wrongOrder1;
82+
83+
protected readonly ?string $wrongOrder2;
8084
}

src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public function getErrorList()
4949
76 => 1,
5050
80 => 1,
5151
82 => 1,
52+
84 => 1,
53+
86 => 1,
5254
];
5355

5456
}//end getErrorList()

0 commit comments

Comments
 (0)