Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Routing][Serializer] Add possible missing imports of NamedArgumentConstructor #41706

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
Closed
wants to merge 2 commits into from
Closed

[Routing][Serializer] Add possible missing imports of NamedArgumentConstructor #41706

wants to merge 2 commits into from

Conversation

michaljusiega
Copy link
Contributor

@michaljusiega michaljusiega commented Jun 14, 2021 •
edited
Loading

Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets ~
License MIT
Doc PR

Hi.
This PR fixes possible missing imports of NamedArgumentConstructor (etc. added in #40266).

The main reason for this pull request and apply this changes is error caused by JMSSerializer as below:

Luxmed\Intranet\Exceptions\JMSSerializerException : [Semantical Error] The annotation "@NamedArgumentConstructor" in class Symfony\Component\Routing\Annotation\Route was never imported. Did you maybe forget to add a "use" statement for this annotation?
 /var/www/intranet/src/Service/JMSSerializerService.php:136
 /var/www/intranet/tests/DTO/SearchAnnotationDTOTest.php:197
 
 Caused by
 Doctrine\Common\Annotations\AnnotationException: [Semantical Error] The annotation "@NamedArgumentConstructor" in class Symfony\Component\Routing\Annotation\Route was never imported. Did you maybe forget to add a "use" statement for this annotation?
 
 /var/www/intranet/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationException.php:39
 /var/www/intranet/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:803
 /var/www/intranet/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:719
 /var/www/intranet/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:376
 /var/www/intranet/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php:146
 /var/www/intranet/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/CachedReader.php:64
 /var/www/intranet/vendor/jms/serializer/src/Metadata/Driver/AnnotationDriver.php:92
 /var/www/intranet/vendor/jms/metadata/src/Driver/DriverChain.php:32
 /var/www/intranet/vendor/jms/serializer/src/Metadata/Driver/TypedPropertiesDriver.php:63
 /var/www/intranet/vendor/jms/metadata/src/MetadataFactory.php:111
 /var/www/intranet/vendor/jms/serializer/src/GraphNavigator/SerializationGraphNavigator.php:218
 /var/www/intranet/vendor/jms/serializer/src/JsonSerializationVisitor.php:137
 /var/www/intranet/vendor/jms/serializer/src/GraphNavigator/SerializationGraphNavigator.php:260
 /var/www/intranet/vendor/jms/serializer/src/Serializer.php:252
 /var/www/intranet/vendor/jms/serializer/src/Serializer.php:163
 /var/www/intranet/src/Service/JMSSerializerService.php:134
 /var/www/intranet/tests/DTO/SearchAnnotationDTOTest.php:197

@michaljusiega michaljusiega requested a review from dunglas as a code owner June 14, 2021 20:07
@carsonbot carsonbot added this to the 5.3 milestone Jun 14, 2021
derrabus
derrabus requested changes Jun 14, 2021
View reviewed changes
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

We don't need to import core annotations. The current code is correct.

@michaljusiega
Copy link
Contributor Author

We don't need to import core annotations. The current code is correct.

So what should I do to avoid this issue ? :)

@derrabus
Copy link
Member

What version of Doctrine Annotations do you use? Can you maybe create a reproducer so I can analyze the problem?

@michaljusiega
Copy link
Contributor Author 

composer show
...
doctrine/annotations                 1.13.1    Docblock Annotations Parser
...

I will try to create testcase / reproducer after my work :)

@michaljusiega
Copy link
Contributor Author

michaljusiega commented Jun 15, 2021 •
edited
Loading

@derrabus I added test case for Route annotation.

@michaljusiega
Copy link
Contributor Author

@derrabus - friendly ping ;)

@nicolas-grekas nicolas-grekas requested a review from derrabus July 12, 2021 13:01
@derrabus
Copy link
Member

Sorry, I‘m on vacation at the moment and did not have time to investigate further. I hope that I can get back to you by the end of the week.

derrabus
derrabus requested changes Jul 17, 2021
View reviewed changes
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. I don't really understand why you (or JMS serializer) would want to parse the annotations on an annotation class. That being said, importing that annotation should not be necessary because it's not necessary for the other core annotations.

See doctrine/annotations#419

@derrabus
Copy link
Member

Closing in favor of doctrine/annotations#419.

@derrabus derrabus closed this Jul 17, 2021
@michaljusiega
Copy link
Contributor Author

Thank you @derrabus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus requested changes

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Bug Routing Serializer Status: Needs Work
Projects
None yet
Milestone
5.3
Development

Successfully merging this pull request may close these issues.
3 participants
@michaljusiega @derrabus @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.