Skip to content

Fix method name is empty when parenthesis is missing in @method #72

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 6, 2016
Merged

Fix method name is empty when parenthesis is missing in @method #72

merged 1 commit into from
Jun 6, 2016

Conversation

fabiang
Copy link
Contributor

@fabiang fabiang commented Feb 26, 2016

Fix method name is empty when parenthesis is missing in @method

@dunglas
Copy link
Contributor

dunglas commented Mar 1, 2016

👍, I've tested it and this patch works for me.

@dunglas dunglas mentioned this pull request Mar 1, 2016
Closed
@dunglas
Copy link
Contributor

dunglas commented Mar 27, 2016

ping @mvriel, any chance to get this one merged?

@fabiang
Copy link
Contributor Author

fabiang commented May 30, 2016

Merging this would help fixing some of my unit tests and ci builds, as this breaks mocking with Prophecy in some cases.

@dunglas dunglas mentioned this pull request May 30, 2016
Merged
@dunglas
Copy link
Contributor

dunglas commented May 30, 2016 •
edited
Loading

This bug prevents PHPUnit and PHPSpec to work with Symfony 3.1 (released today). If you merge the patch from @fabiang please create a new tag too.

@GrahamCampbell
Copy link
Contributor

@dunglas Is correct. This is a blocker for PHPUnit. This is affecting a lot of people.

@soullivaneuh
Copy link
Contributor

@dunglas @GrahamCampbell Is this error I recently got on Travis related? https://travis-ci.org/sonata-project/SonataAdminBundle/jobs/133876692#L493

Only fail when Symfony 3.1 is installed and tested with PHPUnit 5.

Do you have a workaround waiting this issue?

@stof
Copy link

stof commented May 31, 2016

@soullivaneuh no it is not. Your issue is related to https://github.com/sonata-project/SonataAdminBundle/blob/daa65de93c11737481ab99475bb5b551234082ce/Route/RoutesCache.php#L62 not handling the case where $reflection->getFilename() does not give you a path, which happens in 2 cases:

  • internal classes (will return false)
  • eval'd code (will return a string containing eval()'d code among other things)

Symfony 3.1 is stricter about receiving a proper path in FileResource

@soullivaneuh
Copy link
Contributor

@stof Ok sorry for the out of subject message and thank you for the tip. 👍

soullivaneuh added a commit to soullivaneuh/SonataAdminBundle that referenced this pull request May 31, 2016
@soullivaneuh
Fix DefaultRouteGenerator cache tests with Symfony 3.1
9f5b691 
Symfony\Component\Config\Resource\FileResource fail if the given path is invalid since Symfony 3.1.

It was the case from the begining because of mocking an interface.

A real admin class is mocked instead.

Ref: phpDocumentor/ReflectionDocBlock#72 (comment)
@soullivaneuh soullivaneuh mentioned this pull request May 31, 2016
Merged
soullivaneuh added a commit to soullivaneuh/SonataAdminBundle that referenced this pull request May 31, 2016
@soullivaneuh
Fix DefaultRouteGenerator cache tests with Symfony 3.1
e8a4ef5 
Symfony\Component\Config\Resource\FileResource fail if the given path is invalid since Symfony 3.1.

It was the case from the begining because of mocking an interface.

A condition is added to the RouteCaches class.

Ref: phpDocumentor/ReflectionDocBlock#72 (comment)
soullivaneuh added a commit to soullivaneuh/SonataAdminBundle that referenced this pull request May 31, 2016
@soullivaneuh
Fix DefaultRouteGenerator cache tests with Symfony 3.1
9b8b477 
Symfony\Component\Config\Resource\FileResource fail if the given path is invalid since Symfony 3.1.

It was the case from the begining because of mocking an interface.

A condition is added to the RoutesCache class.

Ref: phpDocumentor/ReflectionDocBlock#72 (comment)
soullivaneuh added a commit to soullivaneuh/SonataAdminBundle that referenced this pull request May 31, 2016
@soullivaneuh
Fix DefaultRouteGenerator cache tests with Symfony 3.1
2555342 
Symfony\Component\Config\Resource\FileResource fail if the given path is invalid since Symfony 3.1.

It was the case from the begining because of mocking an interface.

A condition is added to the RoutesCache class.

Ref: phpDocumentor/ReflectionDocBlock#72 (comment)
soullivaneuh added a commit to sonata-project/SonataAdminBundle that referenced this pull request May 31, 2016
@soullivaneuh
Fix DefaultRouteGenerator cache tests with Symfony 3.1 (#3899)
43b1359 
Symfony\Component\Config\Resource\FileResource fail if the given path is invalid since Symfony 3.1.

It was the case from the begining because of mocking an interface.

A condition is added to the RoutesCache class.

Ref: phpDocumentor/ReflectionDocBlock#72 (comment)
@scaytrase
Copy link

Any chance this gets merged?

ping @mvriel

@dunglas
Copy link
Contributor

dunglas commented Jun 2, 2016

https://twitter.com/mvriel/status/737584530370875393

greg0ire pushed a commit to greg0ire/SonataAdminBundle that referenced this pull request Jun 2, 2016
@soullivaneuh
Fix DefaultRouteGenerator cache tests with Symfony 3.1 (sonata-projec…
27bbd2b 
…t#3899)

Symfony\Component\Config\Resource\FileResource fail if the given path is invalid since Symfony 3.1.

It was the case from the begining because of mocking an interface.

A condition is added to the RoutesCache class.

Ref: phpDocumentor/ReflectionDocBlock#72 (comment)
@JEDIBC
Copy link

JEDIBC commented Jun 2, 2016

👍

@mvriel mvriel merged commit 9a0bd76 into phpDocumentor:master Jun 6, 2016
@mvriel
Copy link
Member

mvriel commented Jun 6, 2016

I have verified this fix and merged it; I am not entirely sure that this fix conforms spec (I find it hard to imagine what the exact impact is of removing the () since those are also a means to determine which part of the signature is the method name). But given the significant impact I am going to merge this anyway.

In a few minutes I will tag another release.

Thank you for this contribution; it is good to see these incompatibilities addressed.

@fabiang fabiang deleted the fix-missing-parenthesis branch June 6, 2016 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@fabiang @dunglas @GrahamCampbell @soullivaneuh @stof @scaytrase @JEDIBC @mvriel