Skip to content

PHP 8.4 | PSR2/PropertyDeclaration: add support for final properties #950

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
merged 1 commit into from
Apr 14, 2025
Merged

PHP 8.4 | PSR2/PropertyDeclaration: add support for final properties #950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 27 additions & 2 deletions src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
$find[] = T_VARIABLE;
$find[] = T_VAR;
$find[] = T_READONLY;
$find[] = T_FINAL;
$find[] = T_SEMICOLON;
$find[] = T_OPEN_CURLY_BRACKET;

Expand Down Expand Up @@ -130,12 +131,36 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
*
* Ref: https://www.php-fig.org/per/coding-style/#46-modifier-keywords
*
* At this time (PHP 8.2), inheritance modifiers cannot be applied to properties and
* the `static` and `readonly` modifiers are mutually exclusive and cannot be used together.
* 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));
$finalPtr = $phpcsFile->findPrevious(T_FINAL, ($stackPtr - 1));
if ($finalPtr > $scopePtr) {
$error = 'The final declaration must come before the visibility declaration';
Copy link
Contributor

Choose a reason for hiding this comment

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

PSR2 predates final properties, but technically https://www.php-fig.org/psr/psr-2/ just says that final needs to come before visibility and it doesn't limit that to methods, so looks great

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly and yes, I did check PSR-PER before adding this new check 😉

$fix = $phpcsFile->addFixableError($error, $stackPtr, 'FinalAfterVisibility');
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();

for ($i = ($finalPtr + 1); $finalPtr < $stackPtr; $i++) {
if ($tokens[$i]['code'] !== T_WHITESPACE) {
break;
}

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

$phpcsFile->fixer->replaceToken($finalPtr, '');
$phpcsFile->fixer->addContentBefore($scopePtr, $tokens[$finalPtr]['content'].' ');

$phpcsFile->fixer->endChangeset();
}
}
}//end if

if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_static'] === true) {
$scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
$staticPtr = $phpcsFile->findPrevious(T_STATIC, ($stackPtr - 1));
Expand Down
11 changes: 11 additions & 0 deletions src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,14 @@ class ReadOnlyProp {

readonly protected ?string $wrongOrder2;
}

class FinalProperties {
final public int $foo,
$bar,
$var = null;

final protected (D|N)|false $foo;
final array $foo;
public FINAL ?int $wrongOrder1;
static protected final ?string $wrongOrder2;
}
11 changes: 11 additions & 0 deletions src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,14 @@ class ReadOnlyProp {

protected readonly ?string $wrongOrder2;
}

class FinalProperties {
final public int $foo,
$bar,
$var = null;

final protected (D|N)|false $foo;
final array $foo;
FINAL public ?int $wrongOrder1;
final protected static ?string $wrongOrder2;
}
5 changes: 5 additions & 0 deletions src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ public function getErrorList()
82 => 1,
84 => 1,
86 => 1,
90 => 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

So something I noticed generally with the tests in phpcs is that the actual error messages used aren't tested as much - you can see how I addressed tests for sniffs at https://github.com/DanielEScherzer/common-phpcs, I test against the output of the report instead of just the line numbers. Would you be interested in me porting that to phpcs so that we can test error messages too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice! I am definitely very aware of that problem and have some ideas about this too, but hadn't gotten round to actioning it yet (priorities, priorities).

If you'd be open to it, I'd be very happy to brainstorm the various options (and consequences of those!) in a call some time after next week. Feel free to ping me on Mastodon or X to set up a day/time.

94 => 1,
95 => 1,
96 => 1,
97 => 2,
];

}//end getErrorList()
Expand Down