Skip to content

Commit d9f1879

Browse files
jrfnljrfnl
committed
PSR2/PropertyDeclaration: update for properties with asymmetric visibility [2]
Per the FIG PSR-PER updates (next version), the "set-visibility MUST be _after_ the (read-)visibility". This commit implements this, but as there may be two visibility modifiers, a minor refactor of the code involved in the wrong order checks for the `final` keyword and the `static` and `readonly` keywords is also needed as we can no longer presume that we can use the visibility keyword found with a search backwards to re-position other keywords. Includes plenty of tests. Includes a minor update to pre-existing test expectations (whitespace only), which is related to the refactor of the "fix"-code. As this sniff does not enforce any spacing requirements after the keywords, this difference is neglicable for the purpose of this sniff. Note: in a future iteration, we may want to look at making the code for the "order" checks more efficient, but for now, at least it will work correctly again. Refs: * php-fig/per-coding-style 99 * php-fig/per-coding-style 114
1 parent 051ef9f commit d9f1879

File tree

4 files changed

+97
-18
lines changed

4 files changed

+97
-18
lines changed

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

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -121,23 +121,57 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
121121
}
122122

123123
/*
124-
* Note: per PSR-PER section 4.6, the order should be:
124+
* Note: per PSR-PER section 4.6 v 2.1/3.0, the order should be:
125125
* - Inheritance modifier: `abstract` or `final`.
126126
* - Visibility modifier: `public`, `protected`, or `private`.
127+
* - Set-visibility modifier: `public(set)`, `protected(set)`, or `private(set)`
127128
* - Scope modifier: `static`.
128129
* - Mutation modifier: `readonly`.
129130
* - Type declaration.
130131
* - Name.
131132
*
132-
* Ref: https://www.php-fig.org/per/coding-style/#46-modifier-keywords
133+
* Ref:
134+
* - https://www.php-fig.org/per/coding-style/#46-modifier-keywords
135+
* - https://github.com/php-fig/per-coding-style/pull/99
133136
*
134137
* The `static` and `readonly` modifiers are mutually exclusive and cannot be used together.
135138
*
136139
* Based on that, the below modifier keyword order checks are sufficient (for now).
137140
*/
138141

139-
if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_final'] === true) {
140-
$scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
142+
$hasVisibilityModifier = ($propertyInfo['scope_specified'] === true || $propertyInfo['set_scope'] !== false);
143+
$lastVisibilityModifier = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
144+
$firstVisibilityModifier = $lastVisibilityModifier;
145+
146+
if ($propertyInfo['scope_specified'] === true && $propertyInfo['set_scope'] !== false) {
147+
$scopePtr = $phpcsFile->findPrevious([T_PUBLIC, T_PROTECTED, T_PRIVATE], ($stackPtr - 1));
148+
$setScopePtr = $phpcsFile->findPrevious([T_PUBLIC_SET, T_PROTECTED_SET, T_PRIVATE_SET], ($stackPtr - 1));
149+
if ($scopePtr > $setScopePtr) {
150+
$error = 'The "read"-visibility must come before the "write"-visibility';
151+
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'AvizKeywordOrder');
152+
if ($fix === true) {
153+
$phpcsFile->fixer->beginChangeset();
154+
155+
for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
156+
if ($tokens[$i]['code'] !== T_WHITESPACE) {
157+
break;
158+
}
159+
160+
$phpcsFile->fixer->replaceToken($i, '');
161+
}
162+
163+
$phpcsFile->fixer->replaceToken($scopePtr, '');
164+
$phpcsFile->fixer->addContentBefore($setScopePtr, $tokens[$scopePtr]['content'].' ');
165+
166+
$phpcsFile->fixer->endChangeset();
167+
}
168+
}
169+
170+
$firstVisibilityModifier = min($scopePtr, $setScopePtr);
171+
}//end if
172+
173+
if ($hasVisibilityModifier === true && $propertyInfo['is_final'] === true) {
174+
$scopePtr = $firstVisibilityModifier;
141175
$finalPtr = $phpcsFile->findPrevious(T_FINAL, ($stackPtr - 1));
142176
if ($finalPtr > $scopePtr) {
143177
$error = 'The final declaration must come before the visibility declaration';
@@ -161,50 +195,50 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
161195
}
162196
}//end if
163197

164-
if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_static'] === true) {
165-
$scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
198+
if ($hasVisibilityModifier === true && $propertyInfo['is_static'] === true) {
199+
$scopePtr = $lastVisibilityModifier;
166200
$staticPtr = $phpcsFile->findPrevious(T_STATIC, ($stackPtr - 1));
167201
if ($scopePtr > $staticPtr) {
168202
$error = 'The static declaration must come after the visibility declaration';
169203
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'StaticBeforeVisibility');
170204
if ($fix === true) {
171205
$phpcsFile->fixer->beginChangeset();
172206

173-
for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
207+
for ($i = ($staticPtr + 1); $staticPtr < $stackPtr; $i++) {
174208
if ($tokens[$i]['code'] !== T_WHITESPACE) {
175209
break;
176210
}
177211

178212
$phpcsFile->fixer->replaceToken($i, '');
179213
}
180214

181-
$phpcsFile->fixer->replaceToken($scopePtr, '');
182-
$phpcsFile->fixer->addContentBefore($staticPtr, $propertyInfo['scope'].' ');
215+
$phpcsFile->fixer->replaceToken($staticPtr, '');
216+
$phpcsFile->fixer->addContent($scopePtr, ' '.$tokens[$staticPtr]['content']);
183217

184218
$phpcsFile->fixer->endChangeset();
185219
}
186220
}
187221
}//end if
188222

189-
if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_readonly'] === true) {
190-
$scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
223+
if ($hasVisibilityModifier === true && $propertyInfo['is_readonly'] === true) {
224+
$scopePtr = $lastVisibilityModifier;
191225
$readonlyPtr = $phpcsFile->findPrevious(T_READONLY, ($stackPtr - 1));
192226
if ($scopePtr > $readonlyPtr) {
193227
$error = 'The readonly declaration must come after the visibility declaration';
194228
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ReadonlyBeforeVisibility');
195229
if ($fix === true) {
196230
$phpcsFile->fixer->beginChangeset();
197231

198-
for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
232+
for ($i = ($readonlyPtr + 1); $readonlyPtr < $stackPtr; $i++) {
199233
if ($tokens[$i]['code'] !== T_WHITESPACE) {
200234
break;
201235
}
202236

203237
$phpcsFile->fixer->replaceToken($i, '');
204238
}
205239

206-
$phpcsFile->fixer->replaceToken($scopePtr, '');
207-
$phpcsFile->fixer->addContentBefore($readonlyPtr, $propertyInfo['scope'].' ');
240+
$phpcsFile->fixer->replaceToken($readonlyPtr, '');
241+
$phpcsFile->fixer->addContent($scopePtr, ' '.$tokens[$readonlyPtr]['content']);
208242

209243
$phpcsFile->fixer->endChangeset();
210244
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,20 @@ class AsymmetricVisibility {
109109
protected(set) public int $wrongOrder1;
110110

111111
private(set) protected ?string $wrongOrder2;
112+
113+
final protected private(set) static bool $correctOrder;
114+
115+
private(set) static final protected bool $wrongOrder3;
116+
private(set) static protected final bool $wrongOrder4;
117+
final protected(set) static public bool $wrongOrder5;
118+
static public(set) final public bool $wrongOrder6;
119+
120+
protected private(set) static final bool $wrongOrder7;
121+
protected final private(set) static bool $wrongOrder8;
122+
static public final protected(set) bool $wrongOrder9;
123+
public static public(set) final bool $wrongOrder10;
124+
125+
private(set) static final bool $wrongOrder11;
126+
final static protected(set) bool $wrongOrder12;
127+
static public(set) final bool $wrongOrder13;
112128
}

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ class ABC {
3737
private static $propC;
3838
public static $propD;
3939
protected static
40-
$propE;
41-
private static /*comment*/ $propF;
40+
$propE;
41+
private static /*comment*/ $propF;
4242
}
4343

4444
class MyClass
@@ -103,7 +103,23 @@ class AsymmetricVisibility {
103103

104104
protected(set) array $unfixed;
105105

106-
protected(set) public int $wrongOrder1;
106+
public protected(set) int $wrongOrder1;
107107

108-
private(set) protected ?string $wrongOrder2;
108+
protected private(set) ?string $wrongOrder2;
109+
110+
final protected private(set) static bool $correctOrder;
111+
112+
final protected private(set) static bool $wrongOrder3;
113+
final protected private(set) static bool $wrongOrder4;
114+
final public protected(set) static bool $wrongOrder5;
115+
final public public(set) static bool $wrongOrder6;
116+
117+
final protected private(set) static bool $wrongOrder7;
118+
final protected private(set) static bool $wrongOrder8;
119+
final public protected(set) static bool $wrongOrder9;
120+
final public public(set) static bool $wrongOrder10;
121+
122+
final private(set) static bool $wrongOrder11;
123+
final protected(set) static bool $wrongOrder12;
124+
final public(set) static bool $wrongOrder13;
109125
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,19 @@ public function getErrorList()
6363
97 => 2,
6464
101 => 1,
6565
105 => 1,
66+
109 => 1,
67+
111 => 1,
68+
115 => 3,
69+
116 => 3,
70+
117 => 2,
71+
118 => 3,
72+
120 => 1,
73+
121 => 1,
74+
122 => 2,
75+
123 => 2,
76+
125 => 1,
77+
126 => 1,
78+
127 => 2,
6679
];
6780

6881
}//end getErrorList()

0 commit comments

Comments
 (0)