BUGFIX: Fix getTagValues with comments on same line as an annotation#2615
BUGFIX: Fix getTagValues with comments on same line as an annotation#2615albe wants to merge 9 commits into
Conversation
|
TODO: Add test cases from #2612 (comment) and make tests pass |
|
@sorenmalling I somehow can't commit the suggestions in the comments above - can you do this? Otherwise I'll redo those changes in my local branch |
Co-authored-by: Alexander Berl <a.berl@outlook.com>
Done, and tests are running |
Co-authored-by: Alexander Berl <a.berl@outlook.com>
BUGFIX: Parse tags and annotations correctly for reflection
| { | ||
| $tagAndValue = []; | ||
| if (preg_match('/@[A-Za-z0-9\\\\]+\\\\([A-Za-z0-9]+)(?:\\((.*)\\))?$/', $line, $tagAndValue) === 0) { | ||
| if (preg_match('/(@[A-Za-z0-9\\\\]+\\\\[A-Za-z0-9]+)(?:(?:\\(|\s)([^\)]*))?/', $line, $tagAndValue) === 0) { |
There was a problem hiding this comment.
This regex captures the test cases in the discussion above, but will blow up if there's a round bracket in the annotation args or comment.
So we should add test cases for at least:
@something("foo()") => ['something' => 'foo()']
@something do not call foo() ever, it's bad => ['something' => 'do not call foo() ever, it\'s bad']
There was a problem hiding this comment.
Phew… those seem far-fetched. Then again, could happen, of course.
Co-authored-by: Alexander Berl <a.berl@outlook.com>
…od.php Co-authored-by: Alexander Berl <a.berl@outlook.com>
kdambekalns
changed the title
|
I think this should go into 7.2 as a |
kdambekalns
changed the title
|
Ha funny thing. This topic still exists and @kitsunet and me lately had fun with #3520 Id say our tag parsing is a hopeless situation and we should deprecate tags. Doctrines annotation parser and php attributes are way more stable. So we should instead deprecate tags and go though the codebase and allow everywhere also fully namespaced annotations/attributes. We could also do it one fix at a time and eventually finish as well ^^ See #3559 |
|
Yeah I think so too. I added another one to the list :D And we should probably deprecate tags for 9.2 |
5 participants
Related to #2612