ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveClassPassTest.php (PHP)

Code before:
        $container = new ContainerBuilder();
        $container->register('Acme\UnknownClass');

        (new ResolveClassPass())->process($container);
    }

    #[IgnoreDeprecations]
    #[Group('legacy')]
    public function testInvalidClassWhoseImplementedInterfaceIsMissingDefinition()
    {
        $this->expectUserDeprecationMessage('Since symfony/dependency-injection 7.4: Service id "Acme\ClassImplementsUnavailableInterface" looks like a FQCN but no corresponding class or interface exists. To resolve this ambiguity, please rename this service to a non-FQCN (e.g. using dots), or create the missing class or interface.');

        $autoloader = function (string $class) {
            if ('Acme\ClassImplementsUnavailableInterface' === $class) {
                new class implements UnavailableInterface {};
            }
        };

        spl_autoload_register($autoloader);

        $container = new ContainerBuilder();
        $container->register('Acme\ClassImplementsUnavailableInterface');

        (new ResolveClassPass())->process($container);

        spl_autoload_unregister($autoloader);
    }
}


Review comment (bug):
this should be in a `finally` block to ensure it is not linked even in case of failure.

Alternatively, define the class in a file that can be loaded through PSR-4, removing the need for this autoloader entirely.

Code after:
        $container = new ContainerBuilder();
        $container->register('Acme\UnknownClass');

        (new ResolveClassPass())->process($container);
    }

    #[IgnoreDeprecations]
    #[Group('legacy')]
    public function testInvalidClassWhoseImplementedInterfaceIsMissingDefinition()
    {
        $this->expectUserDeprecationMessage('Since symfony/dependency-injection 7.4: Service id "Acme\ClassImplementsUnavailableInterface" looks like a FQCN but no corresponding class or interface exists. To resolve this ambiguity, please rename this service to a non-FQCN (e.g. using dots), or create the missing class or interface.');

        $autoloader = function (string $class) {
            if ('Acme\ClassImplementsUnavailableInterface' === $class) {
                new class implements UnavailableInterface {};
            }
        };

        spl_autoload_register($autoloader);

        try {
            $container = new ContainerBuilder();
            $container->register('Acme\ClassImplementsUnavailableInterface');

            (new ResolveClassPass())->process($container);
        } finally {
            spl_autoload_unregister($autoloader);
        }
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Console/Command/Command.php (PHP)

Code before:

    private function getCommandAttribute(?callable $code): ?AsCommand
    {
        if (null === $code) {
            /** @var AsCommand|null $attribute */
            $attribute = (new \ReflectionClass(static::class)->getAttributes(AsCommand::class)[0] ?? null)?->newInstance();

            return $attribute;
        }

        $reflection = new \ReflectionFunction($code(...));

        if ($reflection->isAnonymous() || !$class = $reflection->getClosureScopeClass()) {
            throw new InvalidArgumentException(\sprintf('The command must be an instance of "%s", an invokable object or method.', self::class));
        }

        /** @var AsCommand|null $attribute */
        $attribute = ($reflection->getAttributes(AsCommand::class)[0] ?? null)?->newInstance();

        if (!$attribute && $reflection->getName() === '__invoke') {
            /** @var AsCommand|null $attribute */
            $attribute = ($class->getAttributes(AsCommand::class)[0] ?? null)?->newInstance();
        }

        if (!$attribute) {
            throw new LogicException(\sprintf('The command must use the "%s" attribute.', AsCommand::class));
        }

        return $attribute;
    }
}


Review comment (suggestion):
We should reject adding the attribute on the method `__invoke` and always read the class attribute.
If the attribute is added on both the `__invoke` method and the class, the behavior of which attribute is read is not obvious; and we will end with different behaviors in different integrations.

Code after:
    private function validateName(string $name): void
    {
        if (!preg_match('/^[^\:]++(\:[^\:]++)*$/', $name)) {
            throw new InvalidArgumentException(\sprintf('Command name "%s" is invalid.', $name));
        }
    }

    private function getCommandAttribute(?callable $code): ?AsCommand
    {
        if (null === $code) {
            /** @var AsCommand|null $attribute */
            $attribute = (new \ReflectionClass(static::class)->getAttributes(AsCommand::class)[0] ?? null)?->newInstance();

            return $attribute;
        }

        $reflection = new \ReflectionFunction($code(...));

        if ($reflection->isAnonymous() || !$class = $reflection->getClosureScopeClass()) {
            throw new InvalidArgumentException(\sprintf('The command must be an instance of "%s", an invokable object or a method of an object.', self::class));
        }

        /** @var AsCommand|null $attribute */
        $attribute = ($reflection->getAttributes(AsCommand::class)[0] ?? null)?->newInstance();

        if (!$attribute && $reflection->getName() === '__invoke') {
            /** @var AsCommand|null $attribute */
            $attribute = ($class->getAttributes(AsCommand::class)[0] ?? null)?->newInstance();
        }

        if (!$attribute) {
            throw new LogicException(\sprintf('The command must use the "%s" attribute.', AsCommand::class));
        }

        return $attribute;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Console/Command/TraceableCommand.php (PHP)

Code before:

    public function getHelperSet(): ?HelperSet
    {
        return $this->command->getHelperSet();
    }

    public function isEnabled(): bool
    {
        return $this->command->isEnabled();
    }

    public function complete(CompletionInput $input, CompletionSuggestions $suggestions): void
    {
        $this->command->complete($input, $suggestions);
    }

    /**
     * {@inheritdoc}
     *
     * Calling parent method is required to be used in {@see parent::run()}.
     */
    public function setCode(callable $code): static
    {
        if ($code instanceof InvokableCommand) {
            $r = new \ReflectionFunction(\Closure::bind(function () {
                return $this->code;
            }, $code, InvokableCommand::class)());

            $this->invokableCommandInfo = [
                'class' => $r->getClosureScopeClass()->name,
                'file' => $r->getFileName(),
                'line' => $r->getStartLine(),
            ];
        }

        $this->command->setCode($code);

        return parent::setCode(function (InputInterface $input, OutputInterface $output) use ($code): int {
            $event = $this->stopwatch->start($this->getName().'.code');

            $this->exitCode = $code($input, $output);

            $event->stop();

            return $this->exitCode;
        });
    }

    /**
     * @internal
     */

Review comment (suggestion):
I think `$code->getCode()` should work. This method was added later by https://github.com/symfony/symfony/pull/61078

Code after:

    public function getHelperSet(): ?HelperSet
    {
        return $this->command->getHelperSet();
    }

    public function isEnabled(): bool
    {
        return $this->command->isEnabled();
    }

    public function complete(CompletionInput $input, CompletionSuggestions $suggestions): void
    {
        $this->command->complete($input, $suggestions);
    }

    /**
     * {@inheritdoc}
     *
     * Calling parent method is required to be used in {@see parent::run()}.
     */
    public function setCode(callable $code): static
    {
        if ($code instanceof InvokableCommand) {
            $r = \Closure::bind(function () {
                return $this->invokable;
            }, $code, InvokableCommand::class)();

            $this->invokableCommandInfo = [
                'class' => $r->getClosureScopeClass()->name,
                'file' => $r->getFileName(),
                'line' => $r->getStartLine(),
            ];
        }

        $this->command->setCode($code);

        return parent::setCode(function (InputInterface $input, OutputInterface $output) use ($code): int {
            $event = $this->stopwatch->start($this->getName().'.code');

            $this->exitCode = $code($input, $output);

            $event->stop();

            return $this->exitCode;
        });
    }

    /**
     * @internal
     */
Repository: symfony/symfony
File: src/Symfony/Component/Form/Flow/Type/NavigatorFlowType.php (PHP)

Code before:

/**
 * A navigator type that defines default buttons to interact with a form flow.
 *
 * @author Yonel Ceruto <open@yceruto.dev>
 */
class NavigatorFlowType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder->add('previous', PreviousFlowType::class);
        $builder->add('next', NextFlowType::class);
        $builder->add('finish', FinishFlowType::class);

        if ($options['with_reset']) {
            $builder->add('reset', ResetFlowType::class);
        }
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'label' => false,
            'mapped' => false,
            'priority' => -100,
            'with_reset' => false,
        ]);
    }
}


Review comment (suggestion):
let's define this new option with all properties? 
```php
$resolver->define('with_reset')
    ->allowedTypes('bool')
    ->default(false)
    ->info('...')
```

Code after:

/**
 * A navigator type that defines default buttons to interact with a form flow.
 *
 * @author Yonel Ceruto <open@yceruto.dev>
 */
class NavigatorFlowType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder->add('previous', PreviousFlowType::class);
        $builder->add('next', NextFlowType::class);
        $builder->add('finish', FinishFlowType::class);

        if ($options['with_reset']) {
            $builder->add('reset', ResetFlowType::class);
        }
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'label' => false,
            'mapped' => false,
            'priority' => -100,
        ]);

        $resolver->define('with_reset')
            ->allowedTypes('bool')
            ->default(false)
            ->info('Whether to add a reset button to restart the flow from the first step');
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Form/Extension/Core/DataTransformer/ArrayToPartsTransformer.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\Form\Extension\Core\DataTransformer;

use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;

/**
 * @author Bernhard Schussek <bschussek@gmail.com>
 *
 * @implements DataTransformerInterface<array, array>
 */
class ArrayToPartsTransformer implements DataTransformerInterface
{
    public function __construct(
        private array $partMapping,
    ) {
    }

    public function transform(mixed $value): mixed
    {
        if (!\is_array($value ??= [])) {
            throw new TransformationFailedException('Expected an array.');
        }

        $result = [];

        foreach ($this->partMapping as $partKey => $originalKeys) {
            if (!$value) {
                $result[$partKey] = null;
            } else {
                $result[$partKey] = array_intersect_key($value, array_flip($originalKeys));
            }
        }

        return $result;
    }

    public function reverseTransform(mixed $value): mixed
    {
        if (!\is_array($value)) {
            throw new TransformationFailedException('Expected an array.');
        }

        $result = [];

Review comment (suggestion):
I am not convinced we should rename the arguments. It will mainly cause conflicts when merging up while not adding real value.

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\Form\Extension\Core\DataTransformer;

use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;

/**
 * @author Bernhard Schussek <bschussek@gmail.com>
 *
 * @implements DataTransformerInterface<array, array>
 */
class ArrayToPartsTransformer implements DataTransformerInterface
{
    public function __construct(
        private array $partMapping,
    ) {
    }

    public function transform(mixed $array): mixed
    {
        if (!\is_array($array ??= [])) {
            throw new TransformationFailedException('Expected an array.');
        }

        $result = [];

        foreach ($this->partMapping as $partKey => $originalKeys) {
            if (!$array) {
                $result[$partKey] = null;
            } else {
                $result[$partKey] = array_intersect_key($array, array_flip($originalKeys));
            }
        }

        return $result;
    }

    public function reverseTransform(mixed $array): mixed
    {
        if (!\is_array($array)) {
            throw new TransformationFailedException('Expected an array.');
        }

        $result = [];
Repository: symfony/symfony
File: src/Symfony/Component/Config/Definition/Configuration.php (PHP)

Code before:
use Symfony\Component\Config\Definition\Configurator\DefinitionConfigurator;
use Symfony\Component\Config\Definition\Loader\DefinitionFileLoader;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
 * @author Yonel Ceruto <yonelceruto@gmail.com>
 *
 * @final
 */
class Configuration implements ConfigurationInterface
{
    public function __construct(
        private ConfigurableInterface $subject,
        private ?ContainerBuilder $container,
        private string $alias,
    ) {
    }

    /**
     * @return TreeBuilder<'array'>
     */
    public function getConfigTreeBuilder(): TreeBuilder
    {
        /** @var TreeBuilder<'array'> $treeBuilder */
        $treeBuilder = new TreeBuilder($this->alias);
        $file = (new \ReflectionObject($this->subject))->getFileName();
        $loader = new DefinitionFileLoader($treeBuilder, new FileLocator(\dirname($file)), $this->container);
        $configurator = new DefinitionConfigurator($treeBuilder, $loader, $file, $file);

        $this->subject->configure($configurator);

        return $treeBuilder;
    }
}


Review comment (suggestion):
would this make it easier for SA?
```suggestion
        $treeBuilder = new TreeBuilder($this->alias, 'array');
```

Code after:
use Symfony\Component\Config\Definition\Configurator\DefinitionConfigurator;
use Symfony\Component\Config\Definition\Loader\DefinitionFileLoader;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
 * @author Yonel Ceruto <yonelceruto@gmail.com>
 *
 * @final
 */
class Configuration implements ConfigurationInterface
{
    public function __construct(
        private ConfigurableInterface $subject,
        private ?ContainerBuilder $container,
        private string $alias,
    ) {
    }

    /**
     * @return TreeBuilder<'array'>
     */
    public function getConfigTreeBuilder(): TreeBuilder
    {
        $treeBuilder = new TreeBuilder($this->alias, 'array');
        $file = (new \ReflectionObject($this->subject))->getFileName();
        $loader = new DefinitionFileLoader($treeBuilder, new FileLocator(\dirname($file)), $this->container);
        $configurator = new DefinitionConfigurator($treeBuilder, $loader, $file, $file);

        $this->subject->configure($configurator);

        return $treeBuilder;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/JsonStreamer/Tests/Fixtures/Model/SelfReferencingDummyList.php (PHP)

Code before:
<?php

namespace Fixtures\Model;

namespace Symfony\Component\JsonStreamer\Tests\Fixtures\Model;

class SelfReferencingDummyList
{
    /**
     * @var self[]
     */
    public array $items = [];
}


Review comment (suggestion):
```suggestion
namespace Symfony\Component\JsonStreamer\Tests\Fixtures\Model;
```

Code after:
<?php

namespace Symfony\Component\JsonStreamer\Tests\Fixtures\Model;

class SelfReferencingDummyList
{
    /**
     * @var self[]
     */
    public array $items = [];
}
Repository: symfony/symfony
File: src/Symfony/Component/DependencyInjection/Attribute/AsTagDecorator.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\DependencyInjection\Attribute;

use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * Declares a service as a decorator template for all services with a specific tag.
 */
#[\Attribute(\Attribute::TARGET_CLASS)]
class AsTagDecorator
{
    /**
     * @param string $decoratesTag The tag name to decorate
     * @param int    $priority     The priority of this decoration when multiple decorators are declared for the same tag
     * @param int    $onInvalid    The behavior to adopt when no services have the tag; must be one of the {@see ContainerInterface} constants
     */
    public function __construct(
        public string $decoratesTag,
        public int $priority = 0,
        public int $onInvalid = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE,
    ) {
    }
}


Review comment (suggestion):
```suggestion
 * Declares a class as a decorator of all services with a specific tag.
```

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\DependencyInjection\Attribute;

use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * Declares a class as a decorator of all services with a specific tag.
 *
 * @author Mathias Arlaud <mathias.arlaud@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_CLASS)]
class AsTagDecorator
{
    /**
     * @param string                          $tag       The tag name to decorate
     * @param int                             $priority  The priority of this decoration when multiple decorators are declared for the same tag
     * @param ContainerInterface::*_REFERENCE $onInvalid The behavior to adopt when no services have the tag; must be one of the {@see ContainerInterface} constants
     */
    public function __construct(
        public string $tag,
        public int $priority = 0,
        public int $onInvalid = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE,
    ) {
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/DependencyInjection/Attribute/AsTagDecorator.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\DependencyInjection\Attribute;

use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * Declares a service as a decorator template for all services with a specific tag.
 */
#[\Attribute(\Attribute::TARGET_CLASS)]
class AsTagDecorator
{
    /**
     * @param string $decoratesTag The tag name to decorate
     * @param int    $priority     The priority of this decoration when multiple decorators are declared for the same tag
     * @param int    $onInvalid    The behavior to adopt when no services have the tag; must be one of the {@see ContainerInterface} constants
     */
    public function __construct(
        public string $decoratesTag,
        public int $priority = 0,
        public int $onInvalid = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE,
    ) {
    }
}


Review comment (suggestion):
simpler?
```suggestion
     * @param string $tag The tag name to decorate
```

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\DependencyInjection\Attribute;

use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * Declares a class as a decorator of all services with a specific tag.
 *
 * @author Mathias Arlaud <mathias.arlaud@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_CLASS)]
class AsTagDecorator
{
    /**
     * @param string                          $tag       The tag name to decorate
     * @param int                             $priority  The priority of this decoration when multiple decorators are declared for the same tag
     * @param ContainerInterface::*_REFERENCE $onInvalid The behavior to adopt when no services have the tag; must be one of the {@see ContainerInterface} constants
     */
    public function __construct(
        public string $tag,
        public int $priority = 0,
        public int $onInvalid = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE,
    ) {
    }
}