Merge pull request #140 from webbuilders-group/feature-modern-reflect…

…ion-docblock Move to a modern version of reflection docblock
silverleague · Aug 6, 2022 · 370bf4e · 370bf4e
2 parents 6387bc9 + 6352163
commit 370bf4e
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 26 deletions.
diff --git a/README.md b/README.md
@@ -86,4 +86,6 @@ For further documentation information, see the [docs](docs/en/Index.md)
 This module changes the content of your files and currently there is no backup functionality. PHPStorm has a Local history for files and of course you have your code version controlled...
 I tried to add complete UnitTests, but I can't garantuee every situation is covered.
 
+Windows users should be aware that the PHP Docs are generated with PSR in mind and use \n for line endings rather than Window's \r\n, some editors may have a hard time with these line endings.
+
 This module should **never** be installed on a production environment.
diff --git a/composer.json b/composer.json
@@ -22,9 +22,9 @@
         }
     ],
     "require": {
-        "php": ">=5.6.0",
+        "php": "^7.2 || ^8.0",
         "silverstripe/framework": "^4",
-        "phpdocumentor/reflection-docblock": "^2.0@dev"
+        "phpdocumentor/reflection-docblock": "^5.2"
     },
     "require-dev": {
         "scriptfusion/phpunit-immediate-exception-printer": "^1",

diff --git a/src/Generators/AbstractTagGenerator.php b/src/Generators/AbstractTagGenerator.php
@@ -2,15 +2,21 @@
 
 namespace SilverLeague\IDEAnnotator\Generators;
 
-use Generator;
+use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
+use phpDocumentor\Reflection\DocBlock\StandardTagFactory;
 use phpDocumentor\Reflection\DocBlock\Tag;
-use ReflectionClass;
-use ReflectionException;
+use phpDocumentor\Reflection\DocBlockFactory;
+use phpDocumentor\Reflection\FqsenResolver;
+use phpDocumentor\Reflection\TypeResolver;
 use SilverLeague\IDEAnnotator\DataObjectAnnotator;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Extension;
 use SilverStripe\Core\Injector\Injector;
+use Generator;
+use ReflectionClass;
+use ReflectionException;
+use SilverLeague\IDEAnnotator\Reflection\ShortNameResolver;
 
 /**
  * AbstractTagGenerator
@@ -43,6 +49,13 @@ abstract class AbstractTagGenerator
      */
     protected $tags = [];
 
+    /**
+     * @var StandardTagFactory
+     */
+    protected $tagFactory;
+
+    protected static $pageClassesCache = [];
+
     /**
      * DocBlockTagGenerator constructor.
      *
@@ -57,6 +70,18 @@ public function __construct($className, $existingTags)
         $this->reflector = new ReflectionClass($className);
         $this->tags = $this->getSupportedTagTypes();
 
+        //Init the tag factory
+        if (DataObjectAnnotator::config()->get('use_short_name')) {
+            $fqsenResolver = new ShortNameResolver();
+        } else {
+            $fqsenResolver = new FqsenResolver();
+        }
+
+        $this->tagFactory = new StandardTagFactory($fqsenResolver);
+        $descriptionFactory = new DescriptionFactory($this->tagFactory);
+        $this->tagFactory->addService($descriptionFactory);
+        $this->tagFactory->addService(new TypeResolver($fqsenResolver));
+
         $this->generateTags();
     }
 
@@ -143,9 +168,10 @@ protected function pushMixinTag($tagString)
      */
     protected function pushTagWithExistingComment($type, $tagString)
     {
+        $tagString = sprintf('@%s %s', $type, $tagString);
         $tagString .= $this->getExistingTagCommentByTagString($tagString);
 
-        return new Tag($type, $tagString);
+        return $this->tagFactory->create($tagString);
     }
 
     /**
@@ -155,7 +181,7 @@ protected function pushTagWithExistingComment($type, $tagString)
     public function getExistingTagCommentByTagString($tagString)
     {
         foreach ($this->getExistingTags() as $tag) {
-            $content = $tag->getContent();
+            $content = $tag->__toString();
             // A tag should be followed by a space before it's description
             // This is to prevent `TestThing` and `Test` to be seen as the same, when the shorter
             // is after the longer name
@@ -191,7 +217,7 @@ protected function generateOwnerTags()
         if (Injector::inst()->get($this->className) instanceof Extension) {
             $owners = iterator_to_array($this->getOwnerClasses($className));
             $owners[] = $this->className;
-            $tagString = '\\' . implode("|\\", array_values($owners)) . ' $owner';
+            $tagString = sprintf('\\%s $owner', implode("|\\", array_values($owners)));
             if (DataObjectAnnotator::config()->get('use_short_name')) {
                 foreach ($owners as $key => $owner) {
                     $owners[$key] = $this->getAnnotationClassName($owner);

diff --git a/src/Generators/ControllerTagGenerator.php b/src/Generators/ControllerTagGenerator.php
@@ -2,13 +2,27 @@
 
 namespace SilverLeague\IDEAnnotator\Generators;
 
-use ReflectionClass;
-use ReflectionException;
 use SilverStripe\CMS\Controllers\ContentController;
 use SilverStripe\Core\ClassInfo;
+use SilverStripe\Core\Config\Config;
+use Page;
+use ReflectionClass;
 
 class ControllerTagGenerator extends AbstractTagGenerator
 {
+    /**
+     * ControllerTagGenerator constructor.
+     *
+     * @param string $className
+     * @param        $existingTags
+     * @throws ReflectionException
+     */
+    public function __construct($className, $existingTags)
+    {
+        $this->mapPageTypesToControllerName();
+
+        parent::__construct($className, $existingTags);
+    }
 
     /**
      * @return void
@@ -32,8 +46,18 @@ protected function generateControllerObjectTags()
         if (class_exists($pageClassname) && $this->isContentController($this->className)) {
             $pageClassname = $this->getAnnotationClassName($pageClassname);
 
-            $this->pushPropertyTag($pageClassname . ' dataRecord');
-            $this->pushMethodTag('data()', $pageClassname . ' data()');
+            $this->pushPropertyTag(sprintf('%s dataRecord', $pageClassname));
+            $this->pushMethodTag('data()', sprintf('%s data()', $pageClassname));
+
+            // don't mixin Page, since this is a ContentController method
+            if ($pageClassname !== 'Page') {
+                $this->pushMixinTag($pageClassname);
+            }
+        } elseif ($this->isContentController($this->className) && array_key_exists($this->className, self::$pageClassesCache)) {
+            $pageClassname = $this->getAnnotationClassName(self::$pageClassesCache[$this->className]);
+
+            $this->pushPropertyTag(sprintf('%s dataRecord', $pageClassname));
+            $this->pushMethodTag('data()', sprintf('%s data()', $pageClassname));
 
             // don't mixin Page, since this is a ContentController method
             if ($pageClassname !== 'Page') {
@@ -54,4 +78,20 @@ protected function isContentController($className)
         return ClassInfo::exists(ContentController::class)
             && $reflector->isSubclassOf(ContentController::class);
     }
+
+    /**
+     * Generates the cache of Page types to Controllers when the controller_name config is used
+     */
+    protected function mapPageTypesToControllerName()
+    {
+        if (empty(self::$pageClassesCache)) {
+            $pageClasses = ClassInfo::subclassesFor(Page::class);
+            foreach ($pageClasses as $pageClassname) {
+                $controllerName = Config::inst()->get($pageClassname, 'controller_name');
+                if (!empty($controllerName)) {
+                    self::$pageClassesCache[$controllerName] = $pageClassname;
+                }
+            }
+        }
+    }
 }
diff --git a/src/Generators/DocBlockGenerator.php b/src/Generators/DocBlockGenerator.php
@@ -2,14 +2,15 @@
 
 namespace SilverLeague\IDEAnnotator\Generators;
 
-use InvalidArgumentException;
-use LogicException;
 use phpDocumentor\Reflection\DocBlock;
 use phpDocumentor\Reflection\DocBlock\Serializer;
 use phpDocumentor\Reflection\DocBlock\Tag;
+use phpDocumentor\Reflection\DocBlockFactory;
+use SilverStripe\Control\Controller;
+use InvalidArgumentException;
+use LogicException;
 use ReflectionClass;
 use ReflectionException;
-use SilverStripe\Control\Controller;
 
 /**
  * Class DocBlockGenerator
@@ -34,6 +35,11 @@ class DocBlockGenerator
      */
     protected $tagGenerator;
 
+    /**
+     * @var DocBlockFactory
+     */
+    protected $docBlockFactory;
+
     /**
      * DocBlockGenerator constructor.
      *
@@ -45,6 +51,7 @@ public function __construct($className)
     {
         $this->className = $className;
         $this->reflector = new ReflectionClass($className);
+        $this->docBlockFactory = DocBlockFactory::createInstance();
 
         $generatorClass = $this->reflector->isSubclassOf(Controller::class)
             ? ControllerTagGenerator::class : OrmTagGenerator::class;
@@ -59,7 +66,11 @@ public function __construct($className)
     public function getExistingTags()
     {
         $docBlock = $this->getExistingDocBlock();
-        $docBlock = new DocBlock($docBlock);
+        if (!$docBlock) {
+            return [];
+        }
+
+        $docBlock = $this->docBlockFactory->create($docBlock);
 
         return $docBlock->getTags();
     }
@@ -97,15 +108,14 @@ public function getGeneratedDocBlock()
      */
     protected function mergeGeneratedTagsIntoDocBlock($existingDocBlock)
     {
-        $docBlock = new DocBlock($this->removeExistingSupportedTags($existingDocBlock));
+        $docBlock = $this->docBlockFactory->create(($existingDocBlock ?: "/**\n*/"));
 
-        if (!$docBlock->getText()) {
-            $docBlock->setText('Class \\' . $this->className);
+        $summary = $docBlock->getSummary();
+        if (!$summary) {
+            $summary = sprintf('Class \\%s', $this->className);
         }
 
-        foreach ($this->getGeneratedTags() as $tag) {
-            $docBlock->appendTag($tag);
-        }
+        $docBlock = new DocBlock($summary, $docBlock->getDescription(), $this->getGeneratedTags());
 
         $serializer = new Serializer();
         $docBlock = $serializer->getDocComment($docBlock);

diff --git a/src/Generators/OrmTagGenerator.php b/src/Generators/OrmTagGenerator.php
@@ -44,7 +44,7 @@ class OrmTagGenerator extends AbstractTagGenerator
      */
     protected static $dbfield_tagnames = [
         DBInt::class     => 'int',
-        DBBoolean::class => 'boolean',
+        DBBoolean::class => 'bool',
         DBFloat::class   => 'float',
         DBDecimal::class => 'float',
     ];

diff --git a/src/Helpers/AnnotatePermissionChecker.php b/src/Helpers/AnnotatePermissionChecker.php
@@ -120,7 +120,7 @@ public function classNameIsSupported($className)
      */
     public function moduleIsAllowed($moduleName)
     {
-        return in_array($moduleName, $this->enabledModules(), null);
+        return in_array($moduleName, $this->enabledModules(), false);
     }
 
     /**

diff --git a/src/Reflection/ShortNameResolver.php b/src/Reflection/ShortNameResolver.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace SilverLeague\IDEAnnotator\Reflection;
+
+use phpDocumentor\Reflection\Fqsen;
+use phpDocumentor\Reflection\FqsenResolver;
+use phpDocumentor\Reflection\Types\Context;
+
+class ShortNameResolver extends FqsenResolver
+{
+    public function resolve(string $fqsen, ?Context $context = null): Fqsen
+    {
+        if ($context === null) {
+            $context = new Context('');
+        }
+
+        return new Fqsen($fqsen);
+    }
+}
diff --git a/tests/unit/DataObjectAnnotatorTest.php b/tests/unit/DataObjectAnnotatorTest.php
@@ -204,7 +204,7 @@ public function testInversePlayerRelationOfTeam()
 
         $content = $this->annotator->getGeneratedFileContent(file_get_contents($filePath), Player::class);
 
-        $this->assertContains('@property boolean $IsRetired', $content);
+        $this->assertContains('@property bool $IsRetired', $content);
         $this->assertContains('@property string $ShirtNumber', $content);
         $this->assertContains('@property string $Shirt', $content);
         $this->assertContains('@property int $FavouriteTeamID', $content);
@@ -277,7 +277,7 @@ public function testShortInversePlayerRelationOfTeam()
 
         $content = $this->annotator->getGeneratedFileContent(file_get_contents($filePath), Player::class);
 
-        $this->assertContains('@property boolean $IsRetired', $content);
+        $this->assertContains('@property bool $IsRetired', $content);
         $this->assertContains('@property string $ShirtNumber', $content);
         $this->assertContains('@property int $FavouriteTeamID', $content);
         $this->assertContains('@method Team FavouriteTeam()', $content);