From 5075483305f9811d9b09c3654a50426af3f677b5 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 15:38:49 -0300 Subject: [PATCH 01/15] Created the rule cake.entity.strictPropertyAccess --- .../Model/EntityStrictPropertyAccessRule.php | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/Rule/Model/EntityStrictPropertyAccessRule.php diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php new file mode 100644 index 0000000..a836b2b --- /dev/null +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -0,0 +1,77 @@ + + */ + public function processNode(Node $node, Scope $scope): array + { + assert($node instanceof Assign); + + if (!$node->var instanceof PropertyFetch) { + return []; + } + if (!$node->var->var instanceof Variable + || ($node->var->var->name ?? null) !== 'this' + ) { + return []; + } + $classReflection = $scope->getClassReflection(); + if ($classReflection === null) { + return []; + } + if (!$node->var->name instanceof Identifier) { + return []; + } + $propertyName = $node->var->name->toString(); + if (!$classReflection->hasNativeProperty($propertyName)) { + return []; + } + try { + $property = $classReflection->getNativeProperty($propertyName); + if (!$property->isProtectedSet()) { + return []; + } + return [ + RuleErrorBuilder::message(sprintf( + 'Modifying property "$this->%s" in entity class "%s" is not allowed. Use $this->set(...) for proper dirty tracking.', + $propertyName, + $classReflection->getName(), + )) + ->identifier($this->identifier) + ->build(), + ]; + } catch (MissingPropertyFromReflectionException) { + return []; + } + } +} From 03864745c8767af83cfc65a1877aa278414ba03c Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 15:38:57 -0300 Subject: [PATCH 02/15] Created the rule cake.entity.strictPropertyAccess --- .../EntityStrictPropertyAccessRuleTest.php | 52 +++++++++++++++++++ .../TestCase/Rule/Model/Fake/FailingUser.php | 37 +++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php create mode 100644 tests/TestCase/Rule/Model/Fake/FailingUser.php diff --git a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php new file mode 100644 index 0000000..4e04eab --- /dev/null +++ b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php @@ -0,0 +1,52 @@ +analyse([__DIR__ . '/Fake/FailingUser.php'], [ + [ + 'Modifying property "$this->first_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(...) for proper dirty tracking.', + + + 32, // asserted error line + ], + [ + 'Modifying property "$this->last_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(...) for proper dirty tracking.', + 33, // asserted error line + ], + ]); + } +} diff --git a/tests/TestCase/Rule/Model/Fake/FailingUser.php b/tests/TestCase/Rule/Model/Fake/FailingUser.php new file mode 100644 index 0000000..c2bd097 --- /dev/null +++ b/tests/TestCase/Rule/Model/Fake/FailingUser.php @@ -0,0 +1,37 @@ +password = $value ? password_hash($value, PASSWORD_DEFAULT) : null; +// } +// } + + public string $full_name { + get => $this->first_name . ' ' . $this->last_name; + } + + /** + * @return void + */ + public function fail() + { + //Error properties should be set using $this>set method + $this->first_name = 'John'; + $this->last_name = 'Doe'; + + $this['is_active'] = true;//This is okay + } +} From caedd29c9097986b316ddef55dd8673fd35fee26 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 15:53:21 -0300 Subject: [PATCH 03/15] Created the rule cake.entity.strictPropertyAccess (allow assignment in property hook) --- src/Rule/Model/EntityStrictPropertyAccessRule.php | 6 ++++++ .../Model/EntityStrictPropertyAccessRuleTest.php | 2 -- tests/TestCase/Rule/Model/Fake/FailingUser.php | 10 +++++----- tests/test_app/Model/Entity/User.php | 14 +++++++++++++- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index a836b2b..0629a04 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -10,6 +10,7 @@ use PhpParser\Node\Identifier; use PHPStan\Analyser\Scope; use PHPStan\Reflection\MissingPropertyFromReflectionException; +use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -56,6 +57,11 @@ public function processNode(Node $node, Scope $scope): array if (!$classReflection->hasNativeProperty($propertyName)) { return []; } + $currentFunction = $scope->getFunction(); + if ($currentFunction instanceof PhpMethodFromParserNodeReflection && $currentFunction->getHookedPropertyName() === $propertyName) { + return []; + } + try { $property = $classReflection->getNativeProperty($propertyName); if (!$property->isProtectedSet()) { diff --git a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php index 4e04eab..582fec5 100644 --- a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php +++ b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php @@ -39,8 +39,6 @@ public function testRule(): void $this->analyse([__DIR__ . '/Fake/FailingUser.php'], [ [ 'Modifying property "$this->first_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(...) for proper dirty tracking.', - - 32, // asserted error line ], [ diff --git a/tests/TestCase/Rule/Model/Fake/FailingUser.php b/tests/TestCase/Rule/Model/Fake/FailingUser.php index c2bd097..2ca5d84 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingUser.php +++ b/tests/TestCase/Rule/Model/Fake/FailingUser.php @@ -13,11 +13,11 @@ class FailingUser extends Entity public protected(set) ?string $first_name; public protected(set) ?string $last_name; - public protected(set) ?string $password; //{ -// set(?string $value) { -// $this->password = $value ? password_hash($value, PASSWORD_DEFAULT) : null; -// } -// } + public protected(set) ?string $password { + set(?string $value) { + $this->password = $value ? password_hash($value, PASSWORD_DEFAULT) : null; + } + } public string $full_name { get => $this->first_name . ' ' . $this->last_name; diff --git a/tests/test_app/Model/Entity/User.php b/tests/test_app/Model/Entity/User.php index a67d9d1..0ac3dc8 100644 --- a/tests/test_app/Model/Entity/User.php +++ b/tests/test_app/Model/Entity/User.php @@ -7,7 +7,6 @@ /** * @property int $id - * @property string $name * @property string $role * @property \Cake\I18n\DateTime $last_login * @property \Cake\I18n\DateTime $created @@ -15,4 +14,17 @@ */ class User extends Entity { + /** + * @var string|null + */ + public protected(set) ?string $name; + + /** + * @var string|null + */ + public protected(set) ?string $password { + set(?string $value) { + $this->password = $value ? password_hash($value, PASSWORD_DEFAULT) : null; + } + } } From a444582441a40dce7746ee12235a48822bbc7001 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 16:27:53 -0300 Subject: [PATCH 04/15] Created the rule cake.entity.strictPropertyAccess (improve error message) --- .../Model/EntityStrictPropertyAccessRule.php | 13 ++++++- .../EntityStrictPropertyAccessRuleTest.php | 37 ++++++++++++++++--- .../TestCase/Rule/Model/Fake/FailingUser.php | 11 +++++- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index 0629a04..92275d7 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -9,6 +9,7 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; use PHPStan\Analyser\Scope; +use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Reflection\MissingPropertyFromReflectionException; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Rules\Rule; @@ -21,6 +22,13 @@ class EntityStrictPropertyAccessRule implements Rule */ protected string $identifier = 'cake.entity.strictPropertyAccess'; + /** + * @param \PHPStan\Node\Printer\ExprPrinter $printer + */ + public function __construct(private ExprPrinter $printer) + { + } + /** * @inheritDoc */ @@ -67,11 +75,14 @@ public function processNode(Node $node, Scope $scope): array if (!$property->isProtectedSet()) { return []; } + return [ RuleErrorBuilder::message(sprintf( - 'Modifying property "$this->%s" in entity class "%s" is not allowed. Use $this->set(...) for proper dirty tracking.', + 'Modifying property "%s" in entity class "%s" is not allowed. Use $this->set(\'%s\', %s) for proper dirty tracking.', $propertyName, $classReflection->getName(), + $propertyName, + $this->printer->printExpr($node->expr) )) ->identifier($this->identifier) ->build(), diff --git a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php index 582fec5..eb7c92f 100644 --- a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php +++ b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php @@ -14,6 +14,7 @@ namespace CakeDC\PHPStan\Test\TestCase\Rule\Model; use CakeDC\PHPStan\Rule\Model\EntityStrictPropertyAccessRule; +use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; @@ -24,8 +25,10 @@ class EntityStrictPropertyAccessRuleTest extends RuleTestCase */ protected function getRule(): Rule { + $printer = $this->getContainer()->getByType(ExprPrinter::class); + // getRule() method needs to return an instance of the tested rule - return new EntityStrictPropertyAccessRule(); + return new EntityStrictPropertyAccessRule($printer); } /** @@ -38,12 +41,36 @@ public function testRule(): void // each error consists of the asserted error message, and the asserted error file line $this->analyse([__DIR__ . '/Fake/FailingUser.php'], [ [ - 'Modifying property "$this->first_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(...) for proper dirty tracking.', - 32, // asserted error line + 'Modifying property "first_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'first_name\', \'John\') for proper dirty tracking.', + 35, // asserted error line + ], + [ + 'Modifying property "last_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'last_name\', \'Doe\') for proper dirty tracking.', + 36, // asserted error line + ], + [ + 'Modifying property "password" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'password\', \'0000000\') for proper dirty tracking.', + 37, // asserted error line + ], + [ + 'Modifying property "is_active" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'is_active\', true) for proper dirty tracking.', + 38, // asserted error line + ], + [ + 'Modifying property "id" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'id\', 100) for proper dirty tracking.', + 39, // asserted error line + ], + [ + 'Modifying property "rate" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'rate\', 9.5) for proper dirty tracking.', + 40, // asserted error line + ], + [ + 'Modifying property "first_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'first_name\', $name) for proper dirty tracking.', + 43, // asserted error line ], [ - 'Modifying property "$this->last_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(...) for proper dirty tracking.', - 33, // asserted error line + 'Modifying property "password" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'password\', static::FIELD_ID) for proper dirty tracking.', + 44, // asserted error line ], ]); } diff --git a/tests/TestCase/Rule/Model/Fake/FailingUser.php b/tests/TestCase/Rule/Model/Fake/FailingUser.php index 2ca5d84..b77134b 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingUser.php +++ b/tests/TestCase/Rule/Model/Fake/FailingUser.php @@ -7,7 +7,10 @@ class FailingUser extends Entity { + public const string FIELD_ID = 'id'; + public protected(set) int $id; + public protected(set) float $rate; public protected(set) ?string $email; public protected(set) bool $is_active = false; public protected(set) ?string $first_name; @@ -31,7 +34,13 @@ public function fail() //Error properties should be set using $this>set method $this->first_name = 'John'; $this->last_name = 'Doe'; - + $this->password = '0000000'; + $this->is_active = true; + $this->id = 100; + $this->rate = 9.5; $this['is_active'] = true;//This is okay + $name = 'James'; + $this->first_name = $name; + $this->password = static::FIELD_ID; } } From 28889f8b4f5d4171869ae1c018234b56186e11fd Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 16:39:21 -0300 Subject: [PATCH 05/15] Codesniff are not working with property hooks --- src/Rule/Model/EntityStrictPropertyAccessRule.php | 5 +++-- tests/TestCase/Rule/Model/Fake/FailingUser.php | 10 +++++----- tests/test_app/Model/Entity/User.php | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index 92275d7..a7ccef7 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -49,7 +49,8 @@ public function processNode(Node $node, Scope $scope): array if (!$node->var instanceof PropertyFetch) { return []; } - if (!$node->var->var instanceof Variable + if ( + !$node->var->var instanceof Variable || ($node->var->var->name ?? null) !== 'this' ) { return []; @@ -82,7 +83,7 @@ public function processNode(Node $node, Scope $scope): array $propertyName, $classReflection->getName(), $propertyName, - $this->printer->printExpr($node->expr) + $this->printer->printExpr($node->expr), )) ->identifier($this->identifier) ->build(), diff --git a/tests/TestCase/Rule/Model/Fake/FailingUser.php b/tests/TestCase/Rule/Model/Fake/FailingUser.php index b77134b..713514e 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingUser.php +++ b/tests/TestCase/Rule/Model/Fake/FailingUser.php @@ -16,14 +16,14 @@ class FailingUser extends Entity public protected(set) ?string $first_name; public protected(set) ?string $last_name; - public protected(set) ?string $password { - set(?string $value) { - $this->password = $value ? password_hash($value, PASSWORD_DEFAULT) : null; + public protected(set) ?string $password {//phpcs:ignore + set(?string $value) {//phpcs:ignore + $this->password = $value ? password_hash($value, PASSWORD_DEFAULT) : null;//phpcs:ignore } } - public string $full_name { - get => $this->first_name . ' ' . $this->last_name; + public string $full_name {//phpcs:ignore + get => $this->first_name . ' ' . $this->last_name;//phpcs:ignore } /** diff --git a/tests/test_app/Model/Entity/User.php b/tests/test_app/Model/Entity/User.php index 0ac3dc8..a292077 100644 --- a/tests/test_app/Model/Entity/User.php +++ b/tests/test_app/Model/Entity/User.php @@ -22,9 +22,9 @@ class User extends Entity /** * @var string|null */ - public protected(set) ?string $password { - set(?string $value) { - $this->password = $value ? password_hash($value, PASSWORD_DEFAULT) : null; + public protected(set) ?string $password {//phpcs:ignore + set { + $this->password = $value ? password_hash($value, PASSWORD_DEFAULT) : null;//phpcs:ignore } } } From 98a656c338fddcd57a0abc4ba1e8567c16a4d965 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 16:41:49 -0300 Subject: [PATCH 06/15] Ignore line size error (phpcs) --- src/Rule/Model/EntityStrictPropertyAccessRule.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index a7ccef7..07cae8f 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -67,7 +67,10 @@ public function processNode(Node $node, Scope $scope): array return []; } $currentFunction = $scope->getFunction(); - if ($currentFunction instanceof PhpMethodFromParserNodeReflection && $currentFunction->getHookedPropertyName() === $propertyName) { + if ( + $currentFunction instanceof PhpMethodFromParserNodeReflection + && $currentFunction->getHookedPropertyName() === $propertyName + ) { return []; } @@ -79,7 +82,7 @@ public function processNode(Node $node, Scope $scope): array return [ RuleErrorBuilder::message(sprintf( - 'Modifying property "%s" in entity class "%s" is not allowed. Use $this->set(\'%s\', %s) for proper dirty tracking.', + 'Modifying property "%s" in entity class "%s" is not allowed. Use $this->set(\'%s\', %s) for proper dirty tracking.',// phpcs:ignore $propertyName, $classReflection->getName(), $propertyName, From 437deb527edc2d5a887acf6e65395ca7c6990531 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 17:05:14 -0300 Subject: [PATCH 07/15] cleanup --- src/Rule/Model/EntityStrictPropertyAccessRule.php | 2 +- tests/test_app/Model/Entity/User.php | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index 07cae8f..9d90fb1 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -51,7 +51,7 @@ public function processNode(Node $node, Scope $scope): array } if ( !$node->var->var instanceof Variable - || ($node->var->var->name ?? null) !== 'this' + || $node->var->var->name !== 'this' ) { return []; } diff --git a/tests/test_app/Model/Entity/User.php b/tests/test_app/Model/Entity/User.php index a292077..a3a7242 100644 --- a/tests/test_app/Model/Entity/User.php +++ b/tests/test_app/Model/Entity/User.php @@ -7,6 +7,7 @@ /** * @property int $id + * @property string $name * @property string $role * @property \Cake\I18n\DateTime $last_login * @property \Cake\I18n\DateTime $created @@ -14,11 +15,6 @@ */ class User extends Entity { - /** - * @var string|null - */ - public protected(set) ?string $name; - /** * @var string|null */ From d359ea085c5e1d2c8d8d1f4b0c9584a15f01d840 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 17:07:07 -0300 Subject: [PATCH 08/15] cleanup --- phpstan.neon | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phpstan.neon b/phpstan.neon index b6df14b..ce4beac 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -13,3 +13,8 @@ parameters: ignoreErrors: - identifier: missingType.generics + - + message: '#^Although PHPStan\\Reflection\\Php\\PhpMethodFromParserNodeReflection is covered by backward compatibility promise, this instanceof assumption might break because it''s not guaranteed to always stay the same\.$#' + identifier: phpstanApi.instanceofAssumption + count: 1 + path: src/Rule/Model/EntityStrictPropertyAccessRule.php From c7c5ed422df55f9f946fd653cce2cdbb84fc6aef Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 17:10:16 -0300 Subject: [PATCH 09/15] Enable and doc the rule cake.entity.strictPropertyAccess --- README.md | 3 +++ rules.neon | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/README.md b/README.md index 0c793b2..13c7d67 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,9 @@ Methods covered: ### DisallowEntityArrayAccessRule This rule disallow array access to entity in favor of object notation, is easier to detect a wrong property and to refactor code. +### EntityStrictPropertyAccessRule +This rule check incorrect assigment of native entity properties to avoid issues with dirty tracking. + ### GetMailerExistsClassRule This rule check if the target mailer is a valid class when calling to Cake\Mailer\MailerAwareTrait::getMailer. diff --git a/rules.neon b/rules.neon index aaccac6..5170b9e 100644 --- a/rules.neon +++ b/rules.neon @@ -7,6 +7,7 @@ parameters: tableGetMatchOptionsTypesRule: true ormSelectQueryFindMatchOptionsTypesRule: true disallowEntityArrayAccessRule: true + entityStrictPropertyAccessRule: true getMailerExistsClassRule: true loadComponentExistsClassRule: true controllerMethodMustBeUsedRule: true @@ -21,6 +22,7 @@ parametersSchema: tableGetMatchOptionsTypesRule: anyOf(bool(), arrayOf(bool())) ormSelectQueryFindMatchOptionsTypesRule: anyOf(bool(), arrayOf(bool())) disallowEntityArrayAccessRule: anyOf(bool(), arrayOf(bool())) + entityStrictPropertyAccessRule: anyOf(bool(), arrayOf(bool())) getMailerExistsClassRule: anyOf(bool(), arrayOf(bool())) loadComponentExistsClassRule: anyOf(bool(), arrayOf(bool())) controllerMethodMustBeUsedRule: anyOf(bool(), arrayOf(bool())) @@ -43,6 +45,8 @@ conditionalTags: phpstan.rules.rule: %cakeDC.addBehaviorExistsClassRule% CakeDC\PHPStan\Rule\Model\DisallowEntityArrayAccessRule: phpstan.rules.rule: %cakeDC.disallowEntityArrayAccessRule% + CakeDC\PHPStan\Rule\Model\EntityStrictPropertyAccessRule: + phpstan.rules.rule: %cakeDC.entityStrictPropertyAccessRule% CakeDC\PHPStan\Rule\Mailer\GetMailerExistsClassRule: phpstan.rules.rule: %cakeDC.getMailerExistsClassRule% CakeDC\PHPStan\Rule\Model\TableGetMatchOptionsTypesRule: @@ -69,6 +73,8 @@ services: class: CakeDC\PHPStan\Rule\Model\AddBehaviorExistsClassRule - class: CakeDC\PHPStan\Rule\Model\DisallowEntityArrayAccessRule + - + class: CakeDC\PHPStan\Rule\Model\EntityStrictPropertyAccessRule - class: CakeDC\PHPStan\Rule\Mailer\GetMailerExistsClassRule - From a1f69cebfb3b3a45e4de58282fa33ae61249d8c7 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 17:21:45 -0300 Subject: [PATCH 10/15] Cleanup --- phpstan-test-app.neon | 9 ++++++--- src/Rule/Model/EntityStrictPropertyAccessRule.php | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/phpstan-test-app.neon b/phpstan-test-app.neon index c250b63..0b0efa2 100644 --- a/phpstan-test-app.neon +++ b/phpstan-test-app.neon @@ -8,6 +8,9 @@ parameters: treatPhpDocTypesAsCertain: false cakeDC: disallowEntityArrayAccessRule: true - ignoreErrors: - - - identifier: missingType.generics + ignoreErrors: + - + message: '#^Assign to protected\(set\) property App\\Model\\Entity\\User\:\:\$name\.$#' + identifier: assign.propertyProtectedSet + count: 1 + path: tests/test_app/Model/Table/NotesTable.php diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index 9d90fb1..07d122d 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -3,6 +3,7 @@ namespace CakeDC\PHPStan\Rule\Model; +use Cake\ORM\Entity; use PhpParser\Node; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; @@ -59,6 +60,9 @@ public function processNode(Node $node, Scope $scope): array if ($classReflection === null) { return []; } + if (!$classReflection->isSubclassOf(Entity::class) && $classReflection->getName() !== Entity::class) { + return []; + } if (!$node->var->name instanceof Identifier) { return []; } From e31abdc2f7ab32caa486fe6c62a359003b3a04eb Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 17:31:08 -0300 Subject: [PATCH 11/15] Cleanup --- phpstan-test-app.neon | 6 ------ src/Rule/Model/EntityStrictPropertyAccessRule.php | 13 ++++++++++--- .../Model/EntityStrictPropertyAccessRuleTest.php | 5 +++-- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/phpstan-test-app.neon b/phpstan-test-app.neon index 0b0efa2..7c6c400 100644 --- a/phpstan-test-app.neon +++ b/phpstan-test-app.neon @@ -8,9 +8,3 @@ parameters: treatPhpDocTypesAsCertain: false cakeDC: disallowEntityArrayAccessRule: true - ignoreErrors: - - - message: '#^Assign to protected\(set\) property App\\Model\\Entity\\User\:\:\$name\.$#' - identifier: assign.propertyProtectedSet - count: 1 - path: tests/test_app/Model/Table/NotesTable.php diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index 07d122d..ded2c41 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -13,6 +13,7 @@ use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Reflection\MissingPropertyFromReflectionException; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -26,8 +27,10 @@ class EntityStrictPropertyAccessRule implements Rule /** * @param \PHPStan\Node\Printer\ExprPrinter $printer */ - public function __construct(private ExprPrinter $printer) - { + public function __construct( + private ExprPrinter $printer, + private ReflectionProvider $reflectionProvider, + ) { } /** @@ -60,7 +63,11 @@ public function processNode(Node $node, Scope $scope): array if ($classReflection === null) { return []; } - if (!$classReflection->isSubclassOf(Entity::class) && $classReflection->getName() !== Entity::class) { + $entityBaseReflection = $this->reflectionProvider->getClass(Entity::class); + if ( + !$classReflection->isSubclassOfClass($entityBaseReflection) + && $classReflection->getName() !== Entity::class + ) { return []; } if (!$node->var->name instanceof Identifier) { diff --git a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php index eb7c92f..83e3ad3 100644 --- a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php +++ b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php @@ -15,6 +15,7 @@ use CakeDC\PHPStan\Rule\Model\EntityStrictPropertyAccessRule; use PHPStan\Node\Printer\ExprPrinter; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; @@ -26,9 +27,9 @@ class EntityStrictPropertyAccessRuleTest extends RuleTestCase protected function getRule(): Rule { $printer = $this->getContainer()->getByType(ExprPrinter::class); - + $reflectionProvider = $this->getContainer()->getByType(ReflectionProvider::class); // getRule() method needs to return an instance of the tested rule - return new EntityStrictPropertyAccessRule($printer); + return new EntityStrictPropertyAccessRule($printer, $reflectionProvider); } /** From 7878eaea8a807975386fcd39de609c8a1aa709cf Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Fri, 16 Oct 2026 17:38:13 -0300 Subject: [PATCH 12/15] Cleanup --- tests/test_app/Model/Table/NotesTable.php | 25 ++++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/test_app/Model/Table/NotesTable.php b/tests/test_app/Model/Table/NotesTable.php index b4065a6..0711b56 100644 --- a/tests/test_app/Model/Table/NotesTable.php +++ b/tests/test_app/Model/Table/NotesTable.php @@ -83,10 +83,11 @@ public function warning(): array } /** - * @param \Cake\ORM\Query\SelectQuery $query + * @template TSubject of \Cake\Datasource\EntityInterface|array + * @param \Cake\ORM\Query\SelectQuery $query * @param string|int $year * @param bool $fun - * @return \Cake\ORM\Query\SelectQuery + * @return \Cake\ORM\Query\SelectQuery */ public function findFeatured(SelectQuery $query, string|int $year, bool $fun): SelectQuery { @@ -105,9 +106,10 @@ public function findFeatured(SelectQuery $query, string|int $year, bool $fun): S } /** - * @param \Cake\ORM\Query\SelectQuery $query + * @template TSubject of \Cake\Datasource\EntityInterface|array + * @param \Cake\ORM\Query\SelectQuery $query * @param string $myType - * @return \Cake\ORM\Query\SelectQuery + * @return \Cake\ORM\Query\SelectQuery */ public function findTwoArgsButNotLegacy(SelectQuery $query, string $myType): SelectQuery { @@ -121,9 +123,10 @@ public function findTwoArgsButNotLegacy(SelectQuery $query, string $myType): Sel } /** - * @param \Cake\ORM\Query\SelectQuery $query + * @template TSubject of \Cake\Datasource\EntityInterface|array + * @param \Cake\ORM\Query\SelectQuery $query * @param array $options - * @return \Cake\ORM\Query\SelectQuery + * @return \Cake\ORM\Query\SelectQuery */ public function findLegacy(SelectQuery $query, array $options): SelectQuery { @@ -135,9 +138,10 @@ public function findLegacy(SelectQuery $query, array $options): SelectQuery } /** - * @param \Cake\ORM\Query\SelectQuery $query + * @template TSubject of \Cake\Datasource\EntityInterface|array + * @param \Cake\ORM\Query\SelectQuery $query * @param mixed ...$options - * @return \Cake\ORM\Query\SelectQuery + * @return \Cake\ORM\Query\SelectQuery */ public function findOptionsPacked(SelectQuery $query, mixed ...$options): SelectQuery { @@ -147,9 +151,10 @@ public function findOptionsPacked(SelectQuery $query, mixed ...$options): Select } /** - * @param \Cake\ORM\Query\SelectQuery $query + * @template TSubject of \Cake\Datasource\EntityInterface|array + * @param \Cake\ORM\Query\SelectQuery $query * @param mixed ...$args - * @return \Cake\ORM\Query\SelectQuery + * @return \Cake\ORM\Query\SelectQuery */ public function findArgsPacked(SelectQuery $query, mixed ...$args): SelectQuery { From 287d24a87beccefcb8f574d1cfe2a8ca3e095886 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Sat, 17 Oct 2026 06:36:55 -0300 Subject: [PATCH 13/15] Missing test case --- tests/TestCase/Rule/Model/Fake/FailingUser.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/TestCase/Rule/Model/Fake/FailingUser.php b/tests/TestCase/Rule/Model/Fake/FailingUser.php index 713514e..77c7a3c 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingUser.php +++ b/tests/TestCase/Rule/Model/Fake/FailingUser.php @@ -42,5 +42,6 @@ public function fail() $name = 'James'; $this->first_name = $name; $this->password = static::FIELD_ID; + $this->hidden = ['password'];//Allowed to change from base entity } } From d0b925ccf6a9a54c10cbf16ba1006527755277fc Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Sat, 17 Oct 2026 07:50:26 -0300 Subject: [PATCH 14/15] Correctly handle properties from parent classes --- .../Model/EntityStrictPropertyAccessRule.php | 35 ++++++++----------- .../EntityStrictPropertyAccessRuleTest.php | 4 +++ .../Rule/Model/Fake/BaseFailingUser.php | 11 ++++++ .../TestCase/Rule/Model/Fake/FailingUser.php | 4 ++- 4 files changed, 33 insertions(+), 21 deletions(-) create mode 100644 tests/TestCase/Rule/Model/Fake/BaseFailingUser.php diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index ded2c41..0ad9b5b 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -74,6 +74,10 @@ public function processNode(Node $node, Scope $scope): array return []; } $propertyName = $node->var->name->toString(); + //Ignore properties from \Cake\ORM\Entity + if ($entityBaseReflection->hasNativeProperty($propertyName)) { + return []; + } if (!$classReflection->hasNativeProperty($propertyName)) { return []; } @@ -85,25 +89,16 @@ public function processNode(Node $node, Scope $scope): array return []; } - try { - $property = $classReflection->getNativeProperty($propertyName); - if (!$property->isProtectedSet()) { - return []; - } - - return [ - RuleErrorBuilder::message(sprintf( - 'Modifying property "%s" in entity class "%s" is not allowed. Use $this->set(\'%s\', %s) for proper dirty tracking.',// phpcs:ignore - $propertyName, - $classReflection->getName(), - $propertyName, - $this->printer->printExpr($node->expr), - )) - ->identifier($this->identifier) - ->build(), - ]; - } catch (MissingPropertyFromReflectionException) { - return []; - } + return [ + RuleErrorBuilder::message(sprintf( + 'Modifying property "%s" in entity class "%s" is not allowed. Use $this->set(\'%s\', %s) for proper dirty tracking.',// phpcs:ignore + $propertyName, + $classReflection->getName(), + $propertyName, + $this->printer->printExpr($node->expr), + )) + ->identifier($this->identifier) + ->build(), + ]; } } diff --git a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php index 83e3ad3..0a3e315 100644 --- a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php +++ b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php @@ -73,6 +73,10 @@ public function testRule(): void 'Modifying property "password" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'password\', static::FIELD_ID) for proper dirty tracking.', 44, // asserted error line ], + [ + 'Modifying property "role" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'role\', \'user\') for proper dirty tracking.', + 47, // asserted error line + ], ]); } } diff --git a/tests/TestCase/Rule/Model/Fake/BaseFailingUser.php b/tests/TestCase/Rule/Model/Fake/BaseFailingUser.php new file mode 100644 index 0000000..389faa1 --- /dev/null +++ b/tests/TestCase/Rule/Model/Fake/BaseFailingUser.php @@ -0,0 +1,11 @@ +first_name = $name; $this->password = static::FIELD_ID; $this->hidden = ['password'];//Allowed to change from base entity + $this->new = false; + $this->role = 'user'; } } From 3b831bc85c365777c906480ef8d588fdc4365382 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Sat, 17 Oct 2026 07:59:06 -0300 Subject: [PATCH 15/15] PHPQA fixes --- .../Model/EntityStrictPropertyAccessRule.php | 12 ++++++++---- .../EntityStrictPropertyAccessRuleTest.php | 18 +++++++++--------- tests/TestCase/Rule/Model/Fake/FailingUser.php | 4 +--- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/Rule/Model/EntityStrictPropertyAccessRule.php b/src/Rule/Model/EntityStrictPropertyAccessRule.php index 0ad9b5b..33df42c 100644 --- a/src/Rule/Model/EntityStrictPropertyAccessRule.php +++ b/src/Rule/Model/EntityStrictPropertyAccessRule.php @@ -11,7 +11,7 @@ use PhpParser\Node\Identifier; use PHPStan\Analyser\Scope; use PHPStan\Node\Printer\ExprPrinter; -use PHPStan\Reflection\MissingPropertyFromReflectionException; +use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; @@ -23,6 +23,10 @@ class EntityStrictPropertyAccessRule implements Rule * @var string */ protected string $identifier = 'cake.entity.strictPropertyAccess'; + /** + * @var \PHPStan\Reflection\ClassReflection + */ + protected ClassReflection $entityBaseReflection; /** * @param \PHPStan\Node\Printer\ExprPrinter $printer @@ -31,6 +35,7 @@ public function __construct( private ExprPrinter $printer, private ReflectionProvider $reflectionProvider, ) { + $this->entityBaseReflection = $this->reflectionProvider->getClass(Entity::class); } /** @@ -63,9 +68,8 @@ public function processNode(Node $node, Scope $scope): array if ($classReflection === null) { return []; } - $entityBaseReflection = $this->reflectionProvider->getClass(Entity::class); if ( - !$classReflection->isSubclassOfClass($entityBaseReflection) + !$classReflection->isSubclassOfClass($this->entityBaseReflection) && $classReflection->getName() !== Entity::class ) { return []; @@ -75,7 +79,7 @@ public function processNode(Node $node, Scope $scope): array } $propertyName = $node->var->name->toString(); //Ignore properties from \Cake\ORM\Entity - if ($entityBaseReflection->hasNativeProperty($propertyName)) { + if ($this->entityBaseReflection->hasNativeProperty($propertyName)) { return []; } if (!$classReflection->hasNativeProperty($propertyName)) { diff --git a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php index 0a3e315..84e0274 100644 --- a/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php +++ b/tests/TestCase/Rule/Model/EntityStrictPropertyAccessRuleTest.php @@ -43,39 +43,39 @@ public function testRule(): void $this->analyse([__DIR__ . '/Fake/FailingUser.php'], [ [ 'Modifying property "first_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'first_name\', \'John\') for proper dirty tracking.', - 35, // asserted error line + 33, // asserted error line ], [ 'Modifying property "last_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'last_name\', \'Doe\') for proper dirty tracking.', - 36, // asserted error line + 34, // asserted error line ], [ 'Modifying property "password" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'password\', \'0000000\') for proper dirty tracking.', - 37, // asserted error line + 35, // asserted error line ], [ 'Modifying property "is_active" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'is_active\', true) for proper dirty tracking.', - 38, // asserted error line + 36, // asserted error line ], [ 'Modifying property "id" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'id\', 100) for proper dirty tracking.', - 39, // asserted error line + 37, // asserted error line ], [ 'Modifying property "rate" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'rate\', 9.5) for proper dirty tracking.', - 40, // asserted error line + 38, // asserted error line ], [ 'Modifying property "first_name" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'first_name\', $name) for proper dirty tracking.', - 43, // asserted error line + 41, // asserted error line ], [ 'Modifying property "password" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'password\', static::FIELD_ID) for proper dirty tracking.', - 44, // asserted error line + 42, // asserted error line ], [ 'Modifying property "role" in entity class "CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake\FailingUser" is not allowed. Use $this->set(\'role\', \'user\') for proper dirty tracking.', - 47, // asserted error line + 45, // asserted error line ], ]); } diff --git a/tests/TestCase/Rule/Model/Fake/FailingUser.php b/tests/TestCase/Rule/Model/Fake/FailingUser.php index 9e2f918..06397ed 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingUser.php +++ b/tests/TestCase/Rule/Model/Fake/FailingUser.php @@ -3,8 +3,6 @@ namespace CakeDC\PHPStan\Test\TestCase\Rule\Model\Fake; -use Cake\ORM\Entity; - class FailingUser extends BaseFailingUser { public const string FIELD_ID = 'id'; @@ -31,7 +29,7 @@ class FailingUser extends BaseFailingUser */ public function fail() { - //Error properties should be set using $this>set method + //Error, properties should be set using $this>set method $this->first_name = 'John'; $this->last_name = 'Doe'; $this->password = '0000000';