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 707 forks source link

missing return type for getSubscribedServices in PreSymfony6AbstractFOSRestController #2351

Closed nlhommet closed 2 years ago

nlhommet commented 2 years ago

Hi, The return type for the method getSubscribedServices in PreSymfony6AbstractFOSRestController was forgotten in SF6 support.

In PreSymfony6AbstractFOSRestController.php line 27:

Compile Error: Declaration of FOS\RestBundle\Controller\PreSymfony6AbstractFOSRestController::getSubscribedServices() 
must be compatible with Symfony\Bundle\FrameworkBundle\Controller\AbstractController::getSubscribedServices (): array

Test with :

mbabker commented 2 years ago

Please paste in the results of composer show symfony/*

It should only be using PreSymfony6AbstractFOSRestController if symfony/framework-bundle 5.4 or earlier are installed (which, won't have the return type because of the B/C break).

nlhommet commented 2 years ago
# composer show symfony/*
symfony/cache                      v6.0.0  Provides an extended PSR-6, PSR-16 (and tags) implementation
symfony/cache-contracts            v3.0.0  Generic abstractions related to caching
symfony/config                     v6.0.0  Helps you find, load, combine, autofill and validate configuration values of any kind
symfony/console                    v6.0.0  Eases the creation of beautiful and testable command line interfaces
symfony/dependency-injection       v6.0.0  Allows you to standardize and centralize the way objects are constructed in your application
symfony/deprecation-contracts      v2.5.0  A generic function and convention to trigger deprecation notices
symfony/doctrine-bridge            v6.0.0  Provides integration for Doctrine with various Symfony components
symfony/doctrine-messenger         v6.0.0  Symfony Doctrine Messenger Bridge
symfony/dotenv                     v6.0.0  Registers environment variables from a .env file
symfony/error-handler              v6.0.0  Provides tools to manage errors and ease debugging PHP code
symfony/event-dispatcher           v6.0.0  Provides tools that allow your application components to communicate with each other by dispatching events and listening to them
symfony/event-dispatcher-contracts v3.0.0  Generic abstractions related to dispatching event
symfony/expression-language        v6.0.0  Provides an engine that can compile and evaluate expressions
symfony/filesystem                 v6.0.0  Provides basic utilities for the filesystem
symfony/finder                     v6.0.0  Finds files and directories via an intuitive fluent interface
symfony/flex                       v2.0.1  Composer plugin for Symfony
symfony/framework-bundle           v6.0.0  Provides a tight integration between Symfony components and the Symfony full-stack framework
symfony/http-client                v6.0.0  Provides powerful methods to fetch HTTP resources synchronously or asynchronously
symfony/http-client-contracts      v3.0.0  Generic abstractions related to HTTP clients
symfony/http-foundation            v6.0.0  Defines an object-oriented layer for the HTTP specification
symfony/http-kernel                v6.0.0  Provides a structured process for converting a Request into a Response
symfony/mailer                     v6.0.0  Helps sending emails
symfony/messenger                  v6.0.0  Helps applications send and receive messages to/from other applications or via message queues
symfony/mime                       v6.0.0  Allows manipulating MIME messages
symfony/monolog-bridge             v6.0.0  Provides integration for Monolog with various Symfony components
symfony/monolog-bundle             v3.7.1  Symfony MonologBundle
symfony/password-hasher            v6.0.0  Provides password hashing utilities
symfony/polyfill-intl-grapheme     v1.23.1 Symfony polyfill for intl's grapheme_* functions
symfony/polyfill-intl-idn          v1.23.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions
symfony/polyfill-intl-normalizer   v1.23.0 Symfony polyfill for intl's Normalizer class and related functions
symfony/polyfill-mbstring          v1.23.1 Symfony polyfill for the Mbstring extension
symfony/polyfill-php56             v1.20.0 Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php80             v1.23.1 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions
symfony/polyfill-php81             v1.23.0 Symfony polyfill backporting some PHP 8.1+ features to lower PHP versions
symfony/property-access            v6.0.0  Provides functions to read and write from/to an object or array using a simple string notation
symfony/property-info              v6.0.0  Extracts information about PHP class' properties using metadata of popular sources
symfony/proxy-manager-bridge       v6.0.0  Provides integration for ProxyManager with various Symfony components
symfony/routing                    v6.0.0  Maps an HTTP request to a set of configuration variables
symfony/runtime                    v6.0.0  Enables decoupling PHP applications from global state
symfony/security-bundle            v6.0.0  Provides a tight integration of the Security component into the Symfony full-stack framework
symfony/security-core              v6.0.0  Symfony Security Component - Core Library
symfony/security-csrf              v6.0.0  Symfony Security Component - CSRF Library
symfony/security-http              v6.0.0  Symfony Security Component - HTTP Integration
symfony/serializer                 v6.0.0  Handles serializing and deserializing data structures, including object graphs, into array structures or other formats like XM...
symfony/service-contracts          v2.5.0  Generic abstractions related to writing services
symfony/stopwatch                  v6.0.0  Provides a way to profile code
symfony/string                     v6.0.0  Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a unified way
symfony/translation-contracts      v3.0.0  Generic abstractions related to translation
symfony/twig-bridge                v6.0.0  Provides integration for Twig with various Symfony components
symfony/twig-bundle                v6.0.0  Provides a tight integration of Twig into the Symfony full-stack framework
symfony/validator                  v6.0.0  Provides tools to validate values
symfony/var-dumper                 v6.0.0  Provides mechanisms for walking through any arbitrary PHP variable
symfony/var-exporter               v6.0.0  Allows exporting any serializable PHP data structure to plain PHP code
symfony/web-profiler-bundle        v6.0.0  Provides a development tool that gives detailed information about the execution of any request
symfony/workflow                   v6.0.0  Provides tools for managing a workflow or finite state machine
symfony/yaml                       v6.0.0  Loads and dumps YAML files
mbabker commented 2 years ago

I don't have an environment I can do a quick Symfony 6 test with, but can you try adding a dd($ref->getReturnType()) at https://github.com/FriendsOfSymfony/FOSRestBundle/blob/3.x/Controller/AbstractFOSRestController.php#L18 and post what gets returned? The CI builds with 6.0 are passing, and PHPUnit was passing on the system I was working on the Symfony 6 compat code when I forced both 5.4 and 6.0 to install, so it seems odd that you're hitting this error.

nlhommet commented 2 years ago
^ ReflectionNamedType {#10003
  name: "array"
  allowsNull: false
  isBuiltin: true
 }
mbabker commented 2 years ago

That is so weird...

I'll have to do some testing later. Everything so far is saying that the PostSymfony6AbstractFOSRestController class for Symfony 6 should be used (which has the typehint), yet it's using the class for Symfony <=5.4.

nlhommet commented 2 years ago

In PHP8.1 the return type is mandatory if the method we overload has one. And PreSymfony6AbstractFOSRestController extend AbstractController. https://github.com/symfony/symfony/blob/6.1/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php#L85

mbabker commented 2 years ago

Right, I get that. Adding the return type would be a hard B/C break, so there's a compat layer that's supposed to pick the right controller class to extend based on whether there is a return type on AbstractController::getSubscribedServices(). As your dump shows, it's correctly detecting the return type, yet in the if/else immediately following where I asked you to paste that dump, it's picking the code path for there not being a return type. Which makes no sense. With symfony/framework-bundle >=6.0, PreSymfony6AbstractFOSRestController should NOT be getting used.

mbabker commented 2 years ago

2352 should fix this. It's an issue with the framework cache warmers, not the compat layer.