Support for short names via config

Fix low-prio bug with too much trimming Fix #68
silverleague · Feb 11, 2018 · 7fc689a · 7fc689a
1 parent a466e22
commit 7fc689a
Show file tree

Hide file tree

Showing 20 changed files with 350 additions and 95 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -33,3 +33,12 @@
 
 # 3.0 beta-1
 * Support for SilverStripe 4
+
+# 3.0 rc-1
+* Updated support for SilverStripe 4
+* Added support for the `through` method
+
+# 3.0 rc-2
+* Fixed bug trimming too much whitespace in rc-1
+* Support for short classnames instead of FQN
+* ~~require_once in tests no longer needed~~ Nope, still needed
diff --git a/docs/en/Installation.md b/docs/en/Installation.md
@@ -53,3 +53,24 @@ SilverLeague\IDEAnnotator\DataObjectAnnotator:
       - mysite
       - SilverLeague/IDEAnnotator
 ```
+
+If you don't want to use fully qualified classnames, you can configure that like so:
+
+```yaml
+
+---
+Only:
+    environment: 'dev'
+---
+SilverLeague\IDEAnnotator\DataObjectAnnotator:
+    enabled: true
+    use_short_name: true
+    enabled_modules:
+      - mysite
+```
+
+**NOTE**
+
+- Using short names, will also shorten core names like `ManyManyList`, you'll have to adjust your use statements to work.
+
+- If you change the usage of short names halfway in your project, you may need to clear out all your docblocks before regenerating
diff --git a/src/DataObjectAnnotator.php b/src/DataObjectAnnotator.php
@@ -8,6 +8,7 @@
 use ReflectionException;
 use SilverStripe\Control\Director;
 use SilverStripe\Core\ClassInfo;
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injectable;
@@ -34,25 +35,27 @@ class DataObjectAnnotator
     use Configurable;
     use Extensible;
 
+    /**
+     * All classes that subclass Object
+     * @var array
+     */
+    protected static $extension_classes = [];
     /**
      * @config
      * Enable generation from @see Annotatable and @see DataObjectAnnotatorTask
      * @var bool
      */
     private static $enabled = false;
-
     /**
      * @config
      * Enable modules that are allowed to have generated docblocks for DataObjects and DataExtensions
      * @var array
      */
     private static $enabled_modules = ['mysite'];
-
     /**
      * @var AnnotatePermissionChecker
      */
     private $permissionChecker;
-
     /**
      * @var array
      */
@@ -61,11 +64,32 @@ class DataObjectAnnotator
     /**
      * DataObjectAnnotator constructor.
      * @throws NotFoundExceptionInterface
+     * @throws ReflectionException
      */
     public function __construct()
     {
         // Don't instantiate anything if annotations are not enabled.
         if (static::config()->get('enabled') === true && Director::isDev()) {
+            $extendableClasses = Config::inst()->getAll();
+            // We need to check all config to see if the class is extensible
+            // @todo find a cleaner method, this is already better than previous implementations though
+            foreach ($extendableClasses as $key => $configClass) {
+                if (isset($configClass['extensions']) &&
+                    count($configClass['extensions']) > 0 &&
+                    !in_array(self::$extension_classes, $configClass)
+                ) {
+                    $extension_classes[] = ClassInfo::class_name($key);
+                }
+            }
+
+            // Because the tests re-instantiate the class every time
+            // We need to make it a unique array
+            // Also, it's not a bad practice, making sure the array is unique
+            $extension_classes = array_unique($extension_classes);
+            // Keep it local until done saves roughly 1 to 2 MB of memory usage.
+            // Keeping local I guess!
+            static::$extension_classes = $extension_classes;
+
             $this->permissionChecker = Injector::inst()->get(AnnotatePermissionChecker::class);
             foreach ($this->permissionChecker->getSupportedParentClasses() as $supportedParentClass) {
                 $this->setEnabledClasses($supportedParentClass);
@@ -86,6 +110,33 @@ protected function setEnabledClasses($supportedParentClass)
         }
     }
 
+    /**
+     * @return array
+     */
+    public static function getExtensionClasses()
+    {
+        return self::$extension_classes;
+    }
+
+    /**
+     * @param array $extension_classes
+     */
+    public static function setExtensionClasses($extension_classes)
+    {
+        self::$extension_classes = $extension_classes;
+    }
+
+    /**
+     * Add another extension class
+     * @param $extension_class
+     */
+    public static function pushExtensionClass($extension_class)
+    {
+        if (!in_array($extension_class, self::$extension_classes)) {
+            self::$extension_classes[] = $extension_class;
+        }
+    }
+
     /**
      * @return boolean
      */
@@ -175,8 +226,6 @@ protected function writeFileContent($className)
 
             // we have a change, so write the new file
             if ($generated && $generated !== $original && $className) {
-                // Trim unneeded whitespaces at the end of lines
-                $generated = preg_replace('/\s+$/m', '', $generated);
                 file_put_contents($filePath, $generated);
                 DB::alteration_message($className . ' Annotated', 'created');
             } elseif ($generated === $original && $className) {
@@ -202,6 +251,9 @@ protected function getGeneratedFileContent($fileContent, $className)
         $existing = $generator->getExistingDocBlock();
         $generated = $generator->getGeneratedDocBlock();
 
+        // Trim unneeded whitespaces at the end of lines for PSR-2
+        $generated = preg_replace('/\s+$/m', '', $generated);
+
         if ($existing) {
             $fileContent = str_replace($existing, $generated, $fileContent);
         } else {

diff --git a/src/Extensions/Annotatable.php b/src/Extensions/Annotatable.php
@@ -46,7 +46,7 @@ public function afterCallActionHandler()
     {
         $envIsAllowed = Director::isDev() && Config::inst()->get(DataObjectAnnotator::class, 'enabled');
         $skipAnnotation = $this->owner->getRequest()->getVar('skipannotation');
-        
+
         // Only instatiate things when we want to run it, this is for when the module is accidentally installed
         // on non-dev environments for example
         if ($skipAnnotation === null && $envIsAllowed) {

diff --git a/src/Generators/AbstractTagGenerator.php b/src/Generators/AbstractTagGenerator.php
@@ -40,12 +40,6 @@ abstract class AbstractTagGenerator
      */
     protected $tags = [];
 
-    /**
-     * All classes that subclass Object
-     * @var array
-     */
-    protected $extensionClasses;
-
     /**
      * DocBlockTagGenerator constructor.
      *
@@ -58,15 +52,6 @@ public function __construct($className, $existingTags)
         $this->className = $className;
         $this->existingTags = (array)$existingTags;
         $this->reflector = new \ReflectionClass($className);
-        $extendableClasses = Config::inst()->getAll();
-        $this->extensionClasses = [];
-        // We need to check all config to see if the class is extensible
-        // @todo find a cleaner method
-        foreach ($extendableClasses as $key => $configClass) {
-            if (isset($configClass['extensions']) && count($configClass['extensions']) > 0) {
-                $this->extensionClasses[] = ClassInfo::class_name($key);
-            }
-        }
         $this->tags = $this->getSupportedTagTypes();
 
         $this->generateTags();
@@ -126,7 +111,8 @@ protected function generateExtensionsTags()
     {
         if ($fields = (array)$this->getClassConfig('extensions')) {
             foreach ($fields as $fieldName) {
-                $this->pushMixinTag("\\$fieldName");
+                $mixinName = $this->getAnnotationClassName($fieldName);
+                $this->pushMixinTag($mixinName);
             }
         }
     }
@@ -168,14 +154,32 @@ public function getExistingTagCommentByTagString($tagString)
     {
         foreach ($this->getExistingTags() as $tag) {
             $content = $tag->getContent();
-            if (strpos($content, $tagString) !== false) {
+            // 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
+            if (strpos($content, $tagString . ' ') !== false) {
                 return str_replace($tagString, '', $content);
             }
         }
 
         return '';
     }
 
+    /**
+     * Check if we need to use the short name for a class
+     *
+     * @param string $class
+     * @return string
+     */
+    protected function getAnnotationClassName($class)
+    {
+        if (DataObjectAnnotator::config()->get('use_short_name')) {
+            return ClassInfo::shortName($class);
+        }
+
+        return "\\$class";
+    }
+
     /**
      * @return Tag[]
      */
@@ -193,14 +197,21 @@ protected function generateOwnerTags()
     {
         $className = $this->className;
         if (Injector::inst()->get($this->className) instanceof Extension) {
-            $owners = array_filter($this->extensionClasses, function ($class) use ($className) {
+            $owners = array_filter(DataObjectAnnotator::getExtensionClasses(), function ($class) use ($className) {
                 $config = Config::inst()->get($class, 'extensions');
 
                 return ($config !== null && in_array($className, $config, null));
             });
             $owners[] = $this->className;
             if (!empty($owners)) {
-                $this->pushPropertyTag('\\' . implode("|\\", array_values($owners)) . ' $owner');
+                $tagString = '\\' . implode("|\\", array_values($owners)) . ' $owner';
+                if (Config::inst()->get(DataObjectAnnotator::class, 'use_short_name')) {
+                    foreach ($owners as $key => $owner) {
+                        $owners[$key] = $this->getAnnotationClassName($owner);
+                    }
+                    $tagString = implode("|", array_values($owners)) . ' $owner';
+                }
+                $this->pushPropertyTag($tagString);
             }
         }
     }
@@ -219,7 +230,8 @@ protected function pushPropertyTag($tagString)
      */
     protected function pushMethodTag($methodName, $tagString)
     {
-        if (!$this->reflector->hasMethod($methodName)) {
+        // Exception for `data()` method is needed
+        if (!$this->reflector->hasMethod($methodName) || $methodName === 'data()') {
             $this->tags['methods'][$tagString] = $this->pushTagWithExistingComment('method', $tagString);
         }
     }

diff --git a/src/Generators/ControllerTagGenerator.php b/src/Generators/ControllerTagGenerator.php
@@ -22,16 +22,23 @@ protected function generateTags()
         $this->generateOwnerTags();
     }
 
+    /**
+     * Generate the controller tags, these differ slightly from the standard ORM tags
+     *
+     * @throws ReflectionException
+     */
     protected function generateControllerObjectTags()
     {
         $pageClassname = str_replace(['_Controller', 'Controller'], '', $this->className);
         if (class_exists($pageClassname) && $this->isContentController($this->className)) {
-            $this->pushPropertyTag("\\$pageClassname" . ' dataRecord');
-            $this->pushMethodTag($pageClassname, "\\$pageClassname" . ' data()');
+            $pageClassname = $this->getAnnotationClassName($pageClassname);
+
+            $this->pushPropertyTag($pageClassname . ' dataRecord');
+            $this->pushMethodTag('data()', $pageClassname . ' data()');
 
             // don't mixin Page, since this is a ContentController method
             if ($pageClassname !== 'Page') {
-                $this->pushMixinTag("\\$pageClassname");
+                $this->pushMixinTag($pageClassname);
             }
         }
     }

diff --git a/src/Generators/OrmTagGenerator.php b/src/Generators/OrmTagGenerator.php
@@ -100,7 +100,10 @@ protected function generateBelongsToTags()
         if ($fields = (array)$this->getClassConfig('belongs_to')) {
             foreach ($fields as $fieldName => $dataObjectName) {
                 $dataObjectName = $this->resolveDotNotation($dataObjectName);
-                $this->pushMethodTag($fieldName, "\\{$dataObjectName} {$fieldName}()");
+                $dataObjectName = $this->getAnnotationClassName($dataObjectName);
+                $tagString = "{$dataObjectName} {$fieldName}()";
+
+                $this->pushMethodTag($fieldName, $tagString);
             }
         }
     }
@@ -124,7 +127,10 @@ protected function generateHasOneTags()
         if ($fields = (array)$this->getClassConfig('has_one')) {
             foreach ($fields as $fieldName => $dataObjectName) {
                 $this->pushPropertyTag("int \${$fieldName}ID");
-                $this->pushMethodTag($fieldName, "\\{$dataObjectName} {$fieldName}()");
+                $dataObjectName = $this->getAnnotationClassName($dataObjectName);
+                $tagString = "{$dataObjectName} {$fieldName}()";
+
+                $this->pushMethodTag($fieldName, $tagString);
             }
         }
     }
@@ -137,6 +143,21 @@ protected function generateHasManyTags()
         $this->generateTagsForDataLists($this->getClassConfig('has_many'), DataList::class);
     }
 
+    /**
+     * Generate the $many_many method values.
+     */
+    protected function generateManyManyTags()
+    {
+        $this->generateTagsForDataLists($this->getClassConfig('many_many'), ManyManyList::class);
+    }
+
+    /**
+     * Generate the $belongs_many_many method values.
+     */
+    protected function generateBelongsManyManyTags()
+    {
+        $this->generateTagsForDataLists($this->getClassConfig('belongs_many_many'), ManyManyList::class);
+    }
     /**
      * @param array $fields
      * @param string $listType
@@ -145,29 +166,18 @@ protected function generateTagsForDataLists($fields, $listType = DataList::class
     {
         if (!empty($fields)) {
             foreach ((array)$fields as $fieldName => $dataObjectName) {
+                $fieldName = trim($fieldName);
                 // A many_many with a relation through another DataObject
                 if (is_array($dataObjectName)) {
                     $dataObjectName = $dataObjectName['through'];
                 }
                 $dataObjectName = $this->resolveDotNotation($dataObjectName);
-                $this->pushMethodTag($fieldName, "\\{$listType}|\\{$dataObjectName}[] {$fieldName}()");
+                $listName = $this->getAnnotationClassName($listType);
+                $dataObjectName = $this->getAnnotationClassName($dataObjectName);
+
+                $tagString = "{$listName}|{$dataObjectName}[] {$fieldName}()";
+                $this->pushMethodTag($fieldName, $tagString);
             }
         }
     }
-
-    /**
-     * Generate the $many_many method values.
-     */
-    protected function generateManyManyTags()
-    {
-        $this->generateTagsForDataLists($this->getClassConfig('many_many'), ManyManyList::class);
-    }
-
-    /**
-     * Generate the $belongs_many_many method values.
-     */
-    protected function generateBelongsManyManyTags()
-    {
-        $this->generateTagsForDataLists($this->getClassConfig('belongs_many_many'), ManyManyList::class);
-    }
 }