Skip to content

[make:decorator] Add new maker to create decorator - #1613

Closed
WedgeSama wants to merge 3 commits into
symfony:1.xfrom
WedgeSama:feature/make-decorator
Closed

[make:decorator] Add new maker to create decorator#1613
WedgeSama wants to merge 3 commits into
symfony:1.xfrom
WedgeSama:feature/make-decorator

Conversation

@WedgeSama

@WedgeSama WedgeSama commented Nov 10, 2024

Copy link
Copy Markdown
Contributor

Add new maker to create decorator.
#1401

Allow to create new decorator for existing services.

It try first to decorate by implements:

  • The service ID used is an interface => implements this interface
  • The service class implements one (or more) interface(s) and it do not declare more public method => implements those interfaces

If cannot implements, it will fallback to extends:

  • Simply extends the service's class
  • Do not work for final class

e.g.

bin/console make:decorator Symfony\Component\Routing\Generator\UrlGeneratorInterface MyUrlGenerator
<?php

namespace App;

use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\DependencyInjection\Attribute\AutowireDecorated;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Routing\RequestContext;

#[AsDecorator(UrlGeneratorInterface::class)]
final class MyUrlGenerator implements UrlGeneratorInterface
{
    public function __construct(
        #[AutowireDecorated]
        private readonly UrlGeneratorInterface $inner,
    ) {
    }

    public function generate(string $name, array $parameters = [], int $referenceType = self::ABSOLUTE_PATH): string
    {
        return $this->inner->generate($name, $parameters, $referenceType);
    }

    public function setContext(RequestContext $context): void
    {
        $this->inner->setContext($context);
    }

    public function getContext(): RequestContext
    {
        return $this->inner->getContext();
    }
}

@WedgeSama
WedgeSama marked this pull request as ready for review November 10, 2024 13:58
@WedgeSama
WedgeSama force-pushed the feature/make-decorator branch from c606c72 to dbbc516 Compare November 10, 2024 14:03
Comment thread src/Maker/MakeDecorator.php Outdated
Comment thread src/Maker/MakeDecorator.php
Comment thread tests/DependencyInjection/Fixtures/ServiceA.php
@WedgeSama
WedgeSama force-pushed the feature/make-decorator branch from c815fab to 2fb5feb Compare November 13, 2024 16:34
Comment thread src/Maker/MakeDecorator.php Outdated
…ity/onInvalid

Also fix:
- ignore `__construct`
- fix static method on decorate by implements
- Fix description
}

$shortClass = Str::getShortClassName($class);
$shortNameMap[$shortClass][] = $id;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
$shortNameMap[$shortClass][] = $id;
$shortNameMap[$shortClass] ??= [];
$shortNameMap[$shortClass][] = $id;

I know it works without but I d'ont find it very readable to not explictly create the array. Maybe it is acceptable in the maker bundle.

$serviceClasses[$id] = $class;
}

$shortNameMap = array_map(array_unique(...), $shortNameMap);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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


public function getRealId(string $id): ?string
{
if (\in_array($id, $this->ids)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
if (\in_array($id, $this->ids)) {
if (\in_array($id, $this->ids, true)) {

} else {
$input->setArgument(
'id',
$id = $io->choice(\sprintf('Multiple services found for "%s", choice which one you want to decorate?', $id), $guessRealIds),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
$id = $io->choice(\sprintf('Multiple services found for "%s", choice which one you want to decorate?', $id), $guessRealIds),
$id = $io->choice(\sprintf('Multiple services found for "%s", choose which one you want to decorate:', $id), $guessRealIds),

Comment on lines +96 to +103
foreach ($ref->getConstants(\ReflectionClassConstant::IS_PUBLIC) as $name => $value) {
if ($onInvalid === $value) {
$this->onInvalid = \sprintf('ContainerInterface::%s', $name);
$ok = true;
break;
}
$allowedValues[] = $value;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should probably be a static list IMO. IT didn't changed for quite some time and won't be changed anytime soon I think. Keep it simple.

} elseif ($parameter->isDefaultValueAvailable()) {
$defaultValue = $parameter->getDefaultValue();

if (\is_string($defaultValue)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is variadic also supported ?

return implode('&', array_map($this->parseType(...), $type->getTypes()));
}

throw new RuntimeCommandException('Should never be reach.');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
throw new RuntimeCommandException('Should never be reach.');
throw new RuntimeCommandException('Should never be reached.');

@GromNaN

GromNaN commented Aug 21, 2026

Copy link
Copy Markdown
Member

Thank you for this maker, it fits well within the project's scope for a feature that's genuinely tricky to get right by hand.

Since this branch conflicts with the config changes merged into 1.x since (XML config moved to PHP, help files restructured), I rebased it and applied the remaining review feedback from @Neirda24 (strict in_array, explicit short name map initialization, choice prompt wording, static map for onInvalid constants, variadic parameter support, exception message typo) in #1807.

Feel free to have a look, and let me know if you'd rather continue on this branch instead.

GromNaN added a commit that referenced this pull request Aug 30, 2026
…geSama)

This PR was merged into the 1.x branch.

Discussion
----------

[make:decorator] Add new maker to create decorator

Rebase and continuation of #1613 by `@WedgeSama`, updated for the current `1.x` codebase (config moved from XML to PHP, help files restructured) and with the remaining review feedback from `@Neirda24` applied:

- Fixed the `getRealId()` strict comparison (`in_array` with strict mode).
- Initialized the short name map entries explicitly instead of relying on implicit array creation, and replaced the array_map/array_unique callable-first-class syntax with a plain foreach.
- Fixed the choice prompt wording ("choose which one you want to decorate:").
- Replaced the `ReflectionClass` lookup of `ContainerInterface` constants with a static map for the `onInvalid` option.
- Added support for variadic parameters when parsing methods to decorate (previously produced an incorrect signature and a broken forwarding call).
- Fixed a typo in an internal exception message.

Add a `make:decorator` command that generates a decorator class for an existing service. The maker asks for the service to decorate and the class name of the decorator, then generates a class that either implements the decorated service interfaces or extends its class.

```shell
bin/console make:decorator Symfony\Component\Routing\Generator\UrlGeneratorInterface MyUrlGenerator
```

```php
<?php

namespace App;

use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\DependencyInjection\Attribute\AutowireDecorated;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Routing\RequestContext;

#[AsDecorator(UrlGeneratorInterface::class)]
final class MyUrlGenerator implements UrlGeneratorInterface
{
    public function __construct(
        #[AutowireDecorated]
        private readonly UrlGeneratorInterface $inner,
    ) {
    }

    public function generate(string $name, array $parameters = [], int $referenceType = self::ABSOLUTE_PATH): string
    {
        return $this->inner->generate($name, $parameters, $referenceType);
    }

    public function setContext(RequestContext $context): void
    {
        $this->inner->setContext($context);
    }

    public function getContext(): RequestContext
    {
        return $this->inner->getContext();
    }
}
```

Closes #1401

Commits
-------

ca9ce46 [make:decorator] Add new maker to create decorator
@GromNaN GromNaN closed this Aug 30, 2026
@WedgeSama

Copy link
Copy Markdown
Contributor Author

Sorry, I was in holiday 😅
No problem @GromNaN 😉

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants