ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Component/Notifier/Bridge/Smsbox/Enum/Mode.php (PHP)

Code before:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\Notifier\Bridge\Smsbox\Enum;

enum Mode: string
{
    case Standard = 'Standard';
    case Expert = 'Expert';
    case Response = 'Reponse'; # This is not a typo, it's the French word for "response" which is being used by the API (see https://en.smsbox.net/docs/doc-API-SMSBOX-1.1-EN.pdf)
}


Review comment (suggestion):
Yes, I would keep it simpler:

```suggestion
    case Response = 'Reponse'; // not a typo, see https://en.smsbox.net/docs/doc-API-SMSBOX-1.1-EN.pdf
```

Code after:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\Notifier\Bridge\Smsbox\Enum;

enum Mode: string
{
    case Standard = 'Standard';
    case Expert = 'Expert';
    case Response = 'Reponse';
}
Repository: symfony/symfony
File: src/Symfony/Component/Security/Http/Authenticator/Passport/Badge/UserBadge.php (PHP)

Code before:

            throw $exception;
        }

        if (!$user instanceof UserInterface) {
            throw new AuthenticationServiceException(\sprintf('The user provider must return a UserInterface object, "%s" given.', get_debug_type($user)));
        }

        return $this->user = $user;
    }

    public function getUserLoader(): ?callable
    {
        return $this->userLoader;
    }

    public function setUserLoader(callable $userLoader): void
    {
        $this->userLoader = $userLoader;
    }

    public function isResolved(): bool
    {
        return true;
    }
}


Review comment (security):
```suggestion
            trigger_deprecation('symfony/security-http', '7.3', 'Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
```
?

Code after:
        if (!$user instanceof UserInterface) {
            throw new AuthenticationServiceException(\sprintf('The user provider must return a UserInterface object, "%s" given.', get_debug_type($user)));
        }

        return $this->user = $user;
    }

    public function getUserLoader(): ?callable
    {
        return $this->userLoader;
    }

    public function setUserLoader(callable $userLoader): void
    {
        $this->userLoader = $userLoader;
    }

    public function isResolved(): bool
    {
        return true;
    }

    private function validateUserIdentifier(string $userIdentifier): void
    {
        if ('' === $userIdentifier) {
            trigger_deprecation('symfony/security-http', '7.2', 'Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
            // throw new BadCredentialsException('Empty user identifier.');
        }

        if (\strlen($userIdentifier) > self::MAX_USERNAME_LENGTH) {
            throw new BadCredentialsException('Username too long.');
        }
    }
}
Repository: symfony/symfony
File: src/Symfony/Bundle/FrameworkBundle/Command/CachePoolPruneCommand.php (PHP)

Code before:
        $this->pools = $pools;
    }

    protected function configure(): void
    {
        $this
            ->setHelp(<<<'EOF'
The <info>%command.name%</info> command deletes all expired items from all pruneable pools.

    %command.full_name%
EOF
            )
        ;
    }

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        $io = new SymfonyStyle($input, $output);
        $hadFailure = false;

        foreach ($this->pools as $name => $pool) {
            $io->comment(\sprintf('Pruning cache pool: <info>%s</info>', $name));
            if (!$pool->prune()) {
                $io->error(\sprintf('Cache pool "%s" could not be pruned.', $name));

                $hadFailure = true;
            }
        }

        if ($hadFailure) {
            return 1;
        }

        $io->success('Successfully pruned cache pool(s).');

        return 0;
    }
}


Review comment (performance):
The issue with this approach is that once a generic decorator is added to a pool (like the traceable one), we'll call that method even if the inner pool doesn't have the interface.
In order to go with this behavior, we'd first need to fix CachePoolPrunerPass: that pass uses `$reflection->implementsInterface(PruneableInterface::class)` to decide if a pool should be injected into the pruning command.
That's a mistake. No DI behavior should be directly related to implementing any specific interface (except in fallback code paths).
(this pass has a second minor issue: resolveValue is called twice, the second time being inside the call to getReflectionClass)
A solution might be to make `CachePoolPass` manage a `prunable` attribute on `cache.pool` tags. If not explicitly defined, the attribute could fallback to an interface check.

Code after:
        $this->pools = $pools;
    }

    protected function configure(): void
    {
        $this
            ->setHelp(<<<'EOF'
The <info>%command.name%</info> command deletes all expired items from all pruneable pools.

    %command.full_name%
EOF
            )
        ;
    }

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        $io = new SymfonyStyle($input, $output);
        $exitCode = Command::SUCCESS;

        foreach ($this->pools as $name => $pool) {
            $io->comment(\sprintf('Pruning cache pool: <info>%s</info>', $name));

            if (!$pool->prune()) {
                $io->error(\sprintf('Cache pool "%s" could not be pruned.', $name));
                $exitCode = Command::FAILURE;
            }
        }

        if (Command::SUCCESS === $exitCode) {
            $io->success('Successfully pruned cache pool(s).');
        }

        return $exitCode;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/DependencyInjection/ReverseMappingPass.php (PHP)

Code before:

namespace Symfony\Component\ObjectMapper\DependencyInjection;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\ObjectMapper\Attribute\Map;
use Symfony\Component\ObjectMapper\ObjectMapperInterface;

/**
 * Registers map classes from {@see Map} attribute.
 *
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseMappingPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if (!interface_exists(ObjectMapperInterface::class) || !$container->hasDefinition('object_mapper.metadata_factory.reverse_class')) {
            return;
        }

        $reverseClassObjectMapperMetadataFactory = $container->getDefinition('object_mapper.metadata_factory.reverse_class');

        $classes = [];
        foreach ($container->getDefinitions() as $definition) {
            if ($definition->hasTag('object_mapper.map')) {
                $attributes = $definition->getTag('object_mapper.map')[0];

                if (isset($attributes['source'])) {
                    $classes[$attributes['source']] = $attributes['target'];
                }
            }
        }

        $reverseClassObjectMapperMetadataFactory->replaceArgument(1, $classes);
    }
}


Review comment (security):
Use `$container->findTaggedResourceIds()` instead of reimplementing it manually. The 8.1 branch does not need to care for support for `symfony/dependency-injection < 7.2` IMO.

Code after:

namespace Symfony\Component\ObjectMapper\DependencyInjection;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseMappingPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if (!$container->hasDefinition('object_mapper.metadata_factory.reverse_class')) {
            return;
        }

        $reverseClassObjectMapperMetadataFactory = $container->getDefinition('object_mapper.metadata_factory.reverse_class');

        $classes = [];
        foreach ($container->findTaggedResourceIds('object_mapper.map') as $tags) {
            foreach ($tags as $tag) {
                if (isset($tag['source'], $tag['target'])) {
                    $classes[$tag['source']] = $tag['target'];
                }
            }
        }

        $reverseClassObjectMapperMetadataFactory->replaceArgument(1, $classes);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/DependencyInjection/ReverseMappingPass.php (PHP)

Code before:

namespace Symfony\Component\ObjectMapper\DependencyInjection;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * Registers map classes from {@see Map} attribute.
 *
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseMappingPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if (!$container->hasDefinition('object_mapper.metadata_factory.reverse_class')) {
            return;
        }

        $reverseClassObjectMapperMetadataFactory = $container->getDefinition('object_mapper.metadata_factory.reverse_class');

        $classes = [];
        foreach ($container->findTaggedResourceIds('object_mapper.map') as $tags) {
            foreach ($tags as $tag) {
                if (isset($tag['source'])) {
                    $classes[$tag['source']] = $tag['target'];
                }
            }
        }

        $reverseClassObjectMapperMetadataFactory->replaceArgument(1, $classes);
    }
}


Review comment (suggestion):
```suggestion
                if (isset($tag['source']) && isset($tag['target'])) {
```


Code after:

namespace Symfony\Component\ObjectMapper\DependencyInjection;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseMappingPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if (!$container->hasDefinition('object_mapper.metadata_factory.reverse_class')) {
            return;
        }

        $reverseClassObjectMapperMetadataFactory = $container->getDefinition('object_mapper.metadata_factory.reverse_class');

        $classes = [];
        foreach ($container->findTaggedResourceIds('object_mapper.map') as $tags) {
            foreach ($tags as $tag) {
                if (isset($tag['source'], $tag['target'])) {
                    $classes[$tag['source']] = $tag['target'];
                }
            }
        }

        $reverseClassObjectMapperMetadataFactory->replaceArgument(1, $classes);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Metadata/ReverseClassObjectMapperMetadataFactory.php (PHP)

Code before:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Metadata;

use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseClassObjectMapperMetadataFactory implements ObjectMapperMetadataFactoryInterface
{
    /**
     * @var array<string, Mapping[]>
     */
    private array $attributesCache = [];

    public function __construct(
        private readonly ObjectMapperMetadataFactoryInterface $objectMapperMetadataFactory,
        /** @param array<int, class-string> $classMap */
        private readonly array $classMap,
    ) {
    }

    public function create(object $object, ?string $property = null, array $context = []): array
    {
        $class = $object::class;
        $key = $class.($property ? '.'.$property : '');

        if (isset($this->attributesCache[$key])) {
            return $this->attributesCache[$key];
        }

        $mappings = $this->objectMapperMetadataFactory->create($object, $property, $context);

        if (!$targetClass = $this->classMap[$class] ?? null) {
            return $mappings;
        }


Review comment (suggestion):
```suggestion
     * @var array<string, list<Mapping>>
```
To fit with `ObjectMapperMetadataFactoryInterface` documentation

Code after:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Metadata;

use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * Maps classes based on attributes found on the target's properties.
 *
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseClassObjectMapperMetadataFactory implements ObjectMapperMetadataFactoryInterface
{
    /**
     * @var array<string, list<Mapping>>
     */
    private array $attributesCache = [];

    /**
     * @param array<class-string, class-string> $classMap
     */
    public function __construct(
        private readonly ObjectMapperMetadataFactoryInterface $objectMapperMetadataFactory,
        private readonly array $classMap,
    ) {
    }

    public function create(object $object, ?string $property = null, array $context = []): array
    {
        $class = $object::class;
        $key = $class.($property ? '.'.$property : '');

        if (isset($this->attributesCache[$key])) {
            return $this->attributesCache[$key];
        }

        $mappings = $this->objectMapperMetadataFactory->create($object, $property, $context);
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Metadata/ReverseClassObjectMapperMetadataFactory.php (PHP)

Code before:

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Metadata;

use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseClassObjectMapperMetadataFactory implements ObjectMapperMetadataFactoryInterface
{
    /**
     * @var array<string, list<Mapping>>
     */
    private array $attributesCache = [];

    /**
     * @param array<class-string, class-string $classMap
     */
    public function __construct(
        private readonly ObjectMapperMetadataFactoryInterface $objectMapperMetadataFactory,
        private readonly array $classMap,
    ) {
    }

    public function create(object $object, ?string $property = null, array $context = []): array
    {
        $class = $object::class;
        $key = $class.($property ? '.'.$property : '');

        if (isset($this->attributesCache[$key])) {
            return $this->attributesCache[$key];
        }

        $mappings = $this->objectMapperMetadataFactory->create($object, $property, $context);

        if (!$targetClass = $this->classMap[$class] ?? null) {
            return $mappings;
        }

        if (!$property) {
            $mappings[] = new Mapping($targetClass);


Review comment (suggestion):
```suggestion
     * @param array<class-string, class-string> $classMap
```

Code after:

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Metadata;

use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * Maps classes based on attributes found on the target's properties.
 *
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseClassObjectMapperMetadataFactory implements ObjectMapperMetadataFactoryInterface
{
    /**
     * @var array<string, list<Mapping>>
     */
    private array $attributesCache = [];

    /**
     * @param array<class-string, class-string> $classMap
     */
    public function __construct(
        private readonly ObjectMapperMetadataFactoryInterface $objectMapperMetadataFactory,
        private readonly array $classMap,
    ) {
    }

    public function create(object $object, ?string $property = null, array $context = []): array
    {
        $class = $object::class;
        $key = $class.($property ? '.'.$property : '');

        if (isset($this->attributesCache[$key])) {
            return $this->attributesCache[$key];
        }

        $mappings = $this->objectMapperMetadataFactory->create($object, $property, $context);

        if (!$targetClass = $this->classMap[$class] ?? null) {
            return $mappings;
        }

        if (!$property) {
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/DependencyInjection/ReverseMappingPass.php (PHP)

Code before:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\DependencyInjection;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * Registers map classes from {@see Map} attribute.
 *
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseMappingPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if (!$container->hasDefinition('object_mapper.metadata_factory.reverse_class')) {
            return;
        }

        $reverseClassObjectMapperMetadataFactory = $container->getDefinition('object_mapper.metadata_factory.reverse_class');

        $classes = [];
        foreach ($container->findTaggedResourceIds('object_mapper.map') as $tags) {
            foreach ($tags as $tag) {
                if (isset($tag['source'], $tag['target'])) {
                    $classes[$tag['source']] = $tag['target'];
                }
            }
        }

        $reverseClassObjectMapperMetadataFactory->replaceArgument(1, $classes);
    }
}


Review comment (suggestion):
The code doesn't care about the php attribute so that this title isn't accurate enough:

```suggestion
```

Code after:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\DependencyInjection;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseMappingPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if (!$container->hasDefinition('object_mapper.metadata_factory.reverse_class')) {
            return;
        }

        $reverseClassObjectMapperMetadataFactory = $container->getDefinition('object_mapper.metadata_factory.reverse_class');

        $classes = [];
        foreach ($container->findTaggedResourceIds('object_mapper.map') as $tags) {
            foreach ($tags as $tag) {
                if (isset($tag['source'], $tag['target'])) {
                    $classes[$tag['source']] = $tag['target'];
                }
            }
        }

        $reverseClassObjectMapperMetadataFactory->replaceArgument(1, $classes);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Metadata/ReverseClassObjectMapperMetadataFactory.php (PHP)

Code before:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Metadata;

use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseClassObjectMapperMetadataFactory implements ObjectMapperMetadataFactoryInterface
{
    /**
     * @var array<string, list<Mapping>>
     */
    private array $attributesCache = [];

    /**
     * @param array<class-string, class-string $classMap
     */
    public function __construct(
        private readonly ObjectMapperMetadataFactoryInterface $objectMapperMetadataFactory,
        private readonly array $classMap,
    ) {
    }

    public function create(object $object, ?string $property = null, array $context = []): array
    {
        $class = $object::class;
        $key = $class.($property ? '.'.$property : '');

        if (isset($this->attributesCache[$key])) {
            return $this->attributesCache[$key];
        }

        $mappings = $this->objectMapperMetadataFactory->create($object, $property, $context);

Review comment (security):
```suggestion
 * Maps classes based on attributes found on the target's properties.
 *
 * @author Florent Blaison <florent.blaison@gmail.com>
```

Code after:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Metadata;

use Symfony\Component\ObjectMapper\Attribute\Map;

/**
 * Maps classes based on attributes found on the target's properties.
 *
 * @author Florent Blaison <florent.blaison@gmail.com>
 */
final class ReverseClassObjectMapperMetadataFactory implements ObjectMapperMetadataFactoryInterface
{
    /**
     * @var array<string, list<Mapping>>
     */
    private array $attributesCache = [];

    /**
     * @param array<class-string, class-string> $classMap
     */
    public function __construct(
        private readonly ObjectMapperMetadataFactoryInterface $objectMapperMetadataFactory,
        private readonly array $classMap,
    ) {
    }

    public function create(object $object, ?string $property = null, array $context = []): array
    {
        $class = $object::class;
        $key = $class.($property ? '.'.$property : '');

        if (isset($this->attributesCache[$key])) {
            return $this->attributesCache[$key];
        }