DocBlock Reflection not returning correct tags #5043

Closed
wants to merge 22 commits into
from

Projects

None yet

4 participants

@danez
Contributor
danez commented Aug 28, 2013

Currently when using reflection on a phpDoc the tags returned are not as expected. For example for an @return tag i expect Zend\Code\Generator\DocBlock\Tag\ReturnTag to be returned, but instead Zend\Code\Generator\DocBlock\Tag is always returned (which is missing the types-property from the reflection tag).

When working on a fix i encountered, that the whole generator tags topic does not work with reflection at all. e.g. AuthorTag throws an Exception about undefined function. And the nameing inside the Zend\Code\Generator\DocBlock\Tag* is completely off.

I would like to do a little rewrite, without breaking BC:

  • New renamed properties and getter and setters in Author-, License-, Return-, ParamTag (with BC except Author which is completely broken)
  • Make fromReflection() deprecated from these classes (in favor of TagManager)
  • Add new classes for other Tags similar to the Reflection namespace
  • Move Zend\Code\Generator\DocBlock\Tag to class GenericTag
  • Create a TagManager similar to Reflection namespace (these class decides which class to create)
  • Change DocBlockGenerator to use TagManager
  • Create Tests for new TagClasses and TagManager

What do you think about that?

@danez
Contributor
danez commented Aug 29, 2013

I tried to maintain backward compatibility were I could, the only exceptions are:

  • Zend\Code\Generator\DocBlock\Tag\AuthorTag: removed set/getDatatype() and set/getParamName()
  • Zend\Code\Generator\DocBlock\Tag\AuthorTag: __construct changed from ($options = array()) to ($authorName = null, $authorEmail = null)
  • Zend\Code\Generator\DocBlock\Tag\LicenseTag: __construct changed from ($options = array()) to ($url = null, $licenseName = null)
  • Zend\Code\Generator\DocBlock\Tag\ReturnTag: __construct changed from ($options = array()) to ($types = array(), $description = null)
  • Zend\Code\Generator\DocBlock\Tag\ParamTag: __construct changed from ($options = array()) to ($variableName = null, $types = array(), $description = null)
  • Using DocBlockGenerator::fromReflection() and afterwards getTags() is now returning the new Tag classes (ReturnTag, AuthorTag, ParamTag, ...) where applicable and otherwise GenericTag. The deprecated class Tag will not be returned anymore.
@Maks3w Maks3w commented on an outdated diff Aug 29, 2013
library/Zend/Code/Generator/DocBlock/Tag/MethodTag.php
+ /**
+ * @return string
+ */
+ public function getMethodName()
+ {
+ return $this->methodName;
+ }
+
+ /**
+ * @return string
+ */
+ public function generate()
+ {
+ $output = '@method'
+ . (($this->isStatic) ? ' static' : '')
+ . ((!empty($this->types)) ? ' ' . implode('|', $this->types) : '')
@Maks3w
Maks3w Aug 29, 2013 Member

May you can move the implode(...) part to the parent class in some method like typesToString

@Maks3w
Member
Maks3w commented Aug 29, 2013

@Danez The PR conflicts with some code in master. Can you rebase it?

@Maks3w
Member
Maks3w commented Aug 29, 2013

@ralphschindler May you want to review this.

@danez
Contributor
danez commented Aug 29, 2013

Rebased on master and moved the imploding to abstract class.

Daniel Tschi... added some commits Aug 29, 2013
Daniel Tschinder Add tests for TagClasses 0f54514
Daniel Tschinder test create from reflection 264bde0
Daniel Tschinder Fix generation from Reflection, trim inputs
Also allow is*() methods parallel to set*() methods in TagManager like isStatic()
remove one redundant comment
c503514
Daniel Tschinder Rename getPrototype to getClonedPrototype to better tell what it is d…
…oing
f00dcae
Daniel Tschinder test trimming in Tags
test PropertyClassFactory
0613c9b
@danez
Contributor
danez commented Aug 29, 2013

I think I tested all the new parts and should be ready for review

@Maks3w
Member
Maks3w commented Aug 29, 2013

Could you add class docblocks to all new classes describing and explaining the purpose of the class?

@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...ndTest/Code/Generator/DocBlock/Tag/GenericTagTest.php
@@ -0,0 +1,85 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...ndTest/Code/Generator/DocBlock/Tag/GenericTagTest.php
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
+ */
+
+namespace ZendTest\Code\Generator\DocBlock\Tag;
+
+use Zend\Code\Generator\DocBlock\Tag\GenericTag;
+use Zend\Code\Generator\DocBlock\TagManager;
+use Zend\Code\Reflection\DocBlockReflection;
+
+/**
+ * @category Zend
+ * @package Zend_Code_Generator
+ * @subpackage UnitTests
@samsonasik
samsonasik Aug 29, 2013 Contributor

remove @category @package and @subpackage

@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...endTest/Code/Generator/DocBlock/Tag/MethodTagTest.php
@@ -0,0 +1,103 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...endTest/Code/Generator/DocBlock/Tag/MethodTagTest.php
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
+ */
+
+namespace ZendTest\Code\Generator\DocBlock\Tag;
+
+use Zend\Code\Generator\DocBlock\Tag\MethodTag;
+use Zend\Code\Generator\DocBlock\TagManager;
+use Zend\Code\Reflection\DocBlockReflection;
+
+/**
+ * @category Zend
+ * @package Zend_Code_Generator
+ * @subpackage UnitTests
@samsonasik
samsonasik Aug 29, 2013 Contributor

remove @category @package and @subpackage

@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...dTest/Code/Generator/DocBlock/Tag/PropertyTagTest.php
@@ -0,0 +1,98 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...dTest/Code/Generator/DocBlock/Tag/PropertyTagTest.php
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
+ */
+
+namespace ZendTest\Code\Generator\DocBlock\Tag;
+
+use Zend\Code\Generator\DocBlock\Tag\PropertyTag;
+use Zend\Code\Generator\DocBlock\TagManager;
+use Zend\Code\Reflection\DocBlockReflection;
+
+/**
+ * @category Zend
+ * @package Zend_Code_Generator
+ * @subpackage UnitTests
@samsonasik
samsonasik Aug 29, 2013 Contributor

remove @category @package and @subpackage

@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...endTest/Code/Generator/DocBlock/Tag/ThrowsTagTest.php
@@ -0,0 +1,72 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
@samsonasik samsonasik commented on an outdated diff Aug 29, 2013
...endTest/Code/Generator/DocBlock/Tag/ThrowsTagTest.php
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Code
+ */
+
+namespace ZendTest\Code\Generator\DocBlock\Tag;
+
+use Zend\Code\Generator\DocBlock\Tag\ThrowsTag;
+use Zend\Code\Generator\DocBlock\TagManager;
+use Zend\Code\Reflection\DocBlockReflection;
+
+/**
+ * @category Zend
+ * @package Zend_Code_Generator
+ * @subpackage UnitTests
@samsonasik
samsonasik Aug 29, 2013 Contributor

remove @category @package and @subpackage

Daniel Tschi... added some commits Aug 29, 2013
@danez
Contributor
danez commented Aug 29, 2013

I added class descriptions, all other classes + namespace are selfexplanatory.
According to the comments on the headers I removed @package from all file headers in the test files.
Also I removed @category, @package, @subpackage, @license and @copyright from class comments in the test files.

@danez
Contributor
danez commented Aug 29, 2013

The deprecated comments may need some version in them, but I'm not sure what to put there. 2.2.x? 2.3.0?

Daniel Tschi... and others added some commits Aug 29, 2013
@weierophinney
Member

The deprecated comments may need some version in them, but I'm not sure what to put there. 2.2.x? 2.3.0?

2.3.0 -- this is a substantial change, so I'll target it for the next minor version. That said, the bits I saw were mostly BC, and the bits that weren't really didn't matter as you are not expected to create new instances manually for docblock tags (they should be created as part of reflection on their parents!).

Add the deprecation comments, and I'll do final review -- and thanks!

@danez
Contributor
danez commented Sep 3, 2013

Sometimes you might do and instantiate tags when generating code without reflection. But yeah most of the time you do not.
All the BC changes are up in the first comment.

@weierophinney weierophinney added a commit that referenced this pull request Sep 3, 2013
@weierophinney weierophinney [#5043] Added BC break notes to README
- Based on #5043 (comment)
0d66e97
@weierophinney weierophinney added a commit that referenced this pull request Sep 3, 2013
@weierophinney weierophinney Merge branch 'feature/5043' into develop
Close #5043
b843c29
@weierophinney weierophinney was assigned Sep 3, 2013
@weierophinney
Member

Merged to develop for release with 2.3.0.

@danez
Contributor
danez commented Sep 3, 2013

thanks

@danez danez deleted the unknown repository branch Sep 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment