Skip to content

PSR2/PropertyDeclaration: update for properties with asymmetric visibility #1119

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.

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
64 changes: 49 additions & 15 deletions src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -161,50 +195,50 @@ 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';
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'StaticBeforeVisibility');
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;
}

$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';
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ReadonlyBeforeVisibility');
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;
}

$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();
}
Expand Down
16 changes: 16 additions & 0 deletions src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
16 changes: 14 additions & 2 deletions src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down