FriendsOfSymfony / FOSRestBundle

This Bundle provides various tools to rapidly develop RESTful API's with Symfony
http://symfony.com/doc/master/bundles/FOSRestBundle/index.html
MIT License
2.79k stars 704 forks source link

Missing Attribute Annotation #2301

Closed robmro27 closed 2 years ago

robmro27 commented 3 years ago
namespace FOS\RestBundle\Controller\Annotations;

/**
 * View annotation class.
 *
 * @Annotation
 * @Target({"METHOD","CLASS"})
 */
class View extends Template
{
#[View]

Maybe I missed something but above class not support new PHP 8 attributes ? Checked all classes in Annotation folder. This is not supported yet ?

xabbuh commented 3 years ago

That's right, FOSRestBundle doesn't support attributes yet. Would you like to give this a try?

robmro27 commented 3 years ago

Hmm...checked some things but this will not be easy as for some annotations Symfony blocks you. Check in example @Route. Symfony misses \ReflectionAttribute::IS_INSTANCEOF parameter so all annotations extending symfony build it @Route will not work.

Maybe I am wrong, but this is the code that is problem IMO: https://github.com/symfony/symfony/blob/5.x/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php#L375

I think they need to add this to make @Delete @Get @Head @Link @Patch @Post @Put @Unlink @Lock @Unlock @PropFind @PropPatch @Move @Mkcol @Copy work

mbabker commented 3 years ago

The other part of this that's tricky though is the changed Symfony\Component\Routing\Annotation\Route constructor signature to support attributes (the same changes in the validator is why in one of my newer OSS bundles I gave up on Symfony 4.4 support). Symfony 5.2 expanded the constructor to support named arguments, then 5.3 deprecates the $data array in favor of the new arguments. So the FOS\RestBundle\Controller\Annotations\Route constructor would need its own B/C layer of some kind or there's the potential of having to drop support for 4.4 if that ends up being too difficult to pull off.

Funny enough, the \ReflectionAttribute::IS_INSTANCEOF issue doesn't exist for the @View annotation; the framework extra bundle does use that when parsing attributes.

robmro27 commented 3 years ago

Added PR to SF https://github.com/symfony/symfony/pull/41014, it shows it will be merged in 5.* probably, work on this could be done after that...

robmro27 commented 3 years ago

There is probably another problem with Params annotations. Currently they could be like this, nesting Constraints. I think it's not possible to pass this Constraint similar way to attribute unfortunately. Maybe there was something similar somewhere and was resolved ?

/**
 * @QueryParam(
 *   name="firstName",
 *   key="firstName",
 *   requirements=@Assert\Length(
 *      min = 2,
 *      max = 50,
 *      minMessage = "Your first name must be at least {{ limit }} characters long",
 *      maxMessage = "Your first name cannot be longer than {{ limit }} characters"
 * ),
 *   incompatibles={},
 *   default=null,
 *   description="",
 *   strict=true,
 *   map=false,
 *   nullable=false
 * )
 */
#[QueryParam(name: 'firstName', key: 'firstName', requirements: ???)]

Looks like even doctrine drop nesting:

So no solution probably...

michaljusiega commented 3 years ago

@robmro27 - Nested Attributes will be available since PHP 8.1. See: https://wiki.php.net/rfc/new_in_initializers

W0rma commented 3 years ago

The other part of this that's tricky though is the changed Symfony\Component\Routing\Annotation\Route constructor signature to support attributes (the same changes in the validator is why in one of my newer OSS bundles I gave up on Symfony 4.4 support). Symfony 5.2 expanded the constructor to support named arguments, then 5.3 deprecates the $data array in favor of the new arguments. So the FOS\RestBundle\Controller\Annotations\Route constructor would need its own B/C layer of some kind or there's the potential of having to drop support for 4.4 if that ends up being too difficult to pull off.

I guess this is related to https://github.com/FriendsOfSymfony/FOSRestBundle/issues/2311

I gave it a try in https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2312

GuilhemN commented 3 years ago

The @Route annotation is now compatible with php 8 annotations thanks to @W0rma (see https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2325 and https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2312).

@View and param annotations were not migrated yet though.