feat: add NoEntityRepositoryInLoopRule to PHPStan rules

Author: shyim

rules.neon

@@ -13,6 +13,7 @@ rules:
     - Shopware\PhpStan\Rule\NoUserEntityGetStoreTokenRule
     - Shopware\PhpStan\Rule\ScheduledTaskTooLowIntervalRule
     - Shopware\PhpStan\Rule\SetForeignKeyRule
+    - Shopware\PhpStan\Rule\NoEntityRepositoryInLoopRule
 
 services:
     -

src/Rule/NoEntityRepositoryInLoopRule.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shopware\PhpStan\Rule;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Stmt\Foreach_;
+use PhpParser\Node\Stmt\For_;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Shopware\Core\Framework\DataAbstractionLayer\EntityRepository;
+
+/**
+ * @implements Rule<Node>
+ */
+class NoEntityRepositoryInLoopRule implements Rule
+{
+    public function getNodeType(): string
+    {
+        return Node::class;
+    }
+
+    /**
+     * @return list<IdentifierRuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (!$node instanceof For_ && !$node instanceof Foreach_) {
+            return [];
+        }
+
+        /** @var list<IdentifierRuleError> $errors */
+        $errors = [];
+        $this->checkNode($node, $scope, $errors);
+
+        return $errors;
+    }
+
+    /**
+     * @param list<IdentifierRuleError> $errors
+     */
+    private function checkNode(Node $node, Scope $scope, array &$errors): void
+    {
+        if ($node instanceof MethodCall) {
+            $callerType = $scope->getType($node->var);
+
+            if ($callerType->isObject()->yes() && in_array(EntityRepository::class, $callerType->getObjectClassNames(), true)) {
+                $errors[] = RuleErrorBuilder::message('EntityRepository method calls are not allowed within loops. This can lead to unexpected N:1 queries.')
+                    ->identifier('shopware.noEntityRepositoryInLoop')
+                    ->build();
+            }
+        }
+
+        foreach ($node->getSubNodeNames() as $subNodeName) {
+            $subNode = $node->$subNodeName;
+
+            if ($subNode instanceof Node) {
+                $this->checkNode($subNode, $scope, $errors);
+            }
+
+            if (is_array($subNode)) {
+                foreach ($subNode as $subSubNode) {
+                    if ($subSubNode instanceof Node) {
+                        $this->checkNode($subSubNode, $scope, $errors);
+                    }
+                }
+            }
+        }
+    }
+}

tests/Rule/NoEntityRepositoryInLoopRuleTest.php

@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shopware\PhpStan\Tests\Rule;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Shopware\PhpStan\Rule\NoEntityRepositoryInLoopRule;
+
+/**
+ * @extends RuleTestCase<NoEntityRepositoryInLoopRule>
+ */
+class NoEntityRepositoryInLoopRuleTest extends RuleTestCase
+{
+    protected function getRule(): Rule
+    {
+        return new NoEntityRepositoryInLoopRule();
+    }
+
+    public function testRule(): void
+    {
+        $this->analyse([__DIR__ . '/Fixtures/NoEntityRepositoryInLoop/NoEntityRepositoryInLoop.php'], [
+            [
+                'EntityRepository method calls are not allowed within loops. This can lead to unexpected N:1 queries.',
+                20,
+            ],
+            [
+                'EntityRepository method calls are not allowed within loops. This can lead to unexpected N:1 queries.',
+                28,
+            ],
+        ]);
+    }
+}

tests/Rule/fixtures/NoEntityRepositoryInLoop/NoEntityRepositoryInLoop.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shopware\PhpStan\Tests\Rule\Fixtures;
+
+use Shopware\Core\Framework\DataAbstractionLayer\EntityRepository;
+
+class NoEntityRepositoryInLoop
+{
+    private EntityRepository $repository;
+
+    public function __construct(EntityRepository $repository)
+    {
+        $this->repository = $repository;
+    }
+
+    public function badForLoop(): void
+    {
+        for ($i = 0; $i < 10; $i++) {
+            $this->repository->search(/* some criteria */);
+        }
+    }
+
+    public function badForeachLoop(): void
+    {
+        $items = ['a', 'b', 'c'];
+        foreach ($items as $item) {
+            $this->repository->search(/* some criteria */);
+        }
+    }
+
+    public function goodUsage(): void
+    {
+        // This is fine, not in a loop
+        $this->repository->search(/* some criteria */);
+    }
+}