8000 [TwigBundle] Improve error when autoconfiguring a class with both `ExtensionInterface` and Twig callable attribute by GromNaN · Pull Request #60415 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[TwigBundle] Improve error when autoconfiguring a class with both ExtensionInterface and Twig callable attribute #60415

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.

Sign up for GitHub

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
May 14, 2025
Merged

[TwigBundle] Improve error when autoconfiguring a class with both ExtensionInterface and Twig callable attribute #60415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/AttributeExtensionPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Twig\Attribute\AsTwigFilter;
use Twig\Attribute\AsTwigFunction;
use Twig\Attribute\AsTwigTest;
use Twig\Extension\AbstractExtension;
use Twig\Extension\AttributeExtension;
use Twig\Extension\ExtensionInterface;

/**
* Register an instance of AttributeExtension for each service using the
Expand All @@ -33,6 +36,14 @@ final class AttributeExtensionPass implements CompilerPassInterface

public static function autoconfigureFromAttribute(ChildDefinition $definition, AsTwigFilter|AsTwigFunction|AsTwigTest $attribute, \ReflectionMethod $reflector): void
{
$class = $reflector->getDeclaringClass();
if ($class->implementsInterface(ExtensionInterface::class)) {
if ($class->isSubclassOf(AbstractExtension::class)) {
throw new LogicException(\sprintf('The class "%s" cannot extend "%s" and use the "#[%s]" attribute on method "%s()", choose one or the other.', $class->name, AbstractExtension::class, $attribute::class, $reflector->name));
}
throw new LogicException(\sprintf('The class "%s" cannot implement "%s" and use the "#[%s]" attribute on method "%s()", choose one or the other.', $class->name, ExtensionInterface::class, $attribute::class, $reflector->name));
}

$definition->addTag(self::TAG);

// The service must be tagged as a runtime to call non-static methods
Expand Down
76 changes: 61 additions & 15 deletions src/Symfony/Bundle/TwigBundle/Tests/Functional/AttributeExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,39 @@

namespace Symfony\Bundle\TwigBundle\Tests\Functional;

use PHPUnit\Framework\Attributes\After;
use PHPUnit\Framework\Attributes\Before;
use PHPUnit\Framework\Attributes\BeforeClass;
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
use Symfony\Bundle\TwigBundle\Tests\TestCase;
use Symfony\Bundle\TwigBundle\TwigBundle;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\HttpKernel\Kernel;
use Twig\Attribute\AsTwigFilter;
use Twig\Attribute\AsTwigFunction;
use Twig\Attribute\AsTwigTest;
use Twig\Environment;
use Twig\Error\RuntimeError;
use Twig\Extension\AbstractExtension;
use Twig\Extension\AttributeExtension;

class AttributeExtensionTest extends TestCase
{
public function testExtensionWithAttributes()
/** @beforeClass */
Copy link
Member

Choose a reason for hiding this comment

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

Please also add the PHPUnit attribute, to be compatible with PHPUnit 11

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, attribute added.

I've also seen the pre condition attributes, but none of them can check a composer package version or if a class exists: https://docs.phpunit.de/en/10.5/writing-tests-for-phpunit.html#skipping-tests-using-attributes

Copy link
Member

Choose a reason for hiding this comment

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

Looks weird that they don't have a RequiresClass attribute (and probably RequiresInterface). It would look consistent with the other attributes they have.

#[BeforeClass]
public static function assertTwigVersion(): void
{
if (!class_exists(AttributeExtension::class)) {
self::markTestSkipped('Twig 3.21 is required.');
}
}

$kernel = new class('test', true) extends Kernel
{
public function registerBundles(): iterable
{
return [new FrameworkBundle(), new TwigBundle()];
}

public function testExtensionWithAttributes()
{
$kernel = new class extends AttributeExtensionKernel {
public function registerContainerConfiguration(LoaderInterface $loader): void
{
$loader->load(static function (ContainerBuilder $container) {
Expand All @@ -53,11 +57,6 @@ public function registerContainerConfiguration(LoaderInterface $loader): void
$container->setAlias('twig_test', 'twig')->setPublic(true);
});
}

public function getProjectDir(): string
{
return sys_get_temp_dir().'/'.Kernel::VERSION.'/AttributeExtension';
}
};

$kernel->boot();
Expand All @@ -73,10 +72,30 @@ public function getProjectDir(): string
$twig->getRuntime(StaticExtensionWithAttributes::class);
}

public function testInvalidExtensionClass()
{
$kernel = new class extends AttributeExtensionKernel {
public function registerContainerConfiguration(LoaderInterface $loader): void
{
$loader->load(static function (ContainerBuilder $container) {
$container->register(InvalidExtensionWithAttributes::class, InvalidExtensionWithAttributes::class)
->setAutoconfigured(true);
});
}
};

$this->expectException(LogicException::class);
$this->expectExceptionMessage('The class "Symfony\Bundle\TwigBundle\Tests\Functional\InvalidExtensionWithAttributes" cannot extend "Twig\Extension\AbstractExtension" and use the "#[Twig\Attribute\AsTwigFilter]" attribute on method "funFilter()", choose one or the other.');

$kernel->boot();
}


/**
* @before
* @after
*/
#[Before, After]
protected function deleteTempDir()
{
if (file_exists($dir = sys_get_temp_dir().'/'.Kernel::VERSION.'/AttributeExtension')) {
Expand All @@ -85,6 +104,24 @@ protected function deleteTempDir()
}
}

abstract class AttributeExtensionKernel extends Kernel
{
public function __construct()
{
parent::__construct('test', true);
}

public function registerBundles(): iterable
{
return [new FrameworkBundle(), new TwigBundle()];
}

public function getProjectDir(): string
{
return sys_get_temp_dir().'/'.Kernel::VERSION.'/AttributeExtension';
}
}

class StaticExtensionWithAttributes
{
#[AsTwigFilter('foo')]
Expand Down Expand Up @@ -112,10 +149,19 @@ public function __construct(private bool $prefix)
{
}

#[AsTwigFilter('foo')]
#[AsTwigFunction('foo')]
#[AsTwigFilter('prefix_foo')]
#[AsTwigFunction('prefix_foo')]
public function prefix(string $value): string
{
return $this->prefix.$value;
}
}

class InvalidExtensionWithAttributes extends AbstractExtension
{
#[AsTwigFilter('fun')]
public function funFilter(): string
{
return 'fun';
}
}
Loading
0