ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Transform/MapCollection.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\Transform;

use Symfony\Component\ObjectMapper\Exception\MappingException;
use Symfony\Component\ObjectMapper\ObjectMapper;
use Symfony\Component\ObjectMapper\ObjectMapperInterface;
use Symfony\Component\ObjectMapper\TransformCallableInterface;

/**
 * @template T of object
 *
 * @implements TransformCallableInterface<object, T>
 */
class MapCollection implements TransformCallableInterface
{
    public function __construct(private ObjectMapperInterface $objectMapper = new ObjectMapper())
    {
    }

    public function __invoke(mixed $value, object $source, ?object $target): mixed
    {
        if (!is_iterable($value)) {
            throw new MappingException(\sprintf('The MapCollection transform expects an iterable, "%s" given.', get_debug_type($value)));
        }

        $values = [];
        foreach ($value as $k => $v) {
            $values[$k] = $this->objectMapper->map($v);
        }

        return $values;
    }
}


Review comment (suggestion):
```suggestion
    public function __construct(
        private ObjectMapperInterface $objectMapper = new ObjectMapper(),
    ) {
```

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\Transform;

use Symfony\Component\ObjectMapper\Exception\MappingException;
use Symfony\Component\ObjectMapper\ObjectMapper;
use Symfony\Component\ObjectMapper\ObjectMapperInterface;
use Symfony\Component\ObjectMapper\TransformCallableInterface;

/**
 * @template T of object
 *
 * @implements TransformCallableInterface<object, T>
 */
class MapCollection implements TransformCallableInterface
{
    public function __construct(
        private ObjectMapperInterface $objectMapper = new ObjectMapper(),
    ) {
    }

    public function __invoke(mixed $value, object $source, ?object $target): mixed
    {
        if (!is_iterable($value)) {
            throw new MappingException(\sprintf('The MapCollection transform expects an iterable, "%s" given.', get_debug_type($value)));
        }

        $values = [];
        foreach ($value as $k => $v) {
            $values[$k] = $this->objectMapper->map($v);
        }

        return $values;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Mailer/EventListener/MessageLoggerListener.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\Mailer\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Mailer\Event\MessageEvent;
use Symfony\Component\Mailer\Event\MessageEvents;
use Symfony\Contracts\Service\ResetInterface;

/**
 * Logs Messages.
 *
 * @author Fabien Potencier <fabien@symfony.com>
 */
class MessageLoggerListener implements EventSubscriberInterface, ResetInterface
{
    private MessageEvents $events;

    public function __construct(protected ?\Closure $disabled = null)
    {
        $this->events = new MessageEvents();
    }

    public function reset(): void
    {
        $this->events = new MessageEvents();
    }

    public function onMessage(MessageEvent $event): void
    {
        if ($this->disabled?->__invoke()) {
            return;
        }

        $this->events->add($event);
    }

    public function getEvents(): MessageEvents
    {
        return $this->events;
    }

    public static function getSubscribedEvents(): array
    {

Review comment (suggestion):
```suggestion
    public function __construct(
        private ?\Closure $disabled = null,
    ) {
```

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\Mailer\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Mailer\Event\MessageEvent;
use Symfony\Component\Mailer\Event\MessageEvents;
use Symfony\Contracts\Service\ResetInterface;

/**
 * Logs Messages.
 *
 * @author Fabien Potencier <fabien@symfony.com>
 */
class MessageLoggerListener implements EventSubscriberInterface, ResetInterface
{
    private MessageEvents $events;

    public function __construct(
        protected ?\Closure $disabled = null,
    ) {
        $this->events = new MessageEvents();
    }

    public function reset(): void
    {
        $this->events = new MessageEvents();
    }

    public function onMessage(MessageEvent $event): void
    {
        if ($this->disabled?->__invoke()) {
            return;
        }

        $this->events->add($event);
    }

    public function getEvents(): MessageEvents
    {
        return $this->events;
    }

    public static function getSubscribedEvents(): array
Repository: symfony/symfony
File: src/Symfony/Component/DependencyInjection/Tests/Compiler/ServiceLocatorTagPassTest.php (PHP)

Code before:
        ]);
        $locator = $container->getDefinition($locator);
        $factories = $locator->getArguments()[0];

        static::assertSame(['service-2', 'service-1'], array_keys($factories));
    }

    public function testBindingsAreProcessed()
    {
        $container = new ContainerBuilder();

        $definition = $container->register('foo')
            ->setBindings(['foo' => new ServiceLocatorArgument()]);

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

        $this->assertInstanceOf(Reference::class, $definition->getBindings()['foo']->getValues()[0]);
    }
}

class Locator
{
    public ServiceLocator $locator;

    public function __construct(ServiceLocator $locator)
    {
        $this->locator = $locator;
    }
}

class Service
{
}

class DecoratedService
{
}


Review comment (suggestion):
I would add a third one that uses a string key with a reference to a different id, to cover more cases

Code after:

        $decorated = new Definition(DecoratedService::class);
        $decorated->setPublic(true);
        $decorated->setDecoratedService(Service::class);

        $container->setDefinition(DecoratedService::class, $decorated);

        $container->compile();

        /** @var ServiceLocator $locator */
        $locator = $container->get(Locator::class)->locator;
        static::assertTrue($locator->has(Service::class));
        static::assertFalse($locator->has(DecoratedService::class));
        static::assertInstanceOf(DecoratedService::class, $locator->get(Service::class));
    }

    public function testServicesKeysAreKept()
    {
        $container = new ContainerBuilder();
        $container->register('service-1');
        $container->register('service-2');
        $container->register('service-3');

        $locator = ServiceLocatorTagPass::register($container, [
            new Reference('service-1'),
            'service-2' => new Reference('service-2'),
            'foo' => new Reference('service-3'),
        ]);
        $locator = $container->getDefinition($locator);
        $factories = $locator->getArguments()[0];

        static::assertSame([0, 'service-2', 'foo'], array_keys($factories));
    }

    public function testDefinitionOrderIsTheSame()
    {
        $container = new ContainerBuilder();
        $container->register('service-1');
        $container->register('service-2');

        $locator = ServiceLocatorTagPass::register($container, [
            'service-2' => new Reference('service-2'),
            'service-1' => new Reference('service-1'),
        ]);
        $locator = $container->getDefinition($locator);
        $factories = $locator->getArguments()[0];

        static::assertSame(['service-2', 'service-1'], array_keys($factories));
    }

    public function testBindingsAreProcessed()
Repository: symfony/symfony
File: src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Messenger/DummyCommand.php (PHP)

Code before:
<?php

namespace Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger;

use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Scheduler\Attribute\AsCronTask;

#[AsCronTask(expression: '* * * * *', schedule: 'dummy_command')]
#[AsCronTask(expression: '0 * * * *', arguments: 'test', schedule: 'dummy_command')]
#[AsCommand(name: 'test:dummy-command')]
class DummyCommand extends Command
{
    public static array $calls = [];

    public function configure()
    {
        $this->addArgument('dummy-argument', InputArgument::OPTIONAL);
    }

    public function execute(InputInterface $input, ?OutputInterface $output = null): int
    {
        self::$calls[__FUNCTION__][] = $input->getArgument('dummy-argument');

        return Command::SUCCESS;
    }
}


Review comment (suggestion):
```suggestion
    public function configure(): void
```

This should fix the CI

Code after:
<?php

namespace Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger;

use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Scheduler\Attribute\AsCronTask;

#[AsCronTask(expression: '* * * * *', schedule: 'dummy_command')]
#[AsCronTask(expression: '0 * * * *', arguments: 'test', schedule: 'dummy_command')]
#[AsCommand(name: 'test:dummy-command')]
class DummyCommand extends Command
{
    public static array $calls = [];

    public function configure(): void
    {
        $this->addArgument('dummy-argument', InputArgument::OPTIONAL);
    }

    public function execute(InputInterface $input, ?OutputInterface $output = null): int
    {
        self::$calls[__FUNCTION__][] = $input->getArgument('dummy-argument');

        return Command::SUCCESS;
    }
}
Repository: symfony/symfony
File: .twig-cs-fixer.dist.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.
 */

$ruleset = new TwigCsFixer\Ruleset\Ruleset();
$ruleset->addStandard(new TwigCsFixer\Standard\TwigCsFixer());
$ruleset->addRule(new TwigCsFixer\Rules\Function\IncludeFunctionRule());

$finder = new TwigCsFixer\File\Finder();
$finder->in('src/Symfony/Bridge/')->exclude('Twig/Tests/Fixtures/extractor/');
$finder->in('src/Symfony/Bundle/');
$finder->in('src/Symfony/Component/');

$config = new TwigCsFixer\Config\Config();
$config->setCacheFile('.twig-cs-fixer.cache');
$config->setRuleset($ruleset);
$config->setFinder($finder);
// $config->allowNonFixableRules();

return $config;


Review comment (bug):
> [!NOTE]
> for file exclude, see https://github.com/symfony/symfony/blob/7.4/src/Symfony/Bridge/Twig/Tests/Fixtures/extractor/syntax_error.twig
the tool crash and reports it as 

```bash
 KO src/Symfony/Bridge/Twig/Tests/Fixtures/extractor/syntax_error.twig
 ------- ---------------------------------------------------------- 
  FATAL   >>   | Unable to tokenize file: The template is invalid.  
 ------- ---------------------------------------------------------- 
```

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.
 */

$ruleset = new TwigCsFixer\Ruleset\Ruleset();
$ruleset->addStandard(new TwigCsFixer\Standard\TwigCsFixer());

$finder = new TwigCsFixer\File\Finder();
$finder->in('src/Symfony/Bridge/');
$finder->in('src/Symfony/Bundle/');
$finder->in('src/Symfony/Component/');
$finder->exclude('Fixtures');

$config = new TwigCsFixer\Config\Config();
$config->setCacheFile('.twig-cs-fixer.cache');
$config->setRuleset($ruleset);
$config->setFinder($finder);

return $config;
Repository: symfony/symfony
File: .twig-cs-fixer.dist.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.
 */

$ruleset = new TwigCsFixer\Ruleset\Ruleset();
$ruleset->addStandard(new TwigCsFixer\Standard\TwigCsFixer());
$ruleset->addRule(new TwigCsFixer\Rules\Function\IncludeFunctionRule());

$finder = new TwigCsFixer\File\Finder();
$finder->in('src/Symfony/Bridge/')->exclude('Twig/Tests/Fixtures/extractor/');
$finder->in('src/Symfony/Bundle/');
$finder->in('src/Symfony/Component/');

$config = new TwigCsFixer\Config\Config();
$config->setCacheFile('.twig-cs-fixer.cache');
$config->setRuleset($ruleset);
$config->setFinder($finder);
// $config->allowNonFixableRules();

return $config;


Review comment (suggestion):
Since it's the Symfony code base, I would not recommend using 
```
new TwigCsFixer\Standard\TwigCsFixer()
```
which is opinionated.

And instead use
```
new TwigCsFixer\Standard\Symfony()
```
which is based on best practices from Symfony/Twig docs

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.
 */

$ruleset = new TwigCsFixer\Ruleset\Ruleset();
$ruleset->addStandard(new TwigCsFixer\Standard\TwigCsFixer());

$finder = new TwigCsFixer\File\Finder();
$finder->in('src/Symfony/Bridge/');
$finder->in('src/Symfony/Bundle/');
$finder->in('src/Symfony/Component/');
$finder->exclude('Fixtures');

$config = new TwigCsFixer\Config\Config();
$config->setCacheFile('.twig-cs-fixer.cache');
$config->setRuleset($ruleset);
$config->setFinder($finder);

return $config;
Repository: symfony/symfony
File: .twig-cs-fixer.dist.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.
 */

$ruleset = new TwigCsFixer\Ruleset\Ruleset();
$ruleset->addStandard(new TwigCsFixer\Standard\TwigCsFixer());

$finder = new TwigCsFixer\File\Finder();
$finder->in('src/Symfony/Bridge/');
$finder->in('src/Symfony/Bundle/');
$finder->in('src/Symfony/Component/');
$finder->exclude('Fixtures');

$config = new TwigCsFixer\Config\Config();
$config->setCacheFile('.twig-cs-fixer.cache');
$config->setRuleset($ruleset);
$config->setFinder($finder);
// RFC: should we allow?
// $config->allowNonFixableRules();

return $config;


Review comment (suggestion):
no at least for now, might be reconsidered later

```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.
 */

$ruleset = new TwigCsFixer\Ruleset\Ruleset();
$ruleset->addStandard(new TwigCsFixer\Standard\TwigCsFixer());

$finder = new TwigCsFixer\File\Finder();
$finder->in('src/Symfony/Bridge/');
$finder->in('src/Symfony/Bundle/');
$finder->in('src/Symfony/Component/');
$finder->exclude('Fixtures');

$config = new TwigCsFixer\Config\Config();
$config->setCacheFile('.twig-cs-fixer.cache');
$config->setRuleset($ruleset);
$config->setFinder($finder);

return $config;
Repository: symfony/symfony
File: src/Symfony/Component/JsonPath/Tests/JsonCrawlerTest.php (PHP)

Code before:

    public static function provideSingleQuotedStringProvider(): array
    {
        return [
            [
                "$['caf\\u00e9']",
                ['coffee'],
            ],
            [
                "$['\\u65e5\\u672c']",
                ['Japan'],
            ],
            [
                "$['M\\u00fcller']",
                [],
            ],
            [
                "$['emoji\\ud83d\\ude00']",
                ['smiley'],
            ],
            [
                "$['tab\\there']",
                ['with tab'],
            ],
            [
                '$["quote\\"here"]',
                ['with quote'],
            ],
            [
                "$['backslash\\\\here']",
                ['with backslash'],
            ],
            [
                "$['apostrophe\\'here']",
                ['with apostrophe'],
            ],
            [
                "$['control\\u0001char']",
                ['with control char'],
            ],
            [
                "$['\\u0063af\\u00e9']",
                ['coffee'],
            ],
        ];
    }

    /**
     * @dataProvider provideFilterWithUnicodeProvider
     */
    public function testFilterWithUnicodeStrings(string $jsonPath, int $expectedCount, string $expectedCountry)

Review comment (suggestion):
this data provider is named `provideSingleQuotedStringProvider`, so it should not be changed to use double-quoted strings IMO

Code after:

    public static function provideSingleQuotedStringProvider(): array
    {
        return [
            [
                "$['caf\\u00e9']",
                ['coffee'],
            ],
            [
                "$['\\u65e5\\u672c']",
                ['Japan'],
            ],
            [
                "$['M\\u00fcller']",
                [],
            ],
            [
                "$['emoji\\ud83d\\ude00']",
                ['smiley'],
            ],
            [
                "$['tab\\there']",
                ['with tab'],
            ],
            [
                "$['quote\"here']",
                ['with quote'],
            ],
            [
                "$['backslash\\\\here']",
                ['with backslash'],
            ],
            [
                "$['apostrophe\\'here']",
                ['with apostrophe'],
            ],
            [
                "$['control\\u0001char']",
                ['with control char'],
            ],
            [
                "$['\\u0063af\\u00e9']",
                ['coffee'],
            ],
        ];
    }

    /**
     * @dataProvider provideFilterWithUnicodeProvider
     */
    public function testFilterWithUnicodeStrings(string $jsonPath, int $expectedCount, string $expectedCountry)
Repository: symfony/symfony
File: src/Symfony/Component/Console/Tests/Tester/CommandTesterTest.php (PHP)

Code before:
            ['foo' => 'bar'],
            ['capture_stderr_separately' => true]
        );

        $this->assertSame('foo', $tester->getErrorOutput());
    }

    public function testAInvokableExtendedCommand()
    {
        $command = new SomeInvokableExtendedTestCommand();

        $tester = new CommandTester($command);
        $tester->execute([]);

        $tester->assertCommandIsSuccessful();
    }
}

#[AsCommand(name: 'invokable-extended')]
class SomeInvokableExtendedTestCommand extends Command
{
    public function __invoke(): int
    {
        return 0;
    }
}


Review comment (suggestion):
This case was already working, as this class extends `Command`.

The new feature test would use `InvokableTestCommand`:
```suggestion

    public function testInvokableCommand()
    {
        $command = new InvokableTestCommand();

        $tester = new CommandTester($command);
        $tester->execute([]);

        $tester->assertCommandIsSuccessful();
    }
}
```

Code after:
            ['foo' => 'bar'],
            ['capture_stderr_separately' => true]
        );

        $this->assertSame('foo', $tester->getErrorOutput());
    }

    public function testAInvokableCommand()
    {
        $command = new InvokableTestCommand();

        $tester = new CommandTester($command);
        $tester->execute([]);

        $tester->assertCommandIsSuccessful();
    }

    public function testAInvokableExtendedCommand()
    {
        $command = new InvokableExtendingCommandTestCommand();

        $tester = new CommandTester($command);
        $tester->execute([]);

        $tester->assertCommandIsSuccessful();
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Tests/Fixtures/DefaultValueStdClass/TargetDto.php (PHP)

Code before:
<?php

namespace Symfony\Component\ObjectMapper\Tests\Fixtures\DefaultValueStdClass;

use Symfony\Component\ObjectMapper\Attribute\Map;

class TargetDto
{
    public function __construct(
        public string  $id,
        #[Map(source: 'optional', if: [self::class, 'isDefined'])]
        public ?string $optional = null,
    )
    {
    }

    public static function isDefined($source): bool {
        return isset($source);
    }
}


Review comment (suggestion):
```suggestion
    public static function isDefined($source): bool
    {
```

Code after:
<?php

namespace Symfony\Component\ObjectMapper\Tests\Fixtures\DefaultValueStdClass;

use Symfony\Component\ObjectMapper\Attribute\Map;

class TargetDto
{
    public function __construct(
        public string  $id,
        #[Map(source: 'optional', if: [self::class, 'isDefined'])]
        public ?string $optional = null,
    ) {
    }

    public static function isDefined($source): bool
    {
        return isset($source);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Tests/ObjectMapperTest.php (PHP)

Code before:
        $this->assertEquals('test', $c->bar);
        $this->assertEquals('donotmap', $c->foo);
        $this->assertEquals('foo', $c->doesNotExistInTargetB);
    }

    public function testDefaultValueStdClass()
    {
        $this->expectException(NoSuchPropertyException::class);
        $u = new \stdClass();
        $u->id = 'abc';
        $mapper = new ObjectMapper();
        $b = $mapper->map($u, TargetDto::class);
        $this->assertInstanceOf(TargetDto::class, $b);
        $this->assertEquals('abc', $b->id);
        $this->assertEquals(null, $b->optional);
    }

    public function testDefaultValueStdClassWithPropertyInfo()
    {
        $u = new \stdClass();
        $u->id = 'abc';
        $mapper = new ObjectMapper(propertyAccessor: PropertyAccess::createPropertyAccessorBuilder()->disableExceptionOnInvalidPropertyPath()->getPropertyAccessor());
        $b = $mapper->map($u, TargetDto::class);
        $this->assertInstanceOf(TargetDto::class, $b);
        $this->assertEquals('abc', $b->id);
        $this->assertEquals(null, $b->optional);
    }
}


Review comment (suggestion):
```suggestion
        $this->assertSame('abc', $b->id);
        $this->assertNull($b->optional);
```

Code after:
        $this->assertInstanceOf(MultipleTargetPropertyC::class, $c);
        $this->assertEquals('test', $c->bar);
        $this->assertEquals('donotmap', $c->foo);
        $this->assertEquals('foo', $c->doesNotExistInTargetB);
    }

    public function testDefaultValueStdClass()
    {
        $this->expectException(NoSuchPropertyException::class);
        $u = new \stdClass();
        $u->id = 'abc';
        $mapper = new ObjectMapper();
        $b = $mapper->map($u, TargetDto::class);
    }

    public function testDefaultValueStdClassWithPropertyInfo()
    {
        $u = new \stdClass();
        $u->id = 'abc';
        $mapper = new ObjectMapper(propertyAccessor: PropertyAccess::createPropertyAccessorBuilder()->disableExceptionOnInvalidPropertyPath()->getPropertyAccessor());
        $b = $mapper->map($u, TargetDto::class);
        $this->assertInstanceOf(TargetDto::class, $b);
        $this->assertSame('abc', $b->id);
        $this->assertNull($b->optional);
    }
}