Support @reqired annotation and #[Required] attribute for properties.

Prevent PropertyNotSetInConstructor when ContainerAwareTrait used.
psalm · Mar 24, 2021 · c786fdc · c786fdc
1 parent 8122414
commit c786fdc
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 3 deletions.
diff --git a/src/Handler/ContainerAwareTraitHandler.php b/src/Handler/ContainerAwareTraitHandler.php
@@ -0,0 +1,25 @@
+<?php
+
+namespace Psalm\SymfonyPsalmPlugin\Handler;
+
+use PhpParser\Node\Stmt\ClassLike;
+use Psalm\Codebase;
+use Psalm\FileSource;
+use Psalm\Plugin\Hook\AfterClassLikeVisitInterface;
+use Psalm\Storage\ClassLikeStorage;
+use Symfony\Component\DependencyInjection\ContainerAwareTrait;
+
+class ContainerAwareTraitHandler implements AfterClassLikeVisitInterface
+{
+    public static function afterClassLikeVisit(
+        ClassLike $stmt,
+        ClassLikeStorage $storage,
+        FileSource $statements_source,
+        Codebase $codebase,
+        array &$file_replacements = []
+    ) {
+        if (ContainerAwareTrait::class === $storage->name) {
+            $storage->initialized_properties['container'] = true;
+        }
+    }
+}
diff --git a/src/Handler/RequiredPropertyHandler.php b/src/Handler/RequiredPropertyHandler.php
@@ -0,0 +1,51 @@
+<?php
+
+namespace Psalm\SymfonyPsalmPlugin\Handler;
+
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassLike;
+use Psalm\Codebase;
+use Psalm\FileSource;
+use Psalm\Plugin\Hook\AfterClassLikeVisitInterface;
+use Psalm\Storage\ClassLikeStorage;
+
+class RequiredPropertyHandler implements AfterClassLikeVisitInterface
+{
+    public static function afterClassLikeVisit(
+        ClassLike $stmt,
+        ClassLikeStorage $storage,
+        FileSource $statements_source,
+        Codebase $codebase,
+        array &$file_replacements = []
+    ) {
+        if (!$stmt instanceof Class_) {
+            return;
+        }
+        $reflection = null;
+        foreach ($storage->properties as $name => $property) {
+            if (!empty($storage->initialized_properties[$name])) {
+                continue;
+            }
+            foreach ($property->attributes as $attribute) {
+                if ('Symfony\Contracts\Service\Attribute\Required' === $attribute->fq_class_name) {
+                    $storage->initialized_properties[$name] = true;
+                    continue 2;
+                }
+            }
+            $class = $storage->name;
+            if (!class_exists($class)) {
+                /** @psalm-suppress UnresolvableInclude */
+                require_once $statements_source->getRootFilePath();
+            }
+            /** @psalm-suppress ArgumentTypeCoercion */
+            $reflection = $reflection ?? new \ReflectionClass($class);
+            if ($reflection->hasProperty($name)) {
+                $reflectionProperty = $reflection->getProperty($name);
+                $docCommend = $reflectionProperty->getDocComment();
+                if ($docCommend && false !== strpos(strtoupper($docCommend), '@REQUIRED')) {
+                    $storage->initialized_properties[$name] = true;
+                }
+            }
+        }
+    }
+}
diff --git a/src/Plugin.php b/src/Plugin.php
@@ -9,11 +9,13 @@
 use Psalm\Plugin\RegistrationInterface;
 use Psalm\SymfonyPsalmPlugin\Handler\AnnotationHandler;
 use Psalm\SymfonyPsalmPlugin\Handler\ConsoleHandler;
+use Psalm\SymfonyPsalmPlugin\Handler\ContainerAwareTraitHandler;
 use Psalm\SymfonyPsalmPlugin\Handler\ContainerDependencyHandler;
 use Psalm\SymfonyPsalmPlugin\Handler\ContainerHandler;
 use Psalm\SymfonyPsalmPlugin\Handler\DoctrineQueryBuilderHandler;
 use Psalm\SymfonyPsalmPlugin\Handler\DoctrineRepositoryHandler;
 use Psalm\SymfonyPsalmPlugin\Handler\HeaderBagHandler;
+use Psalm\SymfonyPsalmPlugin\Handler\RequiredPropertyHandler;
 use Psalm\SymfonyPsalmPlugin\Handler\RequiredSetterHandler;
 use Psalm\SymfonyPsalmPlugin\Symfony\ContainerMeta;
 use Psalm\SymfonyPsalmPlugin\Twig\AnalyzedTemplatesTainter;
@@ -60,13 +62,17 @@ public function __invoke(RegistrationInterface $api, SimpleXMLElement $config =
         require_once __DIR__.'/Handler/HeaderBagHandler.php';
         require_once __DIR__.'/Handler/ContainerHandler.php';
         require_once __DIR__.'/Handler/ConsoleHandler.php';
+        require_once __DIR__.'/Handler/ContainerAwareTraitHandler.php';
         require_once __DIR__.'/Handler/ContainerDependencyHandler.php';
+        require_once __DIR__.'/Handler/RequiredPropertyHandler.php';
         require_once __DIR__.'/Handler/RequiredSetterHandler.php';
         require_once __DIR__.'/Handler/DoctrineQueryBuilderHandler.php';
 
         $api->registerHooksFromClass(HeaderBagHandler::class);
         $api->registerHooksFromClass(ConsoleHandler::class);
+        $api->registerHooksFromClass(ContainerAwareTraitHandler::class);
         $api->registerHooksFromClass(ContainerDependencyHandler::class);
+        $api->registerHooksFromClass(RequiredPropertyHandler::class);
         $api->registerHooksFromClass(RequiredSetterHandler::class);
 
         if (class_exists(\Doctrine\ORM\QueryBuilder::class)) {

diff --git a/tests/acceptance/acceptance/PropertyAccessorInterface.feature b/tests/acceptance/acceptance/PropertyAccessorInterface.feature
@@ -41,7 +41,10 @@ Feature: PropertyAccessorInterface
       """
       class Company
       {
-          public string $name = 'Acme';
+          /**
+           * @var string
+           */
+          public $name = 'Acme';
       }
       $company = new Company();
 

diff --git a/tests/acceptance/acceptance/RequiredAttribute.feature b/tests/acceptance/acceptance/RequiredAttribute.feature
@@ -0,0 +1,57 @@
+@symfony-common
+Feature: RequiredAttribute
+
+  Background:
+    Given I have the following config
+      """
+      <?xml version="1.0"?>
+      <psalm errorLevel="1">
+        <projectFiles>
+          <directory name="."/>
+          <ignoreFiles> <directory name="../../vendor"/> </ignoreFiles>
+        </projectFiles>
+
+        <plugins>
+          <pluginClass class="Psalm\SymfonyPsalmPlugin\Plugin">
+            <containerXml>../../tests/acceptance/container.xml</containerXml>
+          </pluginClass>
+        </plugins>
+      </psalm>
+      """
+
+  Scenario: PropertyNotSetInConstructor error is not raised when the @required annotation is present.
+    Given I have the following code
+      """
+      <?php
+
+      class MyServiceA {
+          /**
+           * @required
+           * @var string
+           */
+          public $a;
+          public function __construct(){}
+      }
+      """
+    When I run Psalm
+    Then I see no errors
+
+  Scenario: PropertyNotSetInConstructor error is raised when the @required annotation is not present.
+    Given I have the following code
+      """
+      <?php
+
+      class MyServiceC {
+          /**
+           * @var string
+           */
+          public $a;
+          public function __construct(){}
+
+      }
+      """
+    When I run Psalm
+    Then I see these errors
+      | Type                        | Message                                                                                                          |
+      | PropertyNotSetInConstructor | Property MyServiceC::$a is not defined in constructor of MyServiceC and in any methods called in the constructor |
+    And I see no other errors
diff --git a/tests/acceptance/acceptance/RequiredSetter.feature b/tests/acceptance/acceptance/RequiredSetter.feature
@@ -27,7 +27,8 @@ Feature: Annotation class
       }
 
       final class MyServiceB {
-          private MyServiceA $a;
+          /** @var MyServiceA */
+          private $a;
           public function __construct(){}
 
           /** @required */
@@ -45,7 +46,8 @@ Feature: Annotation class
       }
 
       final class MyServiceB {
-          private MyServiceA $a;
+          /** @var MyServiceA */
+          private $a;
           public function __construct(){}
 
           private function setMyServiceA(MyServiceA $a): void { $this->a = $a; }