From 32d60fcd3ad29efdb6bde07d3335b813e1dc725b Mon Sep 17 00:00:00 2001
From: Daniel Badura <daniel.badura@patchlevel.de>
Date: Wed, 3 Dec 2025 23:20:45 +0100
Subject: [PATCH] Add a new rule which detect writes of state which do not
 happen in apply methods, or do be more precise which are happening in
 `#[Handle]` methods.

---
 extension.neon                            |  8 +-
 src/AggregateRootExtension.php            | 36 ---------
 src/DontWriteStateWhenNotApplyingRule.php | 97 +++++++++++++++++++++++
 tests/Invalid/Profile.php                 |  9 +++
 4 files changed, 110 insertions(+), 40 deletions(-)
 delete mode 100644 src/AggregateRootExtension.php
 create mode 100644 src/DontWriteStateWhenNotApplyingRule.php

diff --git a/extension.neon b/extension.neon
index f9fee41..afb10a3 100644
--- a/extension.neon
+++ b/extension.neon
@@ -1,10 +1,10 @@
 services:
 	-
-		class: Patchlevel\EventSourcingPHPStanExtension\AggregateRootExtension
+		class: Patchlevel\EventSourcingPHPStanExtension\DontRecordWhenApplyingExtension
 		tags:
-			- phpstan.properties.readWriteExtension
+			- phpstan.restrictedMethodUsageExtension
 
 	-
-		class: Patchlevel\EventSourcingPHPStanExtension\DontRecordWhenApplyingExtension
+		class: Patchlevel\EventSourcingPHPStanExtension\DontWriteStateWhenNotApplyingRule
 		tags:
-			- phpstan.restrictedMethodUsageExtension
+			- phpstan.rules.rule
diff --git a/src/AggregateRootExtension.php b/src/AggregateRootExtension.php
deleted file mode 100644
index a7274ae..0000000
--- a/src/AggregateRootExtension.php
+++ /dev/null
@@ -1,36 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Patchlevel\EventSourcingPHPStanExtension;
-
-use Patchlevel\EventSourcing\Aggregate\AggregateRoot;
-use Patchlevel\EventSourcing\Aggregate\ChildAggregate;
-use PHPStan\Reflection\PropertyReflection;
-use PHPStan\Rules\Properties\ReadWritePropertiesExtension;
-
-final class AggregateRootExtension implements ReadWritePropertiesExtension
-{
-    public function isAlwaysRead(PropertyReflection $property, string $propertyName): bool
-    {
-        return false;
-    }
-
-    public function isAlwaysWritten(PropertyReflection $property, string $propertyName): bool
-    {
-        return false;
-    }
-
-    public function isInitialized(PropertyReflection $property, string $propertyName): bool
-    {
-        $interfaces = $property->getDeclaringClass()->getInterfaces();
-
-        foreach ($interfaces as $interface) {
-            if ($interface->getName() === AggregateRoot::class || $interface->getName() === ChildAggregate::class) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-}
diff --git a/src/DontWriteStateWhenNotApplyingRule.php b/src/DontWriteStateWhenNotApplyingRule.php
new file mode 100644
index 0000000..9985fb4
--- /dev/null
+++ b/src/DontWriteStateWhenNotApplyingRule.php
@@ -0,0 +1,97 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Patchlevel\EventSourcingPHPStanExtension;
+
+use Patchlevel\EventSourcing\Aggregate\AggregateRoot;
+use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\NodeVisitorAbstract;
+use PHPStan\Analyser\Scope;
+
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\ObjectType;
+use function sprintf;
+
+final class DontWriteStateWhenNotApplyingRule implements Rule
+{
+    public function getNodeType(): string
+    {
+        return ClassMethod::class;
+    }
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $classReflection = $scope->getClassReflection();
+        if (!$classReflection instanceof ClassReflection) {
+            return [];
+        }
+
+        if (!$classReflection->implementsInterface(AggregateRoot::class)) {
+            return [];
+        }
+
+        // check for handle attribute
+        $hasHandleAttribute = false;
+        foreach ($node->attrGroups as $attrGroup) {
+            foreach ($attrGroup->attrs as $attr) {
+                if ((string) $attr->name === 'Handle' || str_ends_with((string) $attr->name, '\\Handle')) {
+                    $hasHandleAttribute = true;
+                    break 2;
+                }
+            }
+        }
+
+        if (!$hasHandleAttribute) {
+            return [];
+        }
+
+        // now try to find writes by traversing over the node
+        $nodeFinder = new \PhpParser\NodeTraverser();
+        $visitor = new class($node, $scope) extends NodeVisitorAbstract {
+            public array $errors;
+            private ClassMethod $method;
+            private Scope $scope;
+
+            public function __construct(ClassMethod $method, Scope $scope)
+            {
+                $this->errors = [];
+                $this->method = $method;
+                $this->scope = $scope;
+            }
+
+            public function enterNode(Node $node)
+            {
+                if ($node instanceof Assign && $node->var instanceof PropertyFetch) {
+                    /** @var PropertyFetch $propertyFetch */
+                    $propertyFetch = $node->var;
+
+                    if (
+                        $propertyFetch->var instanceof Node\Expr\Variable &&
+                        ($propertyFetch->var->name === 'this' || $propertyFetch->var->name === 'self')
+                    ) {
+                        $this->errors[] = RuleErrorBuilder::message(sprintf(
+                            'Property "%s" should not be written in #[Handle] method "%s::%s()"',
+                            $propertyFetch->name instanceof Node\Identifier ? $propertyFetch->name->name : 'unknown',
+                            $this->scope->getClassReflection()?->getName() ?? 'unknown',
+                            $this->method->name->name
+                        ))
+                            ->identifier('myCustomRules.newPerson')
+                            ->build();
+                    }
+                }
+            }
+        };
+        $nodeFinder->addVisitor($visitor);
+        $nodeFinder->traverse($node->getStmts() ?? []);
+
+        return $visitor->errors;
+    }
+}
diff --git a/tests/Invalid/Profile.php b/tests/Invalid/Profile.php
index d521766..46d7dba 100644
--- a/tests/Invalid/Profile.php
+++ b/tests/Invalid/Profile.php
@@ -5,6 +5,7 @@
 use Patchlevel\EventSourcing\Aggregate\BasicAggregateRoot;
 use Patchlevel\EventSourcing\Aggregate\Uuid;
 use Patchlevel\EventSourcing\Attribute\Apply;
+use Patchlevel\EventSourcing\Attribute\Handle;
 use Patchlevel\EventSourcing\Attribute\Id;
 use Patchlevel\EventSourcingPHPStanExtension\Tests\Valid\ProfileCreated;
 
@@ -14,10 +15,13 @@ class Profile extends BasicAggregateRoot
     private Uuid $id;
     private string $name;
 
+    #[Handle]
     public static function create(Uuid $id, string $name): self
     {
         $self = new self();
         $self->recordThat(new ProfileCreated($id, $name));
+        $self->name = 'asd';
+        $self->changeStateHidden();
 
         return $self;
     }
@@ -36,6 +40,11 @@ public function hiddenRecordThat(ProfileCreated $event): void
         $this->recordThat(new ProfileCreated($event->id, $event->name));
     }
 
+    public function changeStateHidden(): void
+    {
+        $this->name = 'invalid';
+    }
+
     public function id(): Uuid
     {
         return $this->id;