ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Component/HttpFoundation/ParameterBag.php (PHP)

Code before:
    /**
     * Returns the parameters.
     *
     * @param string|null $key The name of the parameter to return or null to get them all
     *
     * @return array<string, mixed>
     *
     * @throws BadRequestException if the value is not an array
     */
    public function all(?string $key = null): array
    {
        if (null === $key) {
            return $this->parameters;
        }

        if (!\is_array($value = $this->parameters[$key] ?? [])) {
            throw new BadRequestException(\sprintf('Unexpected value for parameter "%s": expecting "array", got "%s".', $key, get_debug_type($value)));
        }

        return $value;
    }

    /**
     * Returns the parameter keys.
     *
     * @return array<int, string>
     */
    public function keys(): array
    {
        return array_keys($this->parameters);
    }

    /**
     * Replaces the current parameters by a new set.
     *
     * @param array<string, mixed> $parameters
     */
    public function replace(array $parameters = []): void
    {
        $this->parameters = $parameters;
    }

    /**
     * Adds parameters.
     *
     * @param array<string, mixed> $parameters
     */
    public function add(array $parameters = []): void
    {
        $this->parameters = array_replace($this->parameters, $parameters);
    }

Review comment (suggestion):
`list<string>` would be even better, as both Psalm and phpstan have full supports for lists.

Code after:
    /**
     * Returns the parameters.
     *
     * @template TKey of string|null
     *
     * @param TKey $key The name of the parameter to return or null to get them all
     *
     * @return (TKey is null ? array<string, mixed> : array<mixed>)
     *
     * @throws BadRequestException if the value is not an array
     */
    public function all(?string $key = null): array
    {
        if (null === $key) {
            return $this->parameters;
        }

        if (!\is_array($value = $this->parameters[$key] ?? [])) {
            throw new BadRequestException(\sprintf('Unexpected value for parameter "%s": expecting "array", got "%s".', $key, get_debug_type($value)));
        }

        return $value;
    }

    /**
     * Returns the parameter keys.
     *
     * @return list<string>
     */
    public function keys(): array
    {
        return array_keys($this->parameters);
    }

    /**
     * Replaces the current parameters by a new set.
     *
     * @param array<string, mixed> $parameters
     */
    public function replace(array $parameters = []): void
    {
        $this->parameters = $parameters;
    }

    /**
     * Adds parameters.
     *
     * @param array<string, mixed> $parameters
     */
    public function add(array $parameters = []): void
    {
Repository: symfony/symfony
File: src/Symfony/Component/HttpFoundation/ParameterBag.php (PHP)

Code before:

/**
 * ParameterBag is a container for key/value pairs.
 *
 * @author Fabien Potencier <fabien@symfony.com>
 *
 * @implements \IteratorAggregate<string, mixed>
 */
class ParameterBag implements \IteratorAggregate, \Countable
{
    /**
     * @param array<string, mixed> $parameters
     */
    public function __construct(
        protected array $parameters = [],
    ) {
    }

    /**
     * Returns the parameters.
     *
     * @template TKey of string|null
     *
     * @param TKey $key The name of the parameter to return or null to get them all
     *
     * @return (TKey is null ? array<string, mixed> : mixed)
     *
     * @throws BadRequestException if the value is not an array
     */
    public function all(?string $key = null): mixed
    {
        if (null === $key) {
            return $this->parameters;
        }

        if (!\is_array($value = $this->parameters[$key] ?? [])) {
            throw new BadRequestException(\sprintf('Unexpected value for parameter "%s": expecting "array", got "%s".', $key, get_debug_type($value)));
        }

        return $value;
    }

    /**
     * Returns the parameter keys.
     *
     * @return list<string>
     */
    public function keys(): array
    {
        return array_keys($this->parameters);
    }

Review comment (suggestion):
```suggestion
     * @return (TKey is null ? array<string, mixed> : array)
```

but that's a lot of type boilerplate for nothing IMHO.

Code after:

/**
 * ParameterBag is a container for key/value pairs.
 *
 * @author Fabien Potencier <fabien@symfony.com>
 *
 * @implements \IteratorAggregate<string, mixed>
 */
class ParameterBag implements \IteratorAggregate, \Countable
{
    /**
     * @param array<string, mixed> $parameters
     */
    public function __construct(
        protected array $parameters = [],
    ) {
    }

    /**
     * Returns the parameters.
     *
     * @template TKey of string|null
     *
     * @param TKey $key The name of the parameter to return or null to get them all
     *
     * @return (TKey is null ? array<string, mixed> : array<mixed>)
     *
     * @throws BadRequestException if the value is not an array
     */
    public function all(?string $key = null): array
    {
        if (null === $key) {
            return $this->parameters;
        }

        if (!\is_array($value = $this->parameters[$key] ?? [])) {
            throw new BadRequestException(\sprintf('Unexpected value for parameter "%s": expecting "array", got "%s".', $key, get_debug_type($value)));
        }

        return $value;
    }

    /**
     * Returns the parameter keys.
     *
     * @return list<string>
     */
    public function keys(): array
    {
        return array_keys($this->parameters);
    }
Repository: symfony/symfony
File: src/Symfony/Component/HttpFoundation/ParameterBag.php (PHP)

Code before:

/**
 * ParameterBag is a container for key/value pairs.
 *
 * @author Fabien Potencier <fabien@symfony.com>
 *
 * @implements \IteratorAggregate<string, mixed>
 */
class ParameterBag implements \IteratorAggregate, \Countable
{
    /**
     * @param array<string, mixed> $parameters
     */
    public function __construct(
        protected array $parameters = [],
    ) {
    }

    /**
     * Returns the parameters.
     *
     * @template TKey of string|null
     *
     * @param TKey $key The name of the parameter to return or null to get them all
     *
     * @return (TKey is null ? array<string, mixed> : array)
     *
     * @throws BadRequestException if the value is not an array
     */
    public function all(?string $key = null): array
    {
        if (null === $key) {
            return $this->parameters;
        }

        if (!\is_array($value = $this->parameters[$key] ?? [])) {
            throw new BadRequestException(\sprintf('Unexpected value for parameter "%s": expecting "array", got "%s".', $key, get_debug_type($value)));
        }

        return $value;
    }

    /**
     * Returns the parameter keys.
     *
     * @return list<string>
     */
    public function keys(): array
    {
        return array_keys($this->parameters);
    }

Review comment (suggestion):
can be improved a bit:
```suggestion
     * @return (TKey is null ? array<string, mixed> : array<mixed>)
```

Code after:

/**
 * ParameterBag is a container for key/value pairs.
 *
 * @author Fabien Potencier <fabien@symfony.com>
 *
 * @implements \IteratorAggregate<string, mixed>
 */
class ParameterBag implements \IteratorAggregate, \Countable
{
    /**
     * @param array<string, mixed> $parameters
     */
    public function __construct(
        protected array $parameters = [],
    ) {
    }

    /**
     * Returns the parameters.
     *
     * @template TKey of string|null
     *
     * @param TKey $key The name of the parameter to return or null to get them all
     *
     * @return (TKey is null ? array<string, mixed> : array<mixed>)
     *
     * @throws BadRequestException if the value is not an array
     */
    public function all(?string $key = null): array
    {
        if (null === $key) {
            return $this->parameters;
        }

        if (!\is_array($value = $this->parameters[$key] ?? [])) {
            throw new BadRequestException(\sprintf('Unexpected value for parameter "%s": expecting "array", got "%s".', $key, get_debug_type($value)));
        }

        return $value;
    }

    /**
     * Returns the parameter keys.
     *
     * @return list<string>
     */
    public function keys(): array
    {
        return array_keys($this->parameters);
    }
Repository: symfony/symfony
File: src/Symfony/Component/HttpKernel/Event/ResponseEvent.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\Event;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\HttpKernelInterface;

/**
 * Allows to filter a Response object.
 *
 * You can call getResponse() to retrieve the current response. With
 * setResponse() you can set a new response that will be returned to the
 * browser.
 *
 * @author Bernhard Schussek <bschussek@gmail.com>
 */
final class ResponseEvent extends KernelEvent
{
    public function __construct(
        HttpKernelInterface $kernel,
        Request $request,
        int $requestType,
        private Response $response,
    ) {
        parent::__construct($kernel, $request, $requestType);
    }

    public function getResponse(): Response
    {
        return $this->response;
    }

    public function setResponse(Response $response): void
    {
        $this->response = $response;
    }
}


Review comment (suggestion):
This does not properly document that the list of objects is related to `T` when passing a class name, making the generic type less useful.

This should use the same types than in ControllerEvent instead.

Code after:
        Request $request,
        int $requestType,
        private Response $response,
        private ?ControllerEvent $controllerEvent = null,
    ) {
        parent::__construct($kernel, $request, $requestType);
    }

    public function getResponse(): Response
    {
        return $this->response;
    }

    public function setResponse(Response $response): void
    {
        $this->response = $response;
    }

    /**
     * @template T of class-string|null
     *
     * @param T $className
     *
     * @return array<class-string, list<object>>|list<object>
     *
     * @psalm-return (T is null ? array<class-string, list<object>> : list<object>)
     */
    public function getControllerAttributes(?string $className = null): array
    {
        return $this->controllerEvent?->getAttributes($className) ?? [];
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/HttpClient/Internal/AmpListener.php (PHP)

Code before:
use Amp\Http\Client\Request;
use Amp\Http\Client\Response;
use Amp\Socket\InternetAddress;
use Revolt\EventLoop;
use Symfony\Component\HttpClient\Exception\TransportException;

/**
 * @author Nicolas Grekas <p@tchwork.com>
 *
 * @internal
 */
class AmpListener implements EventListener
{
    private array $info;
    private ?string $connectTimerId = null;

    /**
     * @param resource|null $handle
     */
    public function __construct(
        array &$info,
        private array $pinSha256,
        private \Closure $onProgress,
        private &$handle,
        private float $maxConnectDuration,
        private ?DeferredCancellation $canceller,
    ) {
        $info += [
            'connect_time' => 0.0,
            'pretransfer_time' => 0.0,
            'starttransfer_time' => 0.0,
            'total_time' => 0.0,
            'namelookup_time' => 0.0,
            'primary_ip' => '',
            'primary_port' => 0,
        ];

        $this->info = &$info;
    }

    public function requestStart(Request $request): void
    {
        $this->info['start_time'] ??= microtime(true);

        if (0 < $this->maxConnectDuration) {
            $this->connectTimerId = EventLoop::delay($this->maxConnectDuration, function (): void {
                $this->canceller?->cancel(new TransportException(\sprintf('Max connect duration was reached for "%s".', $this->info['url'])));
            });
        }

        ($this->onProgress)();

Review comment (suggestion):
```suggestion
        private DeferredCancellation $canceller,
```

Code after:
use Amp\Http\Client\Request;
use Amp\Http\Client\Response;
use Amp\Socket\InternetAddress;
use Revolt\EventLoop;
use Symfony\Component\HttpClient\Exception\TransportException;

/**
 * @author Nicolas Grekas <p@tchwork.com>
 *
 * @internal
 */
class AmpListener implements EventListener
{
    private array $info;
    private ?string $connectTimerId = null;

    /**
     * @param resource|null $handle
     */
    public function __construct(
        array &$info,
        private array $pinSha256,
        private \Closure $onProgress,
        private &$handle,
        private float $maxConnectDuration,
        private DeferredCancellation $canceller,
    ) {
        $info += [
            'connect_time' => 0.0,
            'pretransfer_time' => 0.0,
            'starttransfer_time' => 0.0,
            'total_time' => 0.0,
            'namelookup_time' => 0.0,
            'primary_ip' => '',
            'primary_port' => 0,
        ];

        $this->info = &$info;
    }

    public function requestStart(Request $request): void
    {
        $this->info['start_time'] ??= microtime(true);

        if (0 < $this->maxConnectDuration) {
            $this->connectTimerId = EventLoop::delay($this->maxConnectDuration, function (): void {
                $this->canceller->cancel(new TransportException(\sprintf('Max connect duration was reached for "%s".', $this->info['url'])));
            });
        }

        ($this->onProgress)();
Repository: symfony/symfony
File: src/Symfony/Component/TypeInfo/Type/ObjectShapeType.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\TypeInfo\Type;

use Symfony\Component\TypeInfo\Type;
use Symfony\Component\TypeInfo\TypeIdentifier;

/**
 * Represents the exact shape of an anonymous object.
 *
 * @author Benjamin Franzke <ben@bnf.dev>
 *
 * @extends CollectionType<GenericType<BuiltinType<TypeIdentifier::OBJECT>>>
 */
final class ObjectShapeType extends CollectionType
{
    /**
     * @var array<string, array{type: Type, optional: bool}>
     */
    private readonly array $shape;

    /**
     * @param array<string, array{type: Type, optional: bool}> $shape
     */
    public function __construct(
        array $shape,
    ) {
        $valueTypes = array_column($shape, 'type');
        $valueType = [] !== $valueTypes ? CollectionType::mergeCollectionValueTypes($valueTypes) : Type::mixed();

        parent::__construct(Type::generic(Type::builtin(TypeIdentifier::OBJECT), Type::string(), $valueType));

        $sortedShape = $shape;
        ksort($sortedShape);

        $this->shape = $sortedShape;
    }

    /**
     * @return array<string, array{type: Type, optional: bool}>

Review comment (suggestion):
I'm not sure it should extend `CollectionType`. Conceptually, an object is not a collection.

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\TypeInfo\Type;

use Symfony\Component\TypeInfo\Type;
use Symfony\Component\TypeInfo\TypeIdentifier;

/**
 * Represents the exact shape of an anonymous object.
 *
 * @author Benjamin Franzke <ben@bnf.dev>
 */
final class ObjectShapeType extends Type
{
    /**
     * @var array<string, array{type: Type, optional: bool}>
     */
    private readonly array $shape;

    /**
     * @param array<string, array{type: Type, optional: bool}> $shape
     */
    public function __construct(
        array $shape,
    ) {
        $sortedShape = $shape;
        ksort($sortedShape);

        $this->shape = $sortedShape;
    }

    public function getTypeIdentifier(): TypeIdentifier
    {
        return TypeIdentifier::OBJECT;
    }

    /**
     * @return array<string, array{type: Type, optional: bool}>
     */
    public function getShape(): array
Repository: symfony/symfony
File: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TestServiceContainerRealRefPass.php (PHP)

Code before:
                unset($privateServices[$id]);
            }
        }

        foreach ($container->getAliases() as $id => $target) {
            while ($container->hasAlias($target = (string) $target)) {
                $target = $container->getAlias($target);
            }

            if ($definitions[$target]->hasTag('container.private')) {
                $privateServices[$id] = new ServiceClosureArgument(new Reference($target));
            }

            $renamedIds[$id] = $target;
        }

        $privateContainer->replaceArgument(0, $privateServices);

        if ($container->hasDefinition('test.service_container') && $renamedIds) {
            $container->getDefinition('test.service_container')->setArgument(2, $renamedIds);
        }

        $nonSharedServices = [];

        foreach ($definitions as $id => $definition) {
            if ($id && '.' !== $id[0] && !$definition->isShared() && !$definition->hasErrors() && !$definition->isAbstract()) {
                $nonSharedServices[$id] = true;
            }
        }

        if ($container->hasDefinition('test.service_container') && $nonSharedServices) {
            $container->getDefinition('test.service_container')->setArgument(3, $nonSharedServices);
        }
    }
}


Review comment (refactor):
```suggestion
            if (($id && '.' !== $id[0] || isset($privateServices[$id]) && !$definition->isShared() && !$definition->hasErrors() && !$definition->isAbstract()) {
```

Code after:
                unset($privateServices[$id]);
            }
        }

        foreach ($container->getAliases() as $id => $target) {
            while ($container->hasAlias($target = (string) $target)) {
                $target = $container->getAlias($target);
            }

            if ($definitions[$target]->hasTag('container.private')) {
                $privateServices[$id] = new ServiceClosureArgument(new Reference($target));
            }

            $renamedIds[$id] = $target;
        }

        $privateContainer->replaceArgument(0, $privateServices);

        if ($container->hasDefinition('test.service_container') && $renamedIds) {
            $container->getDefinition('test.service_container')->setArgument(2, $renamedIds);
        }

        $nonSharedServices = [];

        foreach ($definitions as $id => $definition) {
            if (($id && '.' !== $id[0] || isset($privateServices[$id])) && !$definition->isShared() && !$definition->hasErrors() && !$definition->isAbstract()) {
                $nonSharedServices[$id] = true;
            }
        }

        if ($container->hasDefinition('test.service_container') && $nonSharedServices) {
            $container->getDefinition('test.service_container')->setArgument(3, $nonSharedServices);
        }
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Security/Core/Authorization/ExpressionLanguage.php (PHP)

Code before:
    class_exists(ExpressionLanguageProvider::class);

    /**
     * Adds some function to the default ExpressionLanguage.
     *
     * @author Fabien Potencier <fabien@symfony.com>
     *
     * @see ExpressionLanguageProvider
     */
    class ExpressionLanguage extends BaseExpressionLanguage
    {
        public function __construct(?CacheItemPoolInterface $cache = null, iterable $providers = [])
        {
            if (!\is_array($providers)) {
                $providers = iterator_to_array($providers, false);
            }

            // prepend the default provider to let users override it easily
            array_unshift($providers, new ExpressionLanguageProvider());

            parent::__construct($cache, $providers);
        }
    }
}

// @php-cs-fixer-ignore no_useless_else Keep class-declaration inside if-else condition to help OPCache, ref https://github.com/symfony/symfony/pull/62926#discussion_r2659506407


Review comment (performance):
```suggestion
// @php-cs-fixer-ignore no_useless_else Keep class-declaration inside if-else condition to help OPCache
```

Code after:
    class_exists(ExpressionLanguageProvider::class);

    /**
     * Adds some function to the default ExpressionLanguage.
     *
     * @author Fabien Potencier <fabien@symfony.com>
     *
     * @see ExpressionLanguageProvider
     */
    class ExpressionLanguage extends BaseExpressionLanguage
    {
        public function __construct(?CacheItemPoolInterface $cache = null, iterable $providers = [])
        {
            if (!\is_array($providers)) {
                $providers = iterator_to_array($providers, false);
            }

            // prepend the default provider to let users override it easily
            array_unshift($providers, new ExpressionLanguageProvider());

            parent::__construct($cache, $providers);
        }
    }
}

// @php-cs-fixer-ignore no_useless_else Keep class-declaration inside if-else condition to help OPCache, especially to prevent class declaration if conditional early-exit on runtime is executed