Rework the rule: DontRecordWhenApplying

docs/getting-started.md

@@ -101,11 +101,11 @@ final class Profile extends BasicAggregateRoot
 Running PHPStan now produces:
 
 ```
-Method Patchlevel\EventSourcing\Aggregate\AggregateRoot::recordThat() is called
-from applyProfileCreated which is an apply method.
+Method Patchlevel\EventSourcing\Aggregate\BasicAggregateRoot::recordThat() records
+an event and is called from apply method applyProfileCreated().
 ```
-:::info
-The check also follows calls into helper methods, so hiding `recordThat()` behind another method does not bypass the rule.
+:::note
+The check follows calls into helper methods, even across multiple levels and into traits, so hiding `recordThat()` behind other methods does not bypass the rule. It covers child aggregates as well. Calling a `recordThat()` method on an unrelated object, for example a collaborator service, is not reported.
 :::
 
 ## Result

extension.neon

@@ -6,5 +6,7 @@ services:
 
 	-
 		class: Patchlevel\EventSourcingPHPStanExtension\DontRecordWhenApplyingExtension
+		arguments:
+			parser: @defaultAnalysisParser
 		tags:
 			- phpstan.restrictedMethodUsageExtension

phpstan-baseline.neon

@@ -1,13 +1,31 @@
 parameters:
 	ignoreErrors:
 		-
-			rawMessage: 'Method Patchlevel\EventSourcing\Aggregate\AggregateRoot::recordThat() is called from applyNameChanged which is an apply method.'
+			rawMessage: 'Method Patchlevel\EventSourcing\Aggregate\BasicChildAggregate::recordThat() records an event and is called from apply method applyNameChanged().'
+			identifier: patchlevel.noRecordThatWhenApplying
+			count: 1
+			path: tests/Invalid/PersonalInformation.php
+
+		-
+			rawMessage: 'Method Patchlevel\EventSourcingPHPStanExtension\Tests\Invalid\Profile::deeplyHiddenRecordThat() records an event and is called from apply method applyNameChanged().'
+			identifier: patchlevel.noRecordThatWhenApplying
+			count: 1
+			path: tests/Invalid/Profile.php
+
+		-
+			rawMessage: 'Method Patchlevel\EventSourcingPHPStanExtension\Tests\Invalid\Profile::hiddenRecordThat() records an event and is called from apply method applyNameChanged().'
+			identifier: patchlevel.noRecordThatWhenApplying
+			count: 1
+			path: tests/Invalid/Profile.php
+
+		-
+			rawMessage: 'Method Patchlevel\EventSourcingPHPStanExtension\Tests\Invalid\Profile::traitHiddenRecordThat() records an event and is called from apply method applyNameChanged().'
 			identifier: patchlevel.noRecordThatWhenApplying
 			count: 1
 			path: tests/Invalid/Profile.php
 
 		-
-			rawMessage: 'Method Patchlevel\EventSourcing\Aggregate\AggregateRoot::recordThat() is called from applyProfileCreated which is an apply method.'
+			rawMessage: 'Method Patchlevel\EventSourcing\Aggregate\BasicAggregateRoot::recordThat() records an event and is called from apply method applyProfileCreated().'
 			identifier: patchlevel.noRecordThatWhenApplying
 			count: 1
 			path: tests/Invalid/Profile.php

src/DontRecordWhenApplyingExtension.php

@@ -5,41 +5,49 @@
 namespace Patchlevel\EventSourcingPHPStanExtension;
 
 use Patchlevel\EventSourcing\Aggregate\AggregateRoot;
+use Patchlevel\EventSourcing\Aggregate\ChildAggregate;
 use Patchlevel\EventSourcing\Attribute\Apply;
+use PhpParser\Node;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\NodeFinder;
-use PhpParser\ParserFactory;
 use PHPStan\Analyser\Scope;
+use PHPStan\Parser\Parser;
+use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\ExtendedMethodReflection;
 use PHPStan\Reflection\FunctionReflection;
 use PHPStan\Reflection\MethodReflection;
 use PHPStan\Rules\RestrictedUsage\RestrictedMethodUsageExtension;
 use PHPStan\Rules\RestrictedUsage\RestrictedUsage;
 
-use function file_get_contents;
+use function array_key_exists;
+use function in_array;
 use function sprintf;
+use function strtolower;
 
 final class DontRecordWhenApplyingExtension implements RestrictedMethodUsageExtension
 {
     private NodeFinder $nodeFinder;
-    private ParserFactory $parserFactory;
 
     /** @var array<string, bool> */
-    private array $recordThatCallCache = [];
+    private array $recordsEventCache = [];
 
-    public function __construct()
+    public function __construct(private readonly Parser $parser)
     {
         $this->nodeFinder = new NodeFinder();
-        $this->parserFactory = new ParserFactory();
     }
 
     public function isRestrictedMethodUsage(
         ExtendedMethodReflection $methodReflection,
         Scope $scope,
     ): RestrictedUsage|null {
-        if (!$methodReflection->getDeclaringClass()->implementsInterface(AggregateRoot::class)) {
+        $declaringClass = $methodReflection->getDeclaringClass();
+
+        if (!$this->isAggregate($declaringClass)) {
             return null;
         }
 
@@ -53,20 +61,29 @@ public function isRestrictedMethodUsage(
             return null;
         }
 
-        if (!$this->callsRecordThat($methodReflection)) {
+        $visited = [];
+
+        if (!$this->recordsEvent($declaringClass, $methodReflection->getName(), $visited)) {
             return null;
         }
 
         return RestrictedUsage::create(
             errorMessage: sprintf(
-                'Method %s::recordThat() is called from %s which is an apply method.',
-                AggregateRoot::class,
-                $function->getName() ?? 'unknown',
+                'Method %s::%s() records an event and is called from apply method %s().',
+                $declaringClass->getName(),
+                $methodReflection->getName(),
+                $function->getName(),
             ),
             identifier: 'patchlevel.noRecordThatWhenApplying',
         );
     }
 
+    private function isAggregate(ClassReflection $classReflection): bool
+    {
+        return $classReflection->implementsInterface(AggregateRoot::class)
+            || $classReflection->implementsInterface(ChildAggregate::class);
+    }
+
     private function isApplyMethod(FunctionReflection|MethodReflection $function): bool
     {
         foreach ($function->getAttributes() as $attribute) {
@@ -78,55 +95,120 @@ private function isApplyMethod(FunctionReflection|MethodReflection $function): b
         return false;
     }
 
-    private function callsRecordThat(ExtendedMethodReflection $methodReflection): bool
+    /** @param array<string, true> $visited */
+    private function recordsEvent(ClassReflection $classReflection, string $methodName, array &$visited): bool
     {
-        if ($methodReflection->getName() === 'recordThat') {
+        if ($methodName === 'recordThat') {
             return true;
         }
 
-        $declaringClass = $methodReflection->getDeclaringClass();
-        $fileName = $declaringClass->getFileName();
+        $cacheKey = sprintf('%s::%s', $classReflection->getName(), $methodName);
 
-        if ($fileName === null || $fileName === false) {
+        if (array_key_exists($cacheKey, $this->recordsEventCache)) {
+            return $this->recordsEventCache[$cacheKey];
+        }
+
+        if (array_key_exists($cacheKey, $visited)) {
             return false;
         }
 
-        $cacheKey = sprintf('%s::%s', $declaringClass->getName(), $methodReflection->getName());
+        $visited[$cacheKey] = true;
+
+        $methodNode = $this->findMethodNode($classReflection, $methodName);
+        $result = false;
 
-        if (isset($this->recordThatCallCache[$cacheKey])) {
-            return $this->recordThatCallCache[$cacheKey];
+        if ($methodNode?->stmts !== null) {
+            foreach ($this->calledMethodNames($methodNode) as $calledMethodName) {
+                if ($this->recordsEvent($classReflection, $calledMethodName, $visited)) {
+                    $result = true;
+                    break;
+                }
+            }
         }
 
-        $parser = $this->parserFactory->createForNewestSupportedVersion();
-        $ast = $parser->parse((string)file_get_contents($fileName));
+        $this->recordsEventCache[$cacheKey] = $result;
 
-        if ($ast === null) {
-            $this->recordThatCallCache[$cacheKey] = false;
+        return $result;
+    }
 
-            return false;
+    private function findMethodNode(ClassReflection $classReflection, string $methodName): ClassMethod|null
+    {
+        $nativeClass = $classReflection->getNativeReflection();
+
+        if (!$nativeClass->hasMethod($methodName)) {
+            return null;
         }
 
-        $methodNode = $this->nodeFinder->findFirst(
-            $ast,
-            static fn (mixed $node): bool => $node instanceof ClassMethod
-                && $node->name->toString() === $methodReflection->getName(),
+        $nativeMethod = $nativeClass->getMethod($methodName);
+        $fileName = $nativeMethod->getFileName();
+
+        if ($fileName === false) {
+            return null;
+        }
+
+        $candidates = $this->nodeFinder->find(
+            $this->parser->parseFile($fileName),
+            static fn (Node $node): bool => $node instanceof ClassMethod
+                && $node->name->toString() === $nativeMethod->getName(),
         );
 
-        if (!$methodNode instanceof ClassMethod || $methodNode->stmts === null) {
-            $this->recordThatCallCache[$cacheKey] = false;
+        if ($candidates === []) {
+            return null;
+        }
 
-            return false;
+        foreach ($candidates as $candidate) {
+            if ($candidate->getStartLine() === $nativeMethod->getStartLine()) {
+                return $candidate instanceof ClassMethod ? $candidate : null;
+            }
         }
 
-        $result = $this->nodeFinder->findFirst(
-            $methodNode->stmts,
-            static fn (mixed $node): bool => $node instanceof MethodCall
-                && $node->name instanceof Identifier
-                && $node->name->toString() === 'recordThat',
-        ) !== null;
+        $candidate = $candidates[0];
 
-        $this->recordThatCallCache[$cacheKey] = $result;
+        return $candidate instanceof ClassMethod ? $candidate : null;
+    }
 
-        return $result;
+    /**
+     * Collects the names of all methods the given method calls on its own
+     * instance ($this->, self::, static::, parent::). Calls on other objects
+     * are ignored, their receiver type cannot be resolved without a scope.
+     *
+     * @return list<string>
+     */
+    private function calledMethodNames(ClassMethod $methodNode): array
+    {
+        $calls = $this->nodeFinder->find(
+            $methodNode->stmts ?? [],
+            static function (Node $node): bool {
+                if ($node instanceof MethodCall) {
+                    return $node->var instanceof Variable
+                        && $node->var->name === 'this'
+                        && $node->name instanceof Identifier;
+                }
+
+                if ($node instanceof StaticCall) {
+                    return $node->class instanceof Name
+                        && in_array(strtolower($node->class->toString()), ['self', 'static', 'parent'], true)
+                        && $node->name instanceof Identifier;
+                }
+
+                return false;
+            },
+        );
+
+        $names = [];
+
+        foreach ($calls as $call) {
+            if (!$call instanceof MethodCall && !$call instanceof StaticCall) {
+                continue;
+            }
+
+            if (!$call->name instanceof Identifier) {
+                continue;
+            }
+
+            $names[] = $call->name->toString();
+        }
+
+        return $names;
     }
 }

tests/Invalid/PersonalInformation.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Patchlevel\EventSourcingPHPStanExtension\Tests\Invalid;
+
+use Patchlevel\EventSourcing\Aggregate\BasicChildAggregate;
+use Patchlevel\EventSourcing\Attribute\Apply;
+use Patchlevel\EventSourcingPHPStanExtension\Tests\Valid\NameChanged;
+
+class PersonalInformation extends BasicChildAggregate
+{
+    private string $name;
+
+    #[Apply]
+    protected function applyNameChanged(NameChanged $event): void
+    {
+        $this->name = $event->name;
+        $this->recordThat(new NameChanged($event->name));
+    }
+
+    public function name(): string
+    {
+        return $this->name;
+    }
+}

tests/Invalid/Profile.php

@@ -6,14 +6,18 @@
 use Patchlevel\EventSourcing\Aggregate\Uuid;
 use Patchlevel\EventSourcing\Attribute\Apply;
 use Patchlevel\EventSourcing\Attribute\Id;
+use Patchlevel\EventSourcingPHPStanExtension\Tests\Valid\EventCollector;
 use Patchlevel\EventSourcingPHPStanExtension\Tests\Valid\NameChanged;
 use Patchlevel\EventSourcingPHPStanExtension\Tests\Valid\ProfileCreated;
 
 class Profile extends BasicAggregateRoot
 {
+    use RecordingHelper;
+
     #[Id]
     private Uuid $id;
     private string $name;
+    private EventCollector $collector;
 
     public static function create(Uuid $id, string $name): self
     {
@@ -28,6 +32,7 @@ protected function applyProfileCreated(ProfileCreated $event): void
     {
         $this->id = $event->id;
         $this->name = $event->name;
+        $this->collector = new EventCollector();
         $this->recordThat(new ProfileCreated($event->id, $event->name));
     }
 
@@ -36,13 +41,26 @@ protected function applyNameChanged(NameChanged $event): void
     {
         $this->name = $event->name;
         $this->hiddenRecordThat($event);
+        $this->deeplyHiddenRecordThat($event);
+        $this->traitHiddenRecordThat($event);
+        $this->collectEvent($event);
     }
 
     public function hiddenRecordThat(NameChanged $event): void
     {
         $this->recordThat(new NameChanged($event->name));
     }
 
+    public function deeplyHiddenRecordThat(NameChanged $event): void
+    {
+        $this->hiddenRecordThat($event);
+    }
+
+    public function collectEvent(NameChanged $event): void
+    {
+        $this->collector->recordThat($event);
+    }
+
     public function id(): Uuid
     {
         return $this->id;

tests/Invalid/RecordingHelper.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Patchlevel\EventSourcingPHPStanExtension\Tests\Invalid;
+
+use Patchlevel\EventSourcingPHPStanExtension\Tests\Valid\NameChanged;
+
+trait RecordingHelper
+{
+    public function traitHiddenRecordThat(NameChanged $event): void
+    {
+        $this->recordThat(new NameChanged($event->name));
+    }
+}