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

[PHP8] Add broken test with failing attributes generation #502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lisachenko
Copy link
Member

This broken test is used to track issue with broken codogen with attributes near class/method/method/function/property/parameter.

@lisachenko lisachenko added the Bug label Apr 14, 2024
@scrutinizer-notifier
Copy link

A new inspection was created.

@lisachenko
Copy link
Member Author

Blocked on laminas/laminas-code#145

@lisachenko lisachenko added this to the 4.0.0 milestone Apr 14, 2024
@lisachenko
Copy link
Member Author

FYI @samsonasik this issues is considered as main blocker for running on codebase with attributes - child class won't contain any attributes and this breaks the logic of clients who expect attributes to be preserved.

@lisachenko
Copy link
Member Author

One solution can be switch to something under our control (I doubt that I'll have enough resources to support this).

Another solution is to completely remove dependency on laminas package and instead use PrettyPrinter and AST nodes from the nikic/php-parser via BuilderFactory::attribute and similar to generate new code.

@samsonasik
Copy link
Contributor

@lisachenko I am currently on my village and will be back on 16th/17th April, so hopefully can fully looking at it after that day as possibly 2 days long trip.

Ceate own builder for it can be solution, but need to check how many laminas-code already involved in other area

@samsonasik
Copy link
Contributor

samsonasik commented May 17, 2024

@lisachenko it seems too tight with laminas-code, and for the alternative, to be able to use php-parser's Printer, we need to know the Node from the ReflectionFunction which may need to use Go\ParserReflection\ReflectionFunction, since Function_ parameter is required, we may scan from very early with Go\ParserReflection\ReflectionFileNamespace, I don't have idea how big the refactor it can be, any idea?

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

Successfully merging this pull request may close these issues.

3 participants