diff --git a/SlevomatCodingStandard/Sniffs/Classes/ReadonlyClassSniff.php b/SlevomatCodingStandard/Sniffs/Classes/ReadonlyClassSniff.php new file mode 100644 index 000000000..72cc7e85b --- /dev/null +++ b/SlevomatCodingStandard/Sniffs/Classes/ReadonlyClassSniff.php @@ -0,0 +1,288 @@ + + */ + public function register(): array + { + return [T_CLASS]; + } + + public function process(File $phpcsFile, int $classPointer): void + { + $this->enable = SniffSettingsHelper::isEnabledByPhpVersion($this->enable, 80200); + if (!$this->enable) { + return; + } + + $constructorPointer = $this->findConstructorPointer($phpcsFile, $classPointer); + + $promotedProperties = $constructorPointer !== null + ? $this->getPromotedProperties($phpcsFile, $constructorPointer) + : []; + + $classBodyProperties = $this->getClassBodyProperties($phpcsFile, $classPointer, $constructorPointer); + + if (count($promotedProperties) === 0 && count($classBodyProperties) === 0) { + return; + } + + $tokens = $phpcsFile->getTokens(); + if ($this->isReadonlyClass($phpcsFile, $classPointer)) { + foreach ($promotedProperties as $promotedProperty) { + $readonlyModifierPointer = $promotedProperty['readonlyModifierPointer']; + if ($readonlyModifierPointer === null) { + continue; + } + + $fix = $phpcsFile->addFixableError( + sprintf( + 'Promoted property %s in readonly class cannot be declared as readonly.', + $tokens[$promotedProperty['propertyPointer']]['content'], + ), + $readonlyModifierPointer, + self::CODE_PROMOTED_PROPERTY_CANNOT_BE_READONLY_IN_READONLY_CLASS, + ); + if (!$fix) { + continue; + } + + $phpcsFile->fixer->beginChangeset(); + $this->removeReadonlyModifier($phpcsFile, $readonlyModifierPointer); + $phpcsFile->fixer->endChangeset(); + } + + foreach ($classBodyProperties as $classBodyProperty) { + $readonlyModifierPointer = $classBodyProperty['readonlyModifierPointer']; + if ($readonlyModifierPointer === null) { + continue; + } + + $fix = $phpcsFile->addFixableError( + sprintf( + 'Property %s in readonly class cannot be declared as readonly.', + $tokens[$classBodyProperty['propertyPointer']]['content'], + ), + $readonlyModifierPointer, + self::CODE_PROMOTED_PROPERTY_CANNOT_BE_READONLY_IN_READONLY_CLASS, + ); + if (!$fix) { + continue; + } + + $phpcsFile->fixer->beginChangeset(); + $this->removeReadonlyModifier($phpcsFile, $readonlyModifierPointer); + $phpcsFile->fixer->endChangeset(); + } + + return; + } + + foreach ($promotedProperties as $promotedProperty) { + if ($promotedProperty['readonlyModifierPointer'] === null) { + return; + } + } + + foreach ($classBodyProperties as $classBodyProperty) { + if ($classBodyProperty['readonlyModifierPointer'] === null) { + return; + } + } + + $fix = $phpcsFile->addFixableError( + sprintf( + 'Class %s can be marked as readonly.', + ClassHelper::getName($phpcsFile, $classPointer), + ), + $classPointer, + self::CODE_CLASS_CAN_BE_READONLY, + ); + if (!$fix) { + return; + } + + $phpcsFile->fixer->beginChangeset(); + foreach ($promotedProperties as $promotedProperty) { + $this->removeReadonlyModifier($phpcsFile, $promotedProperty['readonlyModifierPointer']); + } + + foreach ($classBodyProperties as $classBodyProperty) { + $this->removeReadonlyModifier($phpcsFile, $classBodyProperty['readonlyModifierPointer']); + } + + $phpcsFile->fixer->addContentBefore($classPointer, 'readonly '); + $phpcsFile->fixer->endChangeset(); + } + + private function findConstructorPointer(File $phpcsFile, int $classPointer): ?int + { + $tokens = $phpcsFile->getTokens(); + $classLevel = $tokens[$classPointer]['level']; + + for ($i = $tokens[$classPointer]['scope_opener'] + 1; $i < $tokens[$classPointer]['scope_closer']; $i++) { + if ($tokens[$i]['code'] !== T_FUNCTION) { + continue; + } + + if ($tokens[$i]['level'] !== $classLevel + 1) { + continue; + } + + if (strtolower(FunctionHelper::getName($phpcsFile, $i)) !== '__construct') { + continue; + } + + return $i; + } + + return null; + } + + private function isReadonlyClass(File $phpcsFile, int $classPointer): bool + { + $tokens = $phpcsFile->getTokens(); + $modifierPointer = TokenHelper::findPreviousEffective($phpcsFile, $classPointer - 1); + while ( + $modifierPointer !== null + && in_array($tokens[$modifierPointer]['code'], [T_FINAL, T_ABSTRACT, T_READONLY], true) + ) { + if ($tokens[$modifierPointer]['code'] === T_READONLY) { + return true; + } + + $modifierPointer = TokenHelper::findPreviousEffective($phpcsFile, $modifierPointer - 1); + } + + return false; + } + + /** + * @return list + */ + private function getPromotedProperties(File $phpcsFile, int $constructorPointer): array + { + $tokens = $phpcsFile->getTokens(); + $promotedProperties = []; + + for ($i = $tokens[$constructorPointer]['parenthesis_opener'] + 1; $i < $tokens[$constructorPointer]['parenthesis_closer']; $i++) { + if ($tokens[$i]['code'] !== T_VARIABLE) { + continue; + } + + $pointerBefore = TokenHelper::findPrevious( + $phpcsFile, + [T_COMMA, T_OPEN_PARENTHESIS, T_ATTRIBUTE_END], + $i - 1, + ); + $modifierPointer = TokenHelper::findNextEffective($phpcsFile, $pointerBefore + 1); + if (!in_array($tokens[$modifierPointer]['code'], TokenHelper::PROPERTY_MODIFIERS_TOKEN_CODES, true)) { + continue; + } + + $readonlyModifierPointer = TokenHelper::findNext($phpcsFile, T_READONLY, $modifierPointer, $i); + $promotedProperties[] = [ + 'propertyPointer' => $i, + 'readonlyModifierPointer' => $readonlyModifierPointer, + ]; + } + + return $promotedProperties; + } + + /** + * @return list + */ + private function getClassBodyProperties(File $phpcsFile, int $classPointer, ?int $constructorPointer): array + { + $tokens = $phpcsFile->getTokens(); + $classBodyProperties = []; + + $constructorParamsStart = $constructorPointer !== null ? $tokens[$constructorPointer]['parenthesis_opener'] : null; + $constructorParamsEnd = $constructorPointer !== null ? $tokens[$constructorPointer]['parenthesis_closer'] : null; + + for ($i = $tokens[$classPointer]['scope_opener'] + 1; $i < $tokens[$classPointer]['scope_closer']; $i++) { + if ($tokens[$i]['code'] !== T_VARIABLE) { + continue; + } + + // Skip variables inside the constructor parameter list (those are promoted properties) + if ( + $constructorParamsStart !== null + && $i > $constructorParamsStart + && $i < $constructorParamsEnd + ) { + continue; + } + + if (!PropertyHelper::isProperty($phpcsFile, $i)) { + continue; + } + + // Only include properties directly owned by this class (not nested anonymous classes) + $conditions = $tokens[$i]['conditions'] ?? []; + $lastConditionPointer = null; + foreach (array_keys($conditions) as $condPointer) { + $lastConditionPointer = $condPointer; + } + + if ($lastConditionPointer !== $classPointer) { + continue; + } + + $propertyStartPointer = PropertyHelper::getStartPointer($phpcsFile, $i); + $readonlyModifierPointer = TokenHelper::findNext($phpcsFile, T_READONLY, $propertyStartPointer, $i); + + $classBodyProperties[] = [ + 'propertyPointer' => $i, + 'readonlyModifierPointer' => $readonlyModifierPointer, + ]; + } + + return $classBodyProperties; + } + + private function removeReadonlyModifier(File $phpcsFile, int $readonlyModifierPointer): void + { + $phpcsFile->fixer->replaceToken($readonlyModifierPointer, ''); + $nextPointer = TokenHelper::findNext($phpcsFile, T_WHITESPACE, $readonlyModifierPointer + 1, $readonlyModifierPointer + 2); + if ($nextPointer !== null) { + $phpcsFile->fixer->replaceToken($nextPointer, ''); + } + } + +} diff --git a/doc/classes.md b/doc/classes.md index e1229d50d..abd4471f0 100644 --- a/doc/classes.md +++ b/doc/classes.md @@ -232,6 +232,12 @@ Sniff provides the following settings: * `minLinesCountBeforeMultiline` (default: `null`): minimum number of lines before multiline property * `maxLinesCountBeforeMultiline` (default: `null`): maximum number of lines before multiline property +#### SlevomatCodingStandard.Classes.ReadonlyClass 🔧 + +Reports classes where all promoted constructor properties are declared as `readonly` and suggests marking the whole class as `readonly`. + +In readonly classes, promoted constructor properties must not be explicitly declared as `readonly`. + #### SlevomatCodingStandard.Classes.RequireAbstractOrFinal 🔧 Requires the class to be declared either as abstract or as final. diff --git a/tests/Sniffs/Classes/ReadonlyClassSniffTest.php b/tests/Sniffs/Classes/ReadonlyClassSniffTest.php new file mode 100644 index 000000000..c6a981fe5 --- /dev/null +++ b/tests/Sniffs/Classes/ReadonlyClassSniffTest.php @@ -0,0 +1,99 @@ +skipIfReadonlyClassIsNotSupported(); + $report = self::checkFile(__DIR__ . '/data/readonlyClassNoErrors.php'); + self::assertNoSniffErrorInFile($report); + } + + public function testErrors(): void + { + $this->skipIfReadonlyClassIsNotSupported(); + $report = self::checkFile(__DIR__ . '/data/readonlyClassErrors.php'); + self::assertSame(8, $report->getErrorCount()); + self::assertSniffError($report, 3, ReadonlyClassSniff::CODE_CLASS_CAN_BE_READONLY, 'Class Candidate can be marked as readonly.'); + self::assertSniffError( + $report, + 16, + ReadonlyClassSniff::CODE_PROMOTED_PROPERTY_CANNOT_BE_READONLY_IN_READONLY_CLASS, + 'Promoted property $id in readonly class cannot be declared as readonly.', + ); + self::assertSniffError( + $report, + 17, + ReadonlyClassSniff::CODE_PROMOTED_PROPERTY_CANNOT_BE_READONLY_IN_READONLY_CLASS, + 'Promoted property $name in readonly class cannot be declared as readonly.', + ); + self::assertSniffError( + $report, + 23, + ReadonlyClassSniff::CODE_CLASS_CAN_BE_READONLY, + 'Class FinalCandidate can be marked as readonly.', + ); + self::assertSniffError( + $report, + 30, + ReadonlyClassSniff::CODE_CLASS_CAN_BE_READONLY, + 'Class WithBodyPropertiesCandidate can be marked as readonly.', + ); + self::assertSniffError( + $report, + 36, + ReadonlyClassSniff::CODE_CLASS_CAN_BE_READONLY, + 'Class MixedPropertiesCandidate can be marked as readonly.', + ); + self::assertSniffError( + $report, + 45, + ReadonlyClassSniff::CODE_CLASS_CAN_BE_READONLY, + 'Class WithoutPromotion can be marked as readonly.', + ); + self::assertSniffError( + $report, + 56, + ReadonlyClassSniff::CODE_PROMOTED_PROPERTY_CANNOT_BE_READONLY_IN_READONLY_CLASS, + 'Property $extra in readonly class cannot be declared as readonly.', + ); + self::assertAllFixedInFile($report); + } + + public function testNoConstructorNoErrors(): void + { + $this->skipIfReadonlyClassIsNotSupported(); + $report = self::checkFile(__DIR__ . '/data/readonlyClassNoConstructorNoErrors.php'); + self::assertNoSniffErrorInFile($report); + } + + public function testFindConstructorPointerBranchesNoErrors(): void + { + $this->skipIfReadonlyClassIsNotSupported(); + $report = self::checkFile(__DIR__ . '/data/readonlyClassFindConstructorPointerBranchesNoErrors.php'); + self::assertNoSniffErrorInFile($report); + } + + public function testShouldNotReportIfSniffIsDisabled(): void + { + $this->skipIfReadonlyClassIsNotSupported(); + $report = self::checkFile(__DIR__ . '/data/readonlyClassErrors.php', [ + 'enable' => false, + ]); + self::assertNoSniffErrorInFile($report); + } + + private function skipIfReadonlyClassIsNotSupported(): void + { + if (PHP_VERSION_ID < 80200) { + self::markTestSkipped('Readonly classes are supported in PHP 8.2+ only.'); + } + } + +} diff --git a/tests/Sniffs/Classes/data/readonlyClassErrors.fixed.php b/tests/Sniffs/Classes/data/readonlyClassErrors.fixed.php new file mode 100644 index 000000000..49b1b7982 --- /dev/null +++ b/tests/Sniffs/Classes/data/readonlyClassErrors.fixed.php @@ -0,0 +1,61 @@ += 8.2 + +readonly class Candidate +{ + public function __construct( + private int $id, + public string $name, + ) + { + } +} + +readonly class InvalidReadonly +{ + public function __construct( + private int $id, + protected string $name, + ) + { + } +} + +final readonly class FinalCandidate +{ + public function __construct(private int $id) + { + } +} + +readonly class WithBodyPropertiesCandidate +{ + private int $id; + public string $name; +} + +readonly class MixedPropertiesCandidate +{ + private string $extra; + + public function __construct(private int $id) + { + } +} + +readonly class WithoutPromotion +{ + private int $id; + public function __construct(int $id) + { + $this->id = $id; + } +} + +readonly class InvalidReadonlyWithBodyProperty +{ + private int $extra; + + public function __construct(private int $id) + { + } +} diff --git a/tests/Sniffs/Classes/data/readonlyClassErrors.php b/tests/Sniffs/Classes/data/readonlyClassErrors.php new file mode 100644 index 000000000..993e312f0 --- /dev/null +++ b/tests/Sniffs/Classes/data/readonlyClassErrors.php @@ -0,0 +1,61 @@ += 8.2 + +class Candidate +{ + public function __construct( + private readonly int $id, + public readonly string $name, + ) + { + } +} + +readonly class InvalidReadonly +{ + public function __construct( + private readonly int $id, + protected readonly string $name, + ) + { + } +} + +final class FinalCandidate +{ + public function __construct(private readonly int $id) + { + } +} + +class WithBodyPropertiesCandidate +{ + private readonly int $id; + public readonly string $name; +} + +class MixedPropertiesCandidate +{ + private readonly string $extra; + + public function __construct(private readonly int $id) + { + } +} + +class WithoutPromotion +{ + private readonly int $id; + public function __construct(int $id) + { + $this->id = $id; + } +} + +readonly class InvalidReadonlyWithBodyProperty +{ + private readonly int $extra; + + public function __construct(private int $id) + { + } +} diff --git a/tests/Sniffs/Classes/data/readonlyClassFindConstructorPointerBranchesNoErrors.php b/tests/Sniffs/Classes/data/readonlyClassFindConstructorPointerBranchesNoErrors.php new file mode 100644 index 000000000..74335b380 --- /dev/null +++ b/tests/Sniffs/Classes/data/readonlyClassFindConstructorPointerBranchesNoErrors.php @@ -0,0 +1,28 @@ += 8.2 + +class ConstructorPointerBranches +{ + public function process(): void + { + $anonymous = new class { + public function __construct(private readonly int $id) + { + } + }; + } +} + +class ConstructorPointerBranchesWithAnonClassBodyProperty +{ + public function process(): void + { + $anonymous = new class { + private readonly int $id; + + public function __construct(int $id) + { + $this->id = $id; + } + }; + } +} diff --git a/tests/Sniffs/Classes/data/readonlyClassNoConstructorNoErrors.php b/tests/Sniffs/Classes/data/readonlyClassNoConstructorNoErrors.php new file mode 100644 index 000000000..2b6e10116 --- /dev/null +++ b/tests/Sniffs/Classes/data/readonlyClassNoConstructorNoErrors.php @@ -0,0 +1,11 @@ += 8.2 + +class WithoutConstructor +{ + private int $id; +} + +readonly class ReadonlyWithoutConstructor +{ + private int $id; +} diff --git a/tests/Sniffs/Classes/data/readonlyClassNoErrors.php b/tests/Sniffs/Classes/data/readonlyClassNoErrors.php new file mode 100644 index 000000000..655233e31 --- /dev/null +++ b/tests/Sniffs/Classes/data/readonlyClassNoErrors.php @@ -0,0 +1,24 @@ += 8.2 + +class MixedPromotion +{ + public function __construct(private readonly int $id, private string $name,) + { + } +} + +readonly class ValidReadonly +{ + public function __construct(private int $id, private string $name,) + { + } +} + +class PromotedReadonlyWithNonReadonlyBodyProperty +{ + private int $extra; + + public function __construct(private readonly int $id) + { + } +}