Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PhpParser does not parse use statements from traits. #20

Closed
wants to merge 7 commits into from

Conversation

jaypea
Copy link

@jaypea jaypea commented Dec 3, 2013

when parsing use statements from classes which use traits, the traits itself are not parsed.
this PR adds the missing trait parsing for php >= 5.4.
unit tests included.

Jan Prieser added 5 commits December 3, 2013 17:23
@@ -57,6 +57,12 @@ public function parseClass(\ReflectionClass $class)

$statements = $tokenizer->parseUseStatements($class->getNamespaceName());

if (method_exists($class, 'getTraits') && $traits = $class->getTraits()) {
foreach($traits as $trait) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alignment and PSR-2 compliance

@Ocramius
Copy link
Member

Ocramius commented Dec 6, 2013

Looks good 👍

@stof
Copy link
Member

stof commented Dec 6, 2013

this looks wrong to me: use statements of traits should only affect annotations set on properties and methods coming from the trait, not the whole class

@jaypea
Copy link
Author

jaypea commented Dec 9, 2013

This would make sense, if ReflectionClass::getMethods() would make a difference on the origin of a method.
Since it does not and this PhpParser is part of the Annotation Parsing and is just used to build a lookup table of namespaces to be used by the DocParser it simply does not matter if the use statement was defined inside of a trait or the class itself.

@stof
Copy link
Member

stof commented Dec 9, 2013

@jaypea If you have 2 annotation classes with the same short class name but in different namespaces, using one of them in the trait and the other one in the class itself will break in case you combine use statements.

@jaypea
Copy link
Author

jaypea commented Dec 9, 2013

Yes, the behaviour is atleast undefined, it would use whichever of possible the Annotation classes.
To fix this, the complete library would need to be changed. The AnnotationReader Class is the only place to know the actual ReflectionMethod and needs to set the imports of the docParser based on the actual Trait.

btw: (ReflectionMethod->getFilename() !== ReflectionMethod->getDeclaringClass()->getFilename()) is the only way to check if a method was defined inside of a trait.

@jaypea jaypea mentioned this pull request Dec 20, 2013
@jaypea
Copy link
Author

jaypea commented Dec 20, 2013

@stof i've rewrote this and created a new PR at #22

@jaypea jaypea closed this Dec 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants