8000 [FrameworkBundle][Routing] Remove remaining deprecations by alexandre-daubois · Pull Request #52137 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[FrameworkBundle][Routing] Remove remaining deprecations #52137

8000
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

Merged
merged 1 commit into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions .github/expected-missing-return-types.diff
Original file line number Diff line number Diff line change
Expand Up @@ -408,16 +408,6 @@ diff --git a/src/Symfony/Component/HttpKernel/KernelInterface.php b/src/Symfony/
- public function shutdown();
+ public function shutdown(): void;

/**
diff --git a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php b/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php
--- a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php
+++ b/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php
@@ -324,5 +324,5 @@ abstract class AnnotationClassLoader implements LoaderInterface
* @return void
*/
- abstract protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot);
+ abstract protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot): void;

/**
diff --git a/src/Symfony/Component/Security/Core/Authentication/RememberMe/TokenProviderInterface.php b/src/Symfony/Component/Security/Core/Authentication/RememberMe/TokenProviderInterface.php
--- a/src/Symfony/Component/Security/Core/Authentication/RememberMe/TokenProviderInterface.php
Expand Down
10 changes: 8 additions & 2 deletions UPGRADE-7.0.md
10000
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ FrameworkBundle
* Change default of some config options:

| option | default Symfony <7.0 | default in Symfony 7.0+ |
| -------------------------------------------- | -------------------------- | --------------------------------------------------------------------------- |
|----------------------------------------------|----------------------------|-----------------------------------------------------------------------------|
| `framework.http_method_override` | `true` | `false` |
| `framework.handle_all_throwables` | `false` | `true` |
| `framework.php_errors.log` | `'%kernel.debug%'` | `true` |
Expand All @@ -260,6 +260,10 @@ FrameworkBundle
| `framework.validation.email_validation_mode` | `'loose'` | `'html5'` |
* Remove the `framework.validation.enable_annotations` config option, use `framework.validation.enable_attributes` instead
* Remove the `framework.serializer.enable_annotations` config option, use `framework.serializer.enable_attributes` instead
* Remove the `routing.loader.annotation` service, use the `routing.loader.attribute` service instead
* Remove the `routing.loader.annotation.directory` service, use the `routing.loader.attribute.directory` service instead
* Remove the `routing.loader.annotation.file` service, use the `routing.loader.attribute.file` service instead
* Remove `AnnotatedRouteControllerLoader`, use `AttributeRouteControllerLoader` instead

HttpFoundation
--------------
Expand Down Expand Up @@ -400,7 +404,9 @@ Routing

* Add parameter `array $routeParameters` to `UrlMatcher::handleRouteRequirements()`
* Remove Doctrine annotations support in favor of native attributes. Use `Symfony\Component\Routing\Annotation\Route` as native attribute now
* Change the constructor signature of `AnnotationClassLoader` to `__construct(?string $env = null)`, passing an annotation reader as first argument is not supported anymore
* Remove `AnnotationClassLoader`, use `AttributeClassLoader` instead
* Remove `AnnotationDirectoryLoader`, use `AttributeDirectoryLoader` instead
* Remove `AnnotationFileLoader`, use `AttributeFileLoader` instead

Security
--------
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ CHANGELOG
* Make the `framework.validation.email_validation_mode` config option default to `html5`
* Remove the `framework.validation.enable_annotations` config option, use `framework.validation.enable_attributes` instead
* Remove the `framework.serializer.enable_annotations` config option, use `framework.serializer.enable_attributes` instead
* Remove the `routing.loader.annotation` service, use the `routing.loader.attribute` service instead
* Remove the `routing.loader.annotation.directory` service, use the `routing.loader.attribute.directory` service instead
* Remove the `routing.loader.annotation.file` service, use the `routing.loader.attribute.file` service instead
* Remove `AnnotatedRouteControllerLoader`, use `AttributeRouteControllerLoader` instead

6.4
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,29 +98,20 @@
])
->tag('routing.loader', ['priority' => -10])

->alias('routing.loader.annotation', 'routing.loader.attribute')
->deprecate('symfony/routing', '6.4', 'The "%alias_id%" service is deprecated, use the "routing.loader.attribute" service instead.')

->set('routing.loader.attribute.directory', AttributeDirectoryLoader::class)
->args([
service('file_locator'),
service('routing.loader.attribute'),
])
->tag('routing.loader', ['priority' => -10])

->alias('routing.loader.annotation.directory', 'routing.loader.attribute.directory')
->deprecate('symfony/routing', '6.4', 'The "%alias_id%" service is deprecated, use the "routing.loader.attribute.directory" service instead.')

->set('routing.loader.attribute.file', AttributeFileLoader::class)
->args([
service('file_locator'),
service('routing.loader.attribute'),
])
->tag('routing.loader', ['priority' => -10])

->alias('routing.loader.annotation.file', 'routing.loader.attribute.file')
->deprecate('symfony/routing', '6.4', 'The "%alias_id%" service is deprecated, use the "routing.loader.attribute.file" service instead.')

->set('routing.loader.psr4', Psr4DirectoryLoader::class)
->args([
service('file_locator'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ class AttributeRouteControllerLoader extends AttributeClassLoader
{
/**
* Configures the _controller default parameter of a given Route instance.
*
* @return void
*/
protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot)
protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot): void
Copy link
Member

Choose a reason for hiding this comment

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

This makes SymfonyBundle 6.4 incompatible with SymfonyRouter 7.0

PHP Fatal error: Declaration of Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader::configureRoute(Symfony\Component\Routing\Route $route, ReflectionClass $class, ReflectionMethod $method, object $annot) must be compatible with Symfony\Component\Routing\Loader\AttributeClassLoader::configureRoute(Symfony\Component\Routing\Route $route, ReflectionClass $class, ReflectionMethod $method, object $annot): void in /home/runner/work/aws/aws/vendor/symfony/framework-bundle/Routing/AttributeRouteControllerLoader.php on line 31

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the best way to mitigate this? Return types were added nearly everywhere but all constraints have been kept to ^6.4|^7.0.

Should this return type be removed until the next occasion we can add it?

Copy link
Member
@jderusse jderusse Oct 19, 2023

Choose a reason for hiding this comment

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

I see 2 options:

  • make Bundle require ^7.0
  • adds : void in Bundle:7.0 and in Route:8.0

I think the first is better.
ping @nicolas-grekas

Copy link
Member

Choose a reason for hiding this comment

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

I'm going to remove the added return type in 7.0. That's what we did with the previous name, but I forgot this part when renaming.

{
if ('__invoke' === $method->getName()) {
$route->setDefault('_controller', $class->getName());
Expand All @@ -51,7 +49,3 @@ protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMetho
return str_replace('__', '_', $name);
}
}

if (!class_exists(AnnotatedRouteControllerLoader::class, false)) {
class_alias(AttributeRouteControllerLoader::class, AnnotatedRouteControllerLoader::class);
}
4 changes: 3 additions & 1 deletion src/Symfony/Component/Routing/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ CHANGELOG

* Add argument `$routeParameters` to `UrlMatcher::handleRouteRequirements()`
* Remove Doctrine annotations support in favor of native attributes
* Change the constructor signature of `AnnotationClassLoader` to `__construct(?string $env = null)`, passing an annotation reader as first argument is not supported anymore
* Remove `AnnotationClassLoader`, use `AttributeClassLoader` instead
* Remove `AnnotationDirectoryLoader`, use `AttributeDirectoryLoader` instead
* Remove `AnnotationFileLoader`, use `AttributeFileLoader` instead

6.4
---
Expand Down
25 changes: 0 additions & 25 deletions src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php

This file was deleted.

168 changes: 39 additions & 129 deletions src/Symfony/Component/Routing/Loader/AttributeClassLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\Routing\Loader;

use Doctrine\Common\Annotations\Reader;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\Config\Loader\LoaderResolverInterface;
use Symfony\Component\Config\Resource\FileResource;
Expand Down Expand Up @@ -53,57 +52,18 @@
*/
abstract class AttributeClassLoader implements LoaderInterface
{
/**
* @var Reader|null
*
* @deprecated in Symfony 6.4, this property will be removed in Symfony 7.
*/
protected $reader;

/**
* @var string|null
*/
protected $env;

/**
* @var string
*/
protected $routeAnnotationClass = RouteAnnotation::class;
protected string $routeAnnotationClass = RouteAnnotation::class;
protected int $defaultRouteIndex = 0;

/**
* @var int
*/
protected $defaultRouteIndex = 0;

private bool $hasDeprecatedAnnotations = false;

/**
* @param string|null $env
*/
public function __construct($env = null)
{
if ($env instanceof Reader || null === $env && \func_num_args() > 1 && null !== func_get_arg(1)) {
trigger_deprecation('symfony/routing', '6.4', 'Passing an instance of "%s" as first and the environment as second argument to "%s" is deprecated. Pass the environment as first argument instead.', Reader::class, __METHOD__);

$this->reader = $env;
$env = \func_num_args() > 1 ? func_get_arg(1) : null;
}

if (\is_string($env) || null === $env) {
$this->env = $env;
} elseif ($env instanceof \Stringable || \is_scalar($env)) {
$this->env = (string) $env;
} else {
throw new \TypeError(__METHOD__.sprintf(': Parameter $env was expected to be a string or null, "%s" given.', get_debug_type($env)));
}
public function __construct(
protected readonly ?string $env = null,
) {
}

/**
* Sets the annotation class to read route properties from.
*
* @return void
*/
public function setRouteAnnotationClass(string $class)
public function setRouteAnnotationClass(string $class): void
{
$this->routeAnnotationClass = $class;
}
Expand All @@ -124,59 +84,47 @@ public function load(mixed $class, string $type = null): RouteCollection
throw new \InvalidArgumentException(sprintf('Annotations from class "%s" cannot be read as it is abstract.', $class->getName()));
}

$this->hasDeprecatedAnnotations = false;

try {
$globals = $this->getGlobals($class);
$collection = new RouteCollection();
$collection->addResource(new FileResource($class->getFileName()));
if ($globals['env'] && $this->env !== $globals['env']) {
return $collection;
}
$fqcnAlias = false;
foreach ($class->getMethods() as $method) {
$this->defaultRouteIndex = 0;
$routeNamesBefore = array_keys($collection->all());
foreach ($this->getAnnotations($method) as $annot) {
$this->addRoute($collection, $annot, $globals, $class, $method);
if ('__invoke' === $method->name) {
$fqcnAlias = true;
}
}

if (1 === $collection->count() - \count($routeNamesBefore)) {
$newRouteName = current(array_diff(array_keys($collection->all()), $routeNamesBefore));
$collection->addAlias(sprintf('%s::%s', $class->name, $method->name), $newRouteName);
}
}
if (0 === $collection->count() && $class->hasMethod('__invoke')) {
$globals = $this->resetGlobals();
foreach ($this->getAnnotations($class) as $annot) {
$this->addRoute($collection, $annot, $globals, $class, $class->getMethod('__invoke'));
$globals = $this->getGlobals($class);
$collection = new RouteCollection();
$collection->addResource(new FileResource($class->getFileName()));
if ($globals['env'] && $this->env !== $globals['env']) {
return $collection;
}
$fqcnAlias = false;
foreach ($class->getMethods() as $method) {
$this->defaultRouteIndex = 0;
$routeNamesBefore = array_keys($collection->all());
foreach ($this->getAnnotations($method) as $annot) {
$this->addRoute($collection, $annot, $globals, $class, $method);
if ('__invoke' === $method->name) {
$fqcnAlias = true;
}
}
if ($fqcnAlias && 1 === $collection->count()) {
$collection->addAlias($class->name, $invokeRouteName = key($collection->all()));
$collection->addAlias(sprintf('%s::__invoke', $class->name), $invokeRouteName);
}

if ($this->hasDeprecatedAnnotations) {
trigger_deprecation('symfony/routing', '6.4', 'Class "%s" uses Doctrine Annotations to configure routes, which is deprecated. Use PHP attributes instead.', $class->getName());
if (1 === $collection->count() - \count($routeNamesBefore)) {
$newRouteName = current(array_diff(array_keys($collection->all()), $routeNamesBefore));
$collection->addAlias(sprintf('%s::%s', $class->name, $method->name), $newRouteName);
}
} finally {
$this->hasDeprecatedAnnotations = false;
}
if (0 === $collection->count() && $class->hasMethod('__invoke')) {
$globals = $this->resetGlobals();
foreach ($this->getAnnotations($class) as $annot) {
$this->addRoute($collection, $annot, $globals, $class, $class->getMethod('__invoke'));
$fqcnAlias = true;
}
}
if ($fqcnAlias && 1 === $collection->count()) {
$collection->addAlias($class->name, $invokeRouteName = key($collection->all()));
$collection->addAlias(sprintf('%s::__invoke', $class->name), $invokeRouteName);
}

return $collection;
}

/**
* @param RouteAnnotation $annot or an object that exposes a similar interface
*
* @return void
*/
protected function addRoute(RouteCollection $collection, object $annot, array $globals, \ReflectionClass $class, \ReflectionMethod $method)
protected function addRoute(RouteCollection $collection, object $annot, array $globals, \ReflectionClass $class, \ReflectionMethod $method): void
{
if ($annot->getEnv() && $annot->getEnv() !== $this->env) {
return;
Expand Down Expand Up @@ -263,11 +211,7 @@ protected function addRoute(RouteCollection $collection, object $annot, array $g

public function supports(mixed $resource, string $type = null): bool
{
if ('annotation' === $type) {
trigger_deprecation('symfony/routing', '6.4', 'The "annotation" route type is deprecated, use the "attribute" route type instead.');
}

return \is_string($resource) && preg_match('/^(?:\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)+$/', $resource) && (!$type || \in_array($type, ['annotation', 'attribute'], true));
return \is_string($resource) && preg_match('/^(?:\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)+$/', $resource) && (!$type || 'attribute' === $type);
}

public function setResolver(LoaderResolverInterface $resolver): void
Expand All @@ -280,10 +224,8 @@ public function getResolver(): LoaderResolverInterface

/**
* Gets the default route name for a class method.
*
* @return string
*/
protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMethod $method)
protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMethod $method): string
{
$name = str_replace('\\', '_', $class->name).'_'.$method->name;
$name = \function_exists('mb_strtolower') && preg_match('//u', $name) ? mb_strtolower($name, 'UTF-8') : strtolower($name);
Expand All @@ -298,19 +240,13 @@ protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMetho
/**
* @return array<string, mixed>
*/
protected function getGlobals(\ReflectionClass $class)
protected function getGlobals(\ReflectionClass $class): array
{
$globals = $this->resetGlobals();

$annot = null;
if ($attribute = $class->getAttributes($this->routeAnnotationClass, \ReflectionAttribute::IS_INSTANCEOF)[0] ?? null) {
$annot = $attribute->newInstance();
}
if (!$annot && $annot = $this->reader?->getClassAnnotation($class, $this->routeAnnotationClass)) {
$this->hasDeprecatedAnnotations = true;
}

if ($annot) {
if (null !== $annot->getName()) {
$globals['name'] = $annot->getName();
}
Expand Down Expand Up @@ -380,18 +316,12 @@ private function resetGlobals(): array
];
}

/**
* @return Route
*/
protected function createRoute(string $path, array $defaults, array $requirements, array $options, ?string $host, array $schemes, array $methods, ?string $condition)
protected function createRoute(string $path, array $defaults, array $requirements, array $options, ?string $host, array $schemes, array $methods, ?string $condition): Route
{
return new Route($path, $defaults, $requirements, $options, $host, $schemes, $methods, $condition);
}

/**
* @return void
*/
abstract protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot);
abstract protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot): void;

/**
* @return iterable<int, RouteAnnotation>
Expand All @@ -401,25 +331,5 @@ private function getAnnotations(\ReflectionClass|\ReflectionMethod $reflection):
foreach ($reflection->getAttributes($this->routeAnnotationClass, \ReflectionAttribute::IS_INSTANCEOF) as $attribute) {
yield $attribute->newInstance();
}

if (!$this->reader) {
return;
}

$annotations = $reflection instanceof \ReflectionClass
? $this->reader->getClassAnnotations($reflection)
: $this->reader->getMethodAnnotations($reflection);

foreach ($annotations as $annotation) {
if ($annotation instanceof $this->routeAnnotationClass) {
$this->hasDeprecatedAnnotations = true;

yield $annotation;
}
}
}
}

if (!class_exists(AnnotationClassLoader::class, false)) {
class_alias(AttributeClassLoader::class, AnnotationClassLoader::class);
}
Loading
0