add no getDoctrine() rule (#159)

* add no getDoctrine() rule * misc
symplify · Dec 22, 2024 · 6902f4a · 6902f4a
1 parent 83cf086
commit 6902f4a
Show file tree

Hide file tree

Showing 8 changed files with 124 additions and 166 deletions.
diff --git a/config/symfony-rules.neon b/config/symfony-rules.neon
@@ -5,3 +5,7 @@ rules:
     - Symplify\PHPStanRules\Rules\Symfony\NoListenerWithoutContractRule
     - Symplify\PHPStanRules\Rules\Symfony\NoStringInGetSubscribedEventsRule
     - Symplify\PHPStanRules\Rules\Symfony\RequireInvokableControllerRule
+
+    # dependency injection
+    - Symplify\PHPStanRules\Rules\Symfony\NoGetDoctrineInControllerRule
+    - Symplify\PHPStanRules\Rules\Symfony\NoGetInControllerRule
diff --git a/src/Enum/RuleIdentifier.php b/src/Enum/RuleIdentifier.php
@@ -6,169 +6,70 @@
 
 final class RuleIdentifier
 {
-    /**
-     * @var string
-     */
     public const UPPERCASE_CONSTANT = 'symplify.uppercaseConstant';
 
-    /**
-     * @var string
-     */
     public const SEE_ANNOTATION_TO_TEST = 'symplify.seeAnnotationToTest';
 
-    /**
-     * @var string
-     */
     public const REQUIRE_ATTRIBUTE_NAME = 'symplify.requireAttributeName';
 
-    /**
-     * @var string
-     */
     public const RECTOR_PHP_RULE_IMPLEMENTS_MIN_VERSION = 'rector.phpRuleImplementsMinVersion';
 
-    /**
-     * @var string
-     */
     public const RECTOR_UPGRADE_DOWNGRADE_REGISTERED_IN_SET = 'rector.upgradeDowngradeRegisteredInSet';
 
-    /**
-     * @var string
-     */
     public const PHP_PARSER_NO_LEADING_BACKSLASH_IN_NAME = 'phpParser.noLeadingBackslashInName';
 
-    /**
-     * @var string
-     */
     public const RECTOR_NO_INSTANCE_OF_STATIC_REFLECTION = 'rector.noInstanceOfStaticReflection';
 
-    /**
-     * @var string
-     */
     public const RECTOR_NO_CLASS_REFLECTION_STATIC_REFLECTION = 'rector.noClassReflectionStaticReflection';
 
-    /**
-     * @var string
-     */
     public const PARENT_METHOD_VISIBILITY_OVERRIDE = 'symplify.parentMethodVisibilityOverride';
 
-    /**
-     * @var string
-     */
     public const NO_RETURN_SETTER_METHOD = 'symplify.noReturnSetterMethod';
 
-    /**
-     * @var string
-     */
     public const FORBIDDEN_STATIC_CLASS_CONST_FETCH = 'symplify.forbiddenStaticClassConstFetch';
 
-    /**
-     * @var string
-     */
     public const PREFERRED_CLASS = 'symplify.preferredClass';
 
-    /**
-     * @var string
-     */
     public const NO_TEST_MOCKS = 'symplify.noTestMocks';
 
-    /**
-     * @var string
-     */
     public const NO_GLOBAL_CONST = 'symplify.noGlobalConst';
 
-    /**
-     * @var string
-     */
     public const NO_ENTITY_OUTSIDE_ENTITY_NAMESPACE = 'symplify.noEntityOutsideEntityNamespace';
 
-    /**
-     * @var string
-     */
     public const FORBIDDEN_NODE = 'symplify.forbiddenNode';
 
-    /**
-     * @var string
-     */
     public const MULTIPLE_CLASS_LIKE_IN_FILE = 'symplify.multipleClassLikeInFile';
 
-    /**
-     * @var string
-     */
     public const FORBIDDEN_FUNC_CALL = 'symplify.forbiddenFuncCall';
 
-    /**
-     * @var string
-     */
     public const REQUIRE_ATTRIBUTE_NAMESPACE = 'symplify.requireAttributeNamespace';
 
-    /**
-     * @var string
-     */
     public const FORBIDDEN_ARRAY_METHOD_CALL = 'symplify.forbiddenArrayMethodCall';
 
-    /**
-     * @var string
-     */
     public const FORBIDDEN_EXTEND_OF_NON_ABSTRACT_CLASS = 'symplify.forbiddenExtendOfNonAbstractClass';
 
-    /**
-     * @var string
-     */
     public const EXPLICIT_ABSTRACT_PREFIX_NAME = 'symplify.explicitAbstractPrefixName';
 
-    /**
-     * @var string
-     */
     public const EXPLICIT_INTERFACE_SUFFIX_NAME = 'symplify.explicitInterfaceSuffixName';
 
-    /**
-     * @var string
-     */
     public const EXPLICIT_TRAIT_SUFFIX_NAME = 'symplify.explicitTraitSuffixName';
 
-    /**
-     * @var string
-     */
     public const REQUIRE_UNIQUE_ENUM_CONSTANT = 'symplify.requireUniqueEnumConstant';
 
-    /**
-     * @var string
-     */
     public const REQUIRE_EXCEPTION_NAMESPACE = 'symplify.requireExceptionNamespace';
 
-    /**
-     * @var string
-     */
     public const CLASS_NAME_RESPECTS_PARENT_SUFFIX = 'symplify.classNameRespectsParentSuffix';
 
-    /**
-     * @var string
-     */
     public const REQUIRED_INTERFACE_CONTRACT_NAMESPACE = 'symplify.requiredInterfaceContractNamespace';
 
-    /**
-     * @var string
-     */
     public const SYMFONY_REQUIRE_INVOKABLE_CONTROLLER = 'symfony.requireInvokableController';
 
-    /**
-     * @var string
-     */
     public const NO_VALUE_OBJECT_IN_SERVICE_CONSTRUCTOR = 'symplify.noValueObjectInServiceConstructor';
 
-    /**
-     * @var string
-     */
     public const DOCTRINE_NO_REPOSITORY_CALL_IN_DATA_FIXTURES = 'doctrine.noRepositoryCallInDataFixtures';
 
-    /**
-     * @var string
-     */
     public const PHPUNIT_NO_DOCUMENT_MOCKING = 'phpunit.noDocumentMocking';
 
-    /**
-     * @var string
-     */
     public const NO_DYNAMIC_NAME = 'symplify.noDynamicName';
 
     public const NO_REFERENCE = 'symplify.noReference';
@@ -200,4 +101,6 @@ final class RuleIdentifier
     public const REQUIRE_QUERY_BUILDER_ON_REPOSITORY = 'doctrine.requireQueryBuilderOnRepository';
 
     public const NO_GET_IN_CONTROLLER = 'symfony.noGetInController';
+
+    public const NO_GET_DOCTRINE_IN_CONTROLLER = 'symfony.noGetDoctrineInController';
 }
diff --git a/src/NodeAnalyzer/MethodCallNameAnalyzer.php b/src/NodeAnalyzer/MethodCallNameAnalyzer.php
@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\NodeAnalyzer;
+
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+
+final class MethodCallNameAnalyzer
+{
+    public static function isThisMethodCall(MethodCall $methodCall, string $methodName): bool
+    {
+        if (! $methodCall->name instanceof Identifier) {
+            return false;
+        }
+
+        if ($methodCall->name->toString() !== $methodName) {
+            return false;
+        }
+
+        // is "$this"?
+        if (! $methodCall->var instanceof Variable) {
+            return false;
+        }
+
+        if (! is_string($methodCall->var->name)) {
+            return false;
+        }
+
+        return $methodCall->var->name === 'this';
+    }
+}
diff --git a/src/Rules/Doctrine/NoEntityMockingRule.php b/src/Rules/Doctrine/NoEntityMockingRule.php
@@ -13,6 +13,7 @@
 use PHPStan\Rules\RuleErrorBuilder;
 use Symplify\PHPStanRules\Doctrine\DoctrineEntityDocumentAnalyser;
 use Symplify\PHPStanRules\Enum\RuleIdentifier;
+use Symplify\PHPStanRules\NodeAnalyzer\MethodCallNameAnalyzer;
 
 /**
  * The ORM entities and ODM documents should never be mocked, as it leads to typeless code.
@@ -44,7 +45,7 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if (! $this->isCreateMockMethod($node)) {
+        if (! MethodCallNameAnalyzer::isThisMethodCall($node, 'createMock')) {
             return [];
         }
 
@@ -70,18 +71,4 @@ public function processNode(Node $node, Scope $scope): array
 
         return [];
     }
-
-    private function isCreateMockMethod(MethodCall $methodCall): bool
-    {
-        if ($methodCall->isFirstClassCallable()) {
-            return false;
-        }
-
-        if (! $methodCall->name instanceof Identifier) {
-            return false;
-        }
-
-        $methodName = $methodCall->name->toString();
-        return $methodName === 'createMock';
-    }
 }
diff --git a/src/Rules/Symfony/NoAbstractControllerConstructorRule.php b/src/Rules/Symfony/NoAbstractControllerConstructorRule.php
@@ -16,7 +16,7 @@
  * Check if abstract controller has constructor, as it should use
  * #[Require] instead to avoid parent constructor override
  *
- * @see \Symplify\PHPStanRules\Tests\PHPStan\Rule\NoAbstractControllerConstructorRule\NoAbstractControllerConstructorRuleTest
+ * @see \Symplify\PHPStanRules\Tests\Rules\Symfony\NoAbstractControllerConstructorRule\NoAbstractControllerConstructorRuleTest
  *
  * @implements Rule<Class_>
  */

diff --git a/src/Rules/Symfony/NoGetDoctrineInControllerRule.php b/src/Rules/Symfony/NoGetDoctrineInControllerRule.php
@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Symfony;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Symplify\PHPStanRules\Enum\RuleIdentifier;
+use Symplify\PHPStanRules\NodeAnalyzer\MethodCallNameAnalyzer;
+use Symplify\PHPStanRules\Symfony\NodeAnalyzer\SymfonyControllerAnalyzer;
+
+/**
+ * @implements Rule<MethodCall>
+ */
+final class NoGetDoctrineInControllerRule implements Rule
+{
+    /**
+     * @var string
+     */
+    private const ERROR_MESSAGE = 'Do not use $this->getDoctrine() method in controller. Use __construct(EntityManagerInterface $entityManager) instead';
+
+    public function getNodeType(): string
+    {
+        return MethodCall::class;
+    }
+
+    /**
+     * @param MethodCall $node
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! MethodCallNameAnalyzer::isThisMethodCall($node, 'getDoctrine')) {
+            return [];
+        }
+
+        if (! SymfonyControllerAnalyzer::isControllerScope($scope)) {
+            return [];
+        }
+
+        $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
+            ->file($scope->getFile())
+            ->line($node->getStartLine())
+            ->identifier(RuleIdentifier::NO_GET_DOCTRINE_IN_CONTROLLER)
+            ->build();
+
+        return [$ruleError];
+    }
+}