ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php (PHP)

Code before:
/**
 * @author Wouter de Jong <wouter@wouterj.nl>
 *
 * @internal
 */
class CustomAuthenticatorFactory implements AuthenticatorFactoryInterface
{
    public function getPriority(): int
    {
        return 0;
    }

    public function getKey(): string
    {
        return 'custom_authenticators';
    }

    /**
     * @param ArrayNodeDefinition $builder
     */
    public function addConfiguration(NodeDefinition $builder): void
    {
        // get the parent array node builder ("firewalls") from inside the children builder
        $factoryRootNode = $builder->end()->end();
        $factoryRootNode
            ->fixXmlConfig('custom_authenticator')
            ->validate()
                ->ifTrue(fn ($v) => isset($v['custom_authenticators']) && !$v['custom_authenticators'])
                ->then(function ($v) {
                    unset($v['custom_authenticators']);

                    return $v;
                })
            ->end()
        ;

        $builder
            ->info('An array of service ids for all of your "authenticators".')
            ->requiresAtLeastOneElement()
            ->prototype('scalar')->end();

    }

    public function createAuthenticator(ContainerBuilder $container, string $firewallName, array $config, string $userProviderId): array
    {
        return $config;
    }
}


Review comment (security):
Deprecating `fixXmlConfig` would first require changing how custom authenticators are configured, by avoiding the hack of getting out of the builder in this factory

Code after:
/**
 * @author Wouter de Jong <wouter@wouterj.nl>
 *
 * @internal
 */
class CustomAuthenticatorFactory implements AuthenticatorFactoryInterface
{
    public function getPriority(): int
    {
        return 0;
    }

    public function getKey(): string
    {
        return 'custom_authenticators';
    }

    /**
     * @param ArrayNodeDefinition $builder
     */
    public function addConfiguration(NodeDefinition $builder): void
    {
        // get the parent array node builder ("firewalls") from inside the children builder
        $factoryRootNode = $builder->end()->end();
        $factoryRootNode
            ->fixXmlConfig('custom_authenticator')
            ->validate()
                ->ifTrue(fn ($v) => isset($v['custom_authenticators']) && !$v['custom_authenticators'])
                ->then(function ($v) {
                    unset($v['custom_authenticators']);

                    return $v;
                })
            ->end()
        ;

        $builder
            ->info('An array of service ids for all of your "authenticators".')
            ->requiresAtLeastOneElement()
            ->prototype('scalar')->end();
    }

    public function createAuthenticator(ContainerBuilder $container, string $firewallName, array $config, string $userProviderId): array
    {
        return $config;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php (PHP)

Code before:
     */
    protected MergeBuilder $merge;
    protected bool $allowEmptyValue = true;
    protected mixed $nullEquivalent = null;
    protected mixed $trueEquivalent = true;
    protected mixed $falseEquivalent = false;
    protected string $pathSeparator = BaseNode::DEFAULT_PATH_SEPARATOR;
    /**
     * @var TParent|NodeInterface
     */
    protected NodeParentInterface|NodeInterface|null $parent;
    protected array $attributes = [];

    /**
     * @param TParent $parent
     */
    public function __construct(?string $name, ?NodeParentInterface $parent = null)
    {
        $this->parent = $parent;
        $this->name = $name;
    }

    /**
     * Sets the parent node.
     *
     * @param TParent&NodeParentInterface $parent
     *
     * @return $this
     */
    public function setParent(NodeParentInterface $parent): static
    {
        $this->parent = $parent;

        return $this;
    }

    /**
     * Sets info message.
     *
     * @return $this
     */
    public function info(string $info): static
    {
        return $this->attribute('info', $info);
    }

    /**
     * Sets example configuration.
     *
     * @return $this
     */

Review comment (suggestion):
The proper way to describe this is actually to document that calling this method **changes** the parent type of the instance, instead of enforcing the argument to be the exact same parent type (which would be hard to achieve as the actual type involve the whole chain of parents):

```
@template TNewParent of NodeParentInterface
@psalm-this-out static<TNewParent>
```

Note that I'm not sure how psalm supports `static<U>` in the `psalm-this-out` annotation (rather than things that don't involve `static`). I know that phpstan added support for it last year (I know phpstan a lot better than psalm as I'm using phpstan myself in all my projects)

Code after:
     */
    protected MergeBuilder $merge;
    protected bool $allowEmptyValue = true;
    protected mixed $nullEquivalent = null;
    protected mixed $trueEquivalent = true;
    protected mixed $falseEquivalent = false;
    protected string $pathSeparator = BaseNode::DEFAULT_PATH_SEPARATOR;
    /**
     * @var TParent|NodeInterface
     */
    protected NodeParentInterface|NodeInterface|null $parent;
    protected array $attributes = [];

    /**
     * @param TParent $parent
     */
    public function __construct(?string $name, ?NodeParentInterface $parent = null)
    {
        $this->parent = $parent;
        $this->name = $name;
    }

    /**
     * Sets the parent node.
     *
     * @template TNewParent of NodeParentInterface
     *
     * @psalm-this-out static<TNewParent>
     *
     * @return $this
     */
    public function setParent(NodeParentInterface $parent): static
    {
        $this->parent = $parent;

        return $this;
    }

    /**
     * Sets info message.
     *
     * @return $this
     */
    public function info(string $info): static
    {
        return $this->attribute('info', $info);
    }

    /**
     * Sets example configuration.
     *
Repository: symfony/symfony
File: src/Symfony/Component/Notifier/Bridge/Lox24/Tests/Webhook/Lox24RequestParserTest.php (PHP)

Code before:
                'id' => '123',
                'dlr_code' => 16, // DELIVERY_REJECTED
            ],
        ];

        $request = $this->getRequest($payload);
        $event = $this->parser->parse($request, '');

        $this->assertInstanceOf(SmsEvent::class, $event);
        $this->assertSame('123', $event->getId());
        $this->assertSame(SmsEvent::FAILED, $event->getName());
    }

    public function testSmsDeliveryWithDlrCodePending()
    {
        $request = $this->getRequest([
            'id' => 'webhook-id',
            'name' => 'sms.delivery',
            'data' => [
                'id' => '123',
                'dlr_code' => 2, // DELIVERY_QUEUE (pending)
            ],
        ]);

        $event = $this->parser->parse($request, '');
        $this->assertNull($event); // Should return null for pending states
    }

    public function testSmsDeliveryDryrun()
    {
        $payload = [
            'id' => 'webhook-id',
            'name' => 'sms.delivery.dryrun',
            'data' => [
                'id' => '123',
                'status_code' => 100,
            ],
        ];

        $request = $this->getRequest($payload);
        $event = $this->parser->parse($request, '');

        $this->assertInstanceOf(SmsEvent::class, $event);
        $this->assertSame('123', $event->getId());
        $this->assertSame(SmsEvent::DELIVERED, $event->getName());
    }

    // SMS Incoming Event Tests
    public function testSmsIncomingSuccess()
    {
        $payload = [

Review comment (suggestion):
```suggestion
        $this->assertNull($event);
```

Code after:
        $this->assertSame(SmsEvent::FAILED, $event->getName());
    }

    public function testSmsDeliveryWithDlrCodePending()
    {
        $request = $this->getRequest([
            'id' => 'webhook-id',
            'name' => 'sms.delivery',
            'data' => [
                'id' => '123',
                'dlr_code' => 2,
            ],
        ]);

        $event = $this->parser->parse($request, '');
        $this->assertNull($event);
    }

    public function testSmsDeliveryDryrun()
    {
        $payload = [
            'id' => 'webhook-id',
            'name' => 'sms.delivery.dryrun',
            'data' => [
                'id' => '123',
                'status_code' => 100,
            ],
        ];

        $request = $this->getRequest($payload);
        $event = $this->parser->parse($request, '');

        $this->assertInstanceOf(SmsEvent::class, $event);
        $this->assertSame('123', $event->getId());
        $this->assertSame(SmsEvent::DELIVERED, $event->getName());
    }

    public function testMissingIdField()
    {
        $this->expectException(RejectWebhookException::class);
        $this->expectExceptionMessage('The required fields "id" are missing from the payload.');

        $request = $this->getRequest([
            'name' => 'sms.delivery',
            'data' => ['id' => '123'],
        ]);
        $this->parser->parse($request, '');
    }

    public function testMissingNameField()
    {
Repository: symfony/symfony
File: src/Symfony/Component/Uid/Factory/MockUuidFactory.php (PHP)

Code before:
            }
        };
    }

    public function nameBased(Uuid|string|null $namespace = null): NameBasedUuidFactory
    {
        if (null === $namespace ??= $this->nameBasedNamespace) {
            throw new LogicException(\sprintf('A namespace should be defined when using "%s()".', __METHOD__));
        }

        return new class($this->create(...), $namespace) extends NameBasedUuidFactory {
            public function __construct(
                private \Closure $create,
                private Uuid|string $namespace,
            ) {
            }

            public function create(string $name): UuidV5|UuidV3
            {
                if (!($uuid = ($this->create)()) instanceof UuidV5 && !$uuid instanceof UuidV3) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence is not a UuidV5 or UuidV3: "%s".', get_debug_type($uuid)));
                }

                $factory = new UuidFactory(nameBasedClass: $uuid::class, nameBasedNamespace: $this->namespace);

                if ($uuid->toRfc4122() !== $factory->nameBased()->create($name)->toRfc4122()) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence does not match the expected named UUID: "%s" != "%s".', $uuid->toRfc4122(), $factory->nameBased($this->namespace)->create($name)->toRfc4122()));
                }

                return $uuid;
            }
        };
    }
}


Review comment (suggestion):
```suggestion
                if ($uuid->toRfc4122() !== $expectedUuid = $factory->nameBased()->create($name)->toRfc4122()) {
```

Code after:

    public function nameBased(Uuid|string|null $namespace = null): NameBasedUuidFactory
    {
        if (null === $namespace ??= $this->nameBasedNamespace) {
            throw new LogicException(\sprintf('A namespace should be defined when using "%s()".', __METHOD__));
        }

        return new class($this->create(...), $namespace) extends NameBasedUuidFactory {
            public function __construct(
                private \Closure $create,
                private Uuid|string $namespace,
            ) {
            }

            public function create(string $name): UuidV5|UuidV3
            {
                if (!($uuid = ($this->create)()) instanceof UuidV5 && !$uuid instanceof UuidV3) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence is not a UuidV5 or UuidV3: "%s".', get_debug_type($uuid)));
                }

                $factory = new UuidFactory(nameBasedClass: $uuid::class, nameBasedNamespace: $this->namespace);

                if ($uuid->toRfc4122() !== $expectedUuid = $factory->nameBased()->create($name)->toRfc4122()) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence does not match the expected named UUID: "%s" != "%s".', $uuid->toRfc4122(), $expectedUuid));
                }

                return $uuid;
            }
        };
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Uid/Factory/MockUuidFactory.php (PHP)

Code before:
        };
    }

    public function nameBased(Uuid|string|null $namespace = null): NameBasedUuidFactory
    {
        if (null === $namespace ??= $this->nameBasedNamespace) {
            throw new LogicException(\sprintf('A namespace should be defined when using "%s()".', __METHOD__));
        }

        return new class($this->create(...), $namespace) extends NameBasedUuidFactory {
            public function __construct(
                private \Closure $create,
                private Uuid|string $namespace,
            ) {
            }

            public function create(string $name): UuidV5|UuidV3
            {
                if (!($uuid = ($this->create)()) instanceof UuidV5 && !$uuid instanceof UuidV3) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence is not a UuidV5 or UuidV3: "%s".', get_debug_type($uuid)));
                }

                $factory = new UuidFactory(nameBasedClass: $uuid::class, nameBasedNamespace: $this->namespace);

                if ($uuid->toRfc4122() !== $factory->nameBased()->create($name)->toRfc4122()) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence does not match the expected named UUID: "%s" != "%s".', $uuid->toRfc4122(), $factory->nameBased($this->namespace)->create($name)->toRfc4122()));
                }

                return $uuid;
            }
        };
    }
}


Review comment (suggestion):
```suggestion
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence does not match the expected named UUID: "%s" != "%s".', $uuid->toRfc4122(), $expectedUuid));
```

Code after:
    public function nameBased(Uuid|string|null $namespace = null): NameBasedUuidFactory
    {
        if (null === $namespace ??= $this->nameBasedNamespace) {
            throw new LogicException(\sprintf('A namespace should be defined when using "%s()".', __METHOD__));
        }

        return new class($this->create(...), $namespace) extends NameBasedUuidFactory {
            public function __construct(
                private \Closure $create,
                private Uuid|string $namespace,
            ) {
            }

            public function create(string $name): UuidV5|UuidV3
            {
                if (!($uuid = ($this->create)()) instanceof UuidV5 && !$uuid instanceof UuidV3) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence is not a UuidV5 or UuidV3: "%s".', get_debug_type($uuid)));
                }

                $factory = new UuidFactory(nameBasedClass: $uuid::class, nameBasedNamespace: $this->namespace);

                if ($uuid->toRfc4122() !== $expectedUuid = $factory->nameBased()->create($name)->toRfc4122()) {
                    throw new InvalidArgumentException(\sprintf('Next UUID in sequence does not match the expected named UUID: "%s" != "%s".', $uuid->toRfc4122(), $expectedUuid));
                }

                return $uuid;
            }
        };
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Console/Helper/QuestionHelper.php (PHP)

Code before:
                $exe = $tmpExe;
            }

            $sExec = shell_exec('"'.$exe.'"');
            $value = $trimmable ? rtrim($sExec) : $sExec;
            $output->writeln('');

            if (isset($tmpExe)) {
                unlink($tmpExe);
            }

            return $value;
        }

        $inputHelper = null;

        if (self::$stty && Terminal::hasSttyAvailable()) {
            $inputHelper = new TerminalInputHelper($inputStream);
            shell_exec('stty -echo');
        } elseif ($this->isInteractiveInput($inputStream)) {
            throw new RuntimeException('Unable to hide the response.');
        }

        if (null !== $inputHelper) {
            $inputHelper->waitForInput();
        }

        $value = fgets($inputStream, 4096);

        if (4095 === \strlen($value)) {
            $errOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output;
            $errOutput->warning('The value was possibly truncated by your shell or terminal emulator');
        }

        // Restore the terminal so it behaves normally again
        if (null !== $inputHelper) {
            $inputHelper->finish();
        }

        if (false === $value) {
            throw new MissingInputException('Aborted.');
        }
        if ($trimmable) {
            $value = trim($value);
        }
        $output->writeln('');

        return $value;
    }

    /**

Review comment (refactor):
```suggestion
        $inputHelper?->waitForInput();
```

Code after:
                $exe = $tmpExe;
            }

            $sExec = shell_exec('"'.$exe.'"');
            $value = $trimmable ? rtrim($sExec) : $sExec;
            $output->writeln('');

            if (isset($tmpExe)) {
                unlink($tmpExe);
            }

            return $value;
        }

        $inputHelper = null;

        if (self::$stty && Terminal::hasSttyAvailable()) {
            $inputHelper = new TerminalInputHelper($inputStream);
            shell_exec('stty -echo');
        } elseif ($this->isInteractiveInput($inputStream)) {
            throw new RuntimeException('Unable to hide the response.');
        }

        $inputHelper?->waitForInput();

        $value = fgets($inputStream, 4096);

        if (4095 === \strlen($value)) {
            $errOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output;
            $errOutput->warning('The value was possibly truncated by your shell or terminal emulator');
        }

        // Restore the terminal so it behaves normally again
        $inputHelper?->finish();

        if (false === $value) {
            throw new MissingInputException('Aborted.');
        }
        if ($trimmable) {
            $value = trim($value);
        }
        $output->writeln('');

        return $value;
    }

    /**
     * Validates an attempt.
     *
     * @param callable $interviewer A callable that will ask for a question and return the result
     *
Repository: symfony/symfony
File: src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonTest.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\Validator\Tests\Constraints;

use PHPUnit\Framework\Attributes\IgnoreDeprecations;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\Constraints\AbstractComparison;

class AbstractComparisonTest extends TestCase
{
    #[IgnoreDeprecations]
    public function testConstructorWithArrayOption()
    {
        $comparison = new class(['value' => 42, 'message' => 'my error']) extends AbstractComparison {};

        $this->assertSame(42, $comparison->value);
        $this->assertSame('my error', $comparison->message);
    }
}


Review comment (suggestion):
We also need to add it to the legacy group:


```suggestion
    #[IgnoreDeprecations]
    #[Group('legacy')]
```

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\Validator\Tests\Constraints;

use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\Constraints\AbstractComparison;

class AbstractComparisonTest extends TestCase
{
    #[IgnoreDeprecations]
    #[Group('legacy')]
    public function testConstructorWithArrayOption()
    {
        $comparison = new class(['value' => 42, 'message' => 'my error']) extends AbstractComparison {};

        $this->assertSame(42, $comparison->value);
        $this->assertSame('my error', $comparison->message);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonTest.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\Validator\Tests\Constraints;

use PHPUnit\Framework\Attributes\IgnoreDeprecations;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\Constraints\AbstractComparison;

class AbstractComparisonTest extends TestCase
{
    #[IgnoreDeprecations]
    public function testConstructorWithArrayOption()
    {
        $comparison = new class(['value' => 42, 'message' => 'my error']) extends AbstractComparison {};

        $this->assertSame(42, $comparison->value);
        $this->assertSame('my error', $comparison->message);
    }
}


Review comment (suggestion):
```suggestion
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
```

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\Validator\Tests\Constraints;

use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\Constraints\AbstractComparison;

class AbstractComparisonTest extends TestCase
{
    #[IgnoreDeprecations]
    #[Group('legacy')]
    public function testConstructorWithArrayOption()
    {
        $comparison = new class(['value' => 42, 'message' => 'my error']) extends AbstractComparison {};

        $this->assertSame(42, $comparison->value);
        $this->assertSame('my error', $comparison->message);
    }
}
Repository: symfony/symfony
File: src/Symfony/Bundle/FrameworkBundle/Command/WorkflowDumpCommand.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\Bundle\FrameworkBundle\Command;

use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Workflow\Command\WorkflowDumpCommand as BaseWorkflowDumpCommand;

trigger_deprecation('symfony/framework-bundle', '7.4', 'The "%s" class is deprecated, use "%s" instead.', WorkflowDumpCommand::class, BaseWorkflowDumpCommand::class);

#[AsCommand(name: 'workflow:dump', description: 'Dump a workflow')]
class WorkflowDumpCommand extends BaseWorkflowDumpCommand
{
}


Review comment (suggestion):
instead of removing the phpdoc entirely, you should add the `@deprecated` phpdoc.

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\Bundle\FrameworkBundle\Command;

use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Workflow\Command\WorkflowDumpCommand as BaseWorkflowDumpCommand;

trigger_deprecation('symfony/framework-bundle', '7.4', 'The "%s" class is deprecated, use "%s" instead.', WorkflowDumpCommand::class, BaseWorkflowDumpCommand::class);

/**
 * @deprecated since Symfony 7.4, use {@see \Symfony\Component\Workflow\Command\WorkflowDumpCommand} instead.
 */
#[AsCommand(name: 'workflow:dump', description: 'Dump a workflow')]
class WorkflowDumpCommand extends BaseWorkflowDumpCommand
{
}