[complexity] Add NoJustPropertyAssignRule (#177)

Author: TomasVotruba

README.md

@@ -272,6 +272,48 @@ usort($items, function (array $apples) {
 
 <br>
 
+### NoJustPropertyAssignRule
+
+Instead of assigning service property to a variable, use the property directly
+
+```yaml
+rules:
+    - Symplify\PHPStanRules\Rules\Complexity\NoJustPropertyAssignRule
+```
+
+```php
+class SomeClass
+{
+    // ...
+
+    public function run()
+    {
+        $someService = $this->someService;
+        $someService->run();
+    }
+}
+```
+
+:x:
+
+<br>
+
+```php
+class SomeClass
+{
+    // ...
+
+    public function run()
+    {
+        $this->someService->run();
+    }
+}
+```
+
+:+1:
+
+<br>
+
 ### ForbiddenExtendOfNonAbstractClassRule
 
 Only abstract classes can be extended

config/code-complexity-rules.neon

@@ -1,2 +1,3 @@
 rules:
     - Symplify\PHPStanRules\Rules\NoDynamicNameRule
+    - Symplify\PHPStanRules\Rules\Complexity\NoJustPropertyAssignRule

src/Enum/RuleIdentifier.php

@@ -67,4 +67,6 @@ final class RuleIdentifier
     public const MAXIMUM_IGNORED_ERROR_COUNT = 'symplify.maximumIgnoredErrorCount';
 
     public const STRING_FILE_ABSOLUTE_PATH_EXISTS = 'symplify.stringFileAbsolutePathExists';
+
+    public const NO_JUST_PROPERTY_ASSIGN = 'symplify.noJustPropertyAssign';
 }

src/Rules/Complexity/NoJustPropertyAssignRule.php

@@ -0,0 +1,93 @@
+<?php
+
+namespace Symplify\PHPStanRules\Rules\Complexity;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
+use Symfony\Component\Form\AbstractType;
+use Symplify\PHPStanRules\Enum\RuleIdentifier;
+
+/**
+ * @see \Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\NoJustPropertyAssignRuleTest
+ *
+ * @implements Rule<Assign>
+ */
+final class NoJustPropertyAssignRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'Instead of assigning service property to a variable, use the property directly';
+
+    public function getNodeType(): string
+    {
+        return Assign::class;
+    }
+
+    /**
+     * @param Assign $node
+     * @return RuleError[]
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! $this->isLocalPropertyFetchAssignToVariable($node, $scope)) {
+            return [];
+        }
+
+        if ($this->shouldSkipCurrentClass($scope)) {
+            return [];
+        }
+
+        return [RuleErrorBuilder::message(self::ERROR_MESSAGE)
+            ->identifier(RuleIdentifier::NO_JUST_PROPERTY_ASSIGN)
+            ->build()];
+    }
+
+    private function isLocalPropertyFetchAssignToVariable(Assign $assign, Scope $scope): bool
+    {
+        if (! $assign->var instanceof Variable) {
+            return false;
+        }
+
+        if (! $assign->expr instanceof PropertyFetch) {
+            return false;
+        }
+
+        $propertyFetch = $assign->expr;
+        if (! $propertyFetch->var instanceof Variable) {
+            return false;
+        }
+
+        if ($propertyFetch->var->name !== 'this') {
+            return false;
+        }
+
+        $exprType = $scope->getType($assign->expr);
+        return $exprType->isObject()->yes();
+    }
+
+    private function shouldSkipCurrentClass(Scope $scope): bool
+    {
+        // skip entities as rather static
+        if (str_contains($scope->getFile(), '/Document/') || str_contains($scope->getFile(), '/Entity/')) {
+            return true;
+        }
+
+        if ($scope->isInClass()) {
+            $classReflection = $scope->getClassReflection();
+
+            // skip Symfony form types as rather static
+            if ($classReflection->isSubclassOf(AbstractType::class)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

tests/Rules/Complexity/NoJustPropertyAssignRule/Fixture/ServiceAssign.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\Fixture;
+
+use Doctrine\Persistence\ObjectManager;
+
+final class ServiceAssign
+{
+    private ObjectManager $manager;
+
+    public function __construct(ObjectManager $manager)
+    {
+        $this->manager = $manager;
+    }
+
+    public function someMethod()
+    {
+        $manager = $this->manager;
+    }
+}

tests/Rules/Complexity/NoJustPropertyAssignRule/Fixture/SkipScalarAssign.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\Fixture;
+
+final class SkipScalarAssign
+{
+    private string $name;
+
+    public function __construct(string $name)
+    {
+        $this->name = $name;
+    }
+
+    public function someMethod()
+    {
+        $name = $this->name;
+    }
+}

tests/Rules/Complexity/NoJustPropertyAssignRule/NoJustPropertyAssignRuleTest.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule;
+
+use Iterator;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symplify\PHPStanRules\Rules\Complexity\NoJustPropertyAssignRule;
+
+final class NoJustPropertyAssignRuleTest extends RuleTestCase
+{
+    /**
+     * @param mixed[] $expectedErrorsWithLines
+     */
+    #[DataProvider('provideData')]
+    public function testRule(string $filePath, array $expectedErrorsWithLines): void
+    {
+        $this->analyse([$filePath], $expectedErrorsWithLines);
+    }
+
+    public static function provideData(): Iterator
+    {
+        yield [__DIR__ . '/Fixture/ServiceAssign.php', [
+            [NoJustPropertyAssignRule::ERROR_MESSAGE, 20],
+        ]];
+
+        yield [__DIR__ . '/Fixture/SkipScalarAssign.php', []];
+    }
+
+    protected function getRule(): Rule
+    {
+        return new NoJustPropertyAssignRule();
+    }
+}