ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Component/PropertyInfo/Tests/Fixtures/UnderscoreDummy.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\PropertyInfo\Tests\Fixtures;

/**
 * Fixture class for testing underscore-only properties (issue #61425).
 */
class UnderscoreDummy
{
    private float $_;
    private float $__;

    public function get_(): float
    {
        return $this->_;
    }

    public function get__(): float
    {
        return $this->__;
    }
}


Review comment (suggestion):
```suggestion
 * Fixture class for testing underscore-only properties.
```

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\PropertyInfo\Tests\Fixtures;

/**
 * Fixture class for testing underscore-only properties.
 */
class UnderscoreDummy
{
    private float $_;
    private float $__;

    public function get_(): float
    {
        return $this->_;
    }

    public function get__(): float
    {
        return $this->__;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Console/SignalRegistry/SignalRegistry.php (PHP)

Code before:
     * @var array<array<int, array<callable>>>
     */
    private array $stack = [];

    /**
     * @var array<int, callable|int|string>
     */
    private array $originalHandlers = [];

    public function __construct()
    {
        if (\function_exists('pcntl_async_signals')) {
            pcntl_async_signals(true);
        }
    }

    public function register(int $signal, callable $signalHandler): void
    {
        $previous = pcntl_signal_get_handler($signal);

        if (!isset($this->originalHandlers[$signal])) {
            $this->originalHandlers[$signal] = $previous;
        }

        if (!isset($this->signalHandlers[$signal])) {
            if (\is_callable($previous) && $previous !== [$this, 'handle']) {
                $this->signalHandlers[$signal][] = $previous;
            }
        }

        $this->signalHandlers[$signal][] = $signalHandler;

        pcntl_signal($signal, [$this, 'handle']);
    }

    public static function isSupported(): bool
    {
        return \function_exists('pcntl_signal');
    }

    /**
     * @internal
     */
    public function handle(int $signal): void
    {
        $count = \count($this->signalHandlers[$signal]);

        foreach ($this->signalHandlers[$signal] as $i => $signalHandler) {
            $hasNext = $i !== $count - 1;
            $signalHandler($signal, $hasNext);
        }

Review comment (suggestion):
```suggestion
            if (\is_callable($previous) && [$this, 'handle'] !== $previous) {
```

Code after:
     * @var array<array<int, array<callable>>>
     */
    private array $stack = [];

    /**
     * @var array<int, callable|int|string>
     */
    private array $originalHandlers = [];

    public function __construct()
    {
        if (\function_exists('pcntl_async_signals')) {
            pcntl_async_signals(true);
        }
    }

    public function register(int $signal, callable $signalHandler): void
    {
        $previous = pcntl_signal_get_handler($signal);

        if (!isset($this->originalHandlers[$signal])) {
            $this->originalHandlers[$signal] = $previous;
        }

        if (!isset($this->signalHandlers[$signal])) {
            if (\is_callable($previous) && [$this, 'handle'] !== $previous) {
                $this->signalHandlers[$signal][] = $previous;
            }
        }

        $this->signalHandlers[$signal][] = $signalHandler;

        pcntl_signal($signal, [$this, 'handle']);
    }

    public static function isSupported(): bool
    {
        return \function_exists('pcntl_signal');
    }

    /**
     * @internal
     */
    public function handle(int $signal): void
    {
        $count = \count($this->signalHandlers[$signal]);

        foreach ($this->signalHandlers[$signal] as $i => $signalHandler) {
            $hasNext = $i !== $count - 1;
            $signalHandler($signal, $hasNext);
        }
Repository: symfony/symfony
File: src/Symfony/Component/Console/SignalRegistry/SignalRegistry.php (PHP)

Code before:
     * This prepares the registry for a new set of handlers within a specific scope.
     *
     * @internal
     */
    public function pushCurrentHandlers(): void
    {
        $this->stack[] = $this->signalHandlers;
        $this->signalHandlers = [];
    }

    /**
     * Restores the previous handlers from the stack, making them active.
     *
     * This also restores the original OS-level signal handler if no
     * more handlers are registered for a signal that was just popped.
     *
     * @internal
     */
    public function popPreviousHandlers(): void
    {
        $popped = $this->signalHandlers;
        $this->signalHandlers = array_pop($this->stack) ?? [];

        // Restore OS handler if no more Symfony handlers for this signal
        foreach (array_keys($popped) as $signal) {
            if (empty($this->signalHandlers[$signal]) && isset($this->originalHandlers[$signal])) {
                pcntl_signal($signal, $this->originalHandlers[$signal]);
            }
        }
    }

    /**
     * @internal
     */
    public function scheduleAlarm(int $seconds): void
    {
        pcntl_alarm($seconds);
    }
}


Review comment (suggestion):
```suggestion
        foreach ($popped as $signal => $handlers) {
            if (!($this->signalHandlers[$signal] ?? false) && isset($this->originalHandlers[$signal])) {
```

Code after:
     * This prepares the registry for a new set of handlers within a specific scope.
     *
     * @internal
     */
    public function pushCurrentHandlers(): void
    {
        $this->stack[] = $this->signalHandlers;
        $this->signalHandlers = [];
    }

    /**
     * Restores the previous handlers from the stack, making them active.
     *
     * This also restores the original OS-level signal handler if no
     * more handlers are registered for a signal that was just popped.
     *
     * @internal
     */
    public function popPreviousHandlers(): void
    {
        $popped = $this->signalHandlers;
        $this->signalHandlers = array_pop($this->stack) ?? [];

        // Restore OS handler if no more Symfony handlers for this signal
        foreach ($popped as $signal => $handlers) {
            if (!($this->signalHandlers[$signal] ?? false) && isset($this->originalHandlers[$signal])) {
                pcntl_signal($signal, $this->originalHandlers[$signal]);
            }
        }
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php (PHP)

Code before:
            ],
        ];

        $this->assertSame($expectedOptions, $actualOptions);
    }

    public function testRemoveAlsoRemovesDeprecation()
    {
        $this->resolver->setDefined('foo');
        $this->resolver->setDeprecated('foo', 'vendor/package', '1.0');

        $this->assertTrue($this->resolver->isDeprecated('foo'));

        // Remove the option
        $this->resolver->remove('foo');

        // Assert the deprecation is also gone
        $this->assertFalse($this->resolver->isDeprecated('foo'));

        // Re-define it without deprecation (this should not fail)
        $this->resolver->setDefault('foo', 'bar');
        $this->assertFalse($this->resolver->isDeprecated('foo'));

        // Make sure no deprecation warning is triggered
        $count = 0;
        set_error_handler(function (int $type) use (&$count) {
            if (\E_USER_DEPRECATED === $type) {
                ++$count;
            }

            return false;
        });

        $this->resolver->resolve(['foo' => 'value']);
        $this->assertSame(0, $count);

        restore_error_handler();
    }
}


Review comment (suggestion):
```suggestion
        set_error_handler(static function (int $type) use (&$count) {
```

Code after:
            ],
        ];

        $this->assertSame($expectedOptions, $actualOptions);
    }

    public function testRemoveAlsoRemovesDeprecation()
    {
        $this->resolver->setDefined('foo');
        $this->resolver->setDeprecated('foo', 'vendor/package', '1.0');
        $this->assertTrue($this->resolver->isDeprecated('foo'));

        $this->resolver->remove('foo');
        $this->assertFalse($this->resolver->isDeprecated('foo'));

        $this->resolver->setDefault('foo', 'bar');
        $this->assertFalse($this->resolver->isDeprecated('foo'));

        $count = 0;
        set_error_handler(static function (int $type) use (&$count) {
            if (\E_USER_DEPRECATED === $type) {
                ++$count;
            }

            return false;
        });

        try {
            $this->resolver->resolve(['foo' => 'value']);
            $this->assertSame(0, $count);
        } finally {
            restore_error_handler();
        }
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Yaml/Tests/ParserTest.php (PHP)

Code before:
            ],
            'comment after space' => [
                <<<'YAML'
                {
                    foo: 3 # comment
                }
                YAML,
                ['foo' => 3],
            ],
            'comment after space, but missing space after #' => [
                <<<'YAML'
                {
                    foo: 3 #comment
                }
                YAML,
                ['foo' => 3],
            ],
            'comment after tab' => [
                <<<YAML
                {
                    foo: 3\t# comment
                }
                YAML,
                ['foo' => 3],
            ],
            'comment after tab, but missing space after #' => [
                <<<YAML
                {
                    foo: 3\t#comment
                }
                YAML,
                ['foo' => 3],
            ],
            '# in mapping value' => [
                <<<'YAML'
                {
                    foo: example.com/#about
                }
                YAML,
                ['foo' => 'example.com/#about'],
            ],
        ];
    }

    /**
     * @dataProvider escapedQuotationCharactersInQuotedStrings
     */
    public function testParseQuotedStringContainingEscapedQuotationCharacters(string $yaml, array $expected)
    {
        $this->assertSame($expected, $this->parser->parse($yaml));
    }

Review comment (suggestion):
I am not sure this is a valid example, all online parser that I found terminate parsing the value as soon as they stumble upon a comment line (see https://yaml-online-parser.appspot.com/?yaml=key%3A+foo%0A+bar%0A+%23+m%0A+foobar%0Akey2%3A+baz&type=json for such an example).

Code after:
            [
                'key' => 'unquoted next line final line',
                'another_key' => 'works',
            ],
        ];

        yield 'only comments' => [
            <<<YAML
                key: unquoted
                  # this comment should be ignored
                  # another comment
                another_key: works
                YAML,
            [
                'key' => 'unquoted',
                'another_key' => 'works',
            ],
        ];

        yield 'blank lines and comments' => [
            <<<YAML
                key: unquoted
                  next line

                  # this comment should be ignored
                  final line
                another_key: works
                YAML,
            [
                'key' => "unquoted next line\nfinal line",
                'another_key' => 'works',
            ],
        ];

        yield 'comment at end' => [
            <<<YAML
                key: unquoted
                  next line
                  # comment at end
                another_key: works
                YAML,
            [
                'key' => 'unquoted next line',
                'another_key' => 'works',
            ],
        ];
    }

    /**
     * @dataProvider unquotedStringWithTrailingComment
     */
Repository: symfony/symfony
File: src/Symfony/Component/JsonStreamer/Tests/Fixtures/Model/DummyWithArray.php (PHP)

Code before:
<?php

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

class DummyWithArray
{
    /** @var list<ClassicDummy> */
    public array $dummies;

    public string $customProperty;
}


Review comment (suggestion):
I am not sure if it is actually a good idea to do this change or if we shouldn’t introduce additional `DummyWithList` and `DummyWithNestedList` fixtures instead to cover both cases.

Code after:
<?php

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

class DummyWithArray
{
    /** @var ClassicDummy[] */
    public array $dummies;

    public string $customProperty;
}
Repository: symfony/symfony
File: src/Symfony/Component/Yaml/Tests/ParserTest.php (PHP)

Code before:
        yield 'comments interspersed stops scalar' => [
            <<<YAML
                key: unquoted
                    # this comment terminates
                another_key: works
                YAML,
            [
                'key' => 'unquoted',
                'another_key' => 'works',
            ],
        ];

        yield 'only comments' => [
            <<<YAML
                key: unquoted
                    # this comment terminates
                    # this is just another comment
                another_key: works
                YAML,
            [
                'key' => 'unquoted',
                'another_key' => 'works',
            ],
        ];

        yield 'blank lines are preserved and comment stops scalar' => [
            <<<YAML
                key: unquoted
                    next line

                    # this comment terminates the scalar
                another_key: works
                YAML,
            [
                'key' => 'unquoted next line',
                'another_key' => 'works',
            ],
        ];

        yield 'comment at end stops scalar' => [
            <<<YAML
                key: unquoted
                    next line
                    # comment at end
                another_key: works
                YAML,
            [
                'key' => 'unquoted next line',
                'another_key' => 'works',
            ],
        ];

Review comment (suggestion):
I think we should also have a test covering that an exception is thrown when the comment is not at the end.

Code after:
        yield 'comments interspersed stops scalar' => [
            <<<YAML
                key: unquoted
                  # this comment terminates
                another_key: works
                YAML,
            [
                'key' => 'unquoted',
                'another_key' => 'works',
            ],
        ];

        yield 'only comments' => [
            <<<YAML
                key: unquoted
                  # this comment should be ignored
                  # another comment
                another_key: works
                YAML,
            [
                'key' => 'unquoted',
                'another_key' => 'works',
            ],
        ];

        yield 'blank lines are preserved and comment stops scalar' => [
            <<<YAML
                key: unquoted
                  next line

                  # this comment terminates the scalar
                another_key: works
                YAML,
            [
                'key' => 'unquoted next line',
                'another_key' => 'works',
            ],
        ];

        yield 'comment at end' => [
            <<<YAML
                key: unquoted
                  next line
                  # comment at end
                another_key: works
                YAML,
            [
                'key' => 'unquoted next line',
                'another_key' => 'works',
            ],
        ];
Repository: symfony/symfony
File: src/Symfony/Component/HttpKernel/DependencyInjection/ConfigurableExtension.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\HttpKernel\DependencyInjection;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\Extension as DependencyInjectionExtension

/**
 * This extension sub-class provides first-class integration with the
 * Config/Definition Component.
 *
 * You can use this as base class if
 *
 *    a) you use the Config/Definition component for configuration,
 *    b) your configuration class is named "Configuration", and
 *    c) the configuration class resides in the DependencyInjection sub-folder.
 *
 * @author Johannes M. Schmitt <schmittjoh@gmail.com>
 */
abstract class ConfigurableExtension extends DependencyInjectionExtension
{
    final public function load(array $configs, ContainerBuilder $container): void
    {
        $this->loadInternal($this->processConfiguration($this->getConfiguration($configs, $container), $configs), $container);
    }

    /**
     * Configures the passed container according to the merged configuration.
     */
    abstract protected function loadInternal(array $mergedConfig, ContainerBuilder $container): void;
}

Review comment (security):
```suggestion
use Symfony\Component\DependencyInjection\Extension\Extension as DependencyInjectionExtension;
```

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

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\Extension;

/**
 * This extension sub-class provides first-class integration with the
 * Config/Definition Component.
 *
 * You can use this as base class if
 *
 *    a) you use the Config/Definition component for configuration,
 *    b) your configuration class is named "Configuration", and
 *    c) the configuration class resides in the DependencyInjection sub-folder.
 *
 * @author Johannes M. Schmitt <schmittjoh@gmail.com>
 */
abstract class ConfigurableExtension extends Extension
{
    final public function load(array $configs, ContainerBuilder $container): void
    {
        $this->loadInternal($this->processConfiguration($this->getConfiguration($configs, $container), $configs), $container);
    }

    /**
     * Configures the passed container according to the merged configuration.
     */
    abstract protected function loadInternal(array $mergedConfig, ContainerBuilder $container): void;
}
Repository: symfony/symfony
File: src/Symfony/Component/HttpKernel/DependencyInjection/ConfigurableExtension.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\HttpKernel\DependencyInjection;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\Extension as DependencyInjectionExtension;

/**
 * This extension sub-class provides first-class integration with the
 * Config/Definition Component.
 *
 * You can use this as base class if
 *
 *    a) you use the Config/Definition component for configuration,
 *    b) your configuration class is named "Configuration", and
 *    c) the configuration class resides in the DependencyInjection sub-folder.
 *
 * @author Johannes M. Schmitt <schmittjoh@gmail.com>
 */
abstract class ConfigurableExtension extends DependencyInjectionExtension
{
    final public function load(array $configs, ContainerBuilder $container): void
    {
        $this->loadInternal($this->processConfiguration($this->getConfiguration($configs, $container), $configs), $container);
    }

    /**
     * Configures the passed container according to the merged configuration.
     */
    abstract protected function loadInternal(array $mergedConfig, ContainerBuilder $container): void;
}

Review comment (security):
can't we remove the alias?
```suggestion
use Symfony\Component\DependencyInjection\Extension\Extension;
```

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

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\Extension;

/**
 * This extension sub-class provides first-class integration with the
 * Config/Definition Component.
 *
 * You can use this as base class if
 *
 *    a) you use the Config/Definition component for configuration,
 *    b) your configuration class is named "Configuration", and
 *    c) the configuration class resides in the DependencyInjection sub-folder.
 *
 * @author Johannes M. Schmitt <schmittjoh@gmail.com>
 */
abstract class ConfigurableExtension extends Extension
{
    final public function load(array $configs, ContainerBuilder $container): void
    {
        $this->loadInternal($this->processConfiguration($this->getConfiguration($configs, $container), $configs), $container);
    }

    /**
     * Configures the passed container according to the merged configuration.
     */
    abstract protected function loadInternal(array $mergedConfig, ContainerBuilder $container): void;
}
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 = 'Response';
}


Review comment (suggestion):
```suggestion
    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)
```

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';
}