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

Code before:
    {
        return null === $this->expiringTime ? null : $this->expiringTime - microtime(true);
    }

    public function isExpired(): bool
    {
        return null !== $this->expiringTime && $this->expiringTime <= microtime(true);
    }

    public function __unserialize(array $data): void
    {
        $this->resource = $data['resource'] ?? $data["\0".self::class."\0resource"];
        $this->limit = $data['limit'] ?? $data["\0".self::class."\0limit"];
        $this->weight = $data['weight'] ?? $data["\0".self::class."\0weight"];
        $this->expiringTime = $data['expiringTime'] ?? $data["\0".self::class."\0expiringTime"] ?? null;
        $this->state = $data['state'] ?? $data["\0".self::class."\0state"] ?? [];
    }

    public function __serialize(): array
    {
        return [
            'resource' => $this->resource,
            'limit' => $this->limit,
            'weight' => $this->weight,
            'expiringTime' => $this->expiringTime,
            'state' => $this->state,
        ];
    }
}


Review comment (suggestion):
If we start supporting serialization of semaphore keys, we should probably introduce the `markUnserializable` (similar to the `Lock` component), to support implementing stores which don't support key serialization.

Code after:
        }
    }

    /**
     * @return float|null Remaining lifetime in seconds. Null when the key won't expire.
     */
    public function getRemainingLifetime(): ?float
    {
        return null === $this->expiringTime ? null : $this->expiringTime - microtime(true);
    }

    public function isExpired(): bool
    {
        return null !== $this->expiringTime && $this->expiringTime <= microtime(true);
    }

    public function __unserialize(array $data): void
    {
        $this->resource = $data['resource'] ?? $data["\0".self::class."\0resource"];
        $this->limit = $data['limit'] ?? $data["\0".self::class."\0limit"];
        $this->weight = $data['weight'] ?? $data["\0".self::class."\0weight"];
        $this->expiringTime = $data['expiringTime'] ?? $data["\0".self::class."\0expiringTime"] ?? null;
        $this->state = $data['state'] ?? $data["\0".self::class."\0state"] ?? [];
    }

    public function __serialize(): array
    {
        if (!$this->serializable) {
            throw new UnserializableKeyException('The key cannot be serialized.');
        }

        return [
            'resource' => $this->resource,
            'limit' => $this->limit,
            'weight' => $this->weight,
            'expiringTime' => $this->expiringTime,
            'state' => $this->state,
        ];
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Messenger/Transport/Serialization/Serializer.php (PHP)

Code before:
    }

    private function getContentTypeHeader(): array
    {
        $mimeType = $this->getMimeTypeForFormat();

        return null === $mimeType ? [] : ['Content-Type' => $mimeType];
    }

    private function getMimeTypeForFormat(): ?string
    {
        return match ($this->format) {
            'json' => 'application/json',
            'xml' => 'application/xml',
            'yml',
            'yaml' => 'application/yaml',
            'csv' => 'text/csv',
            default => null,
        };
    }

    private function getTypeFromEnvelope(Envelope $envelope): string
    {
        $messageClass = $envelope->getMessage()::class;

        if ($this->classToTypeMap[$messageClass] ?? false) {
            return $this->classToTypeMap[$messageClass];
        }

        foreach ([$messageClass] + class_parents($messageClass) + class_implements($messageClass) as $class) {
            foreach ((new \ReflectionClass($class))->getAttributes(AsMessage::class, \ReflectionAttribute::IS_INSTANCEOF) as $refAttr) {
                $asMessage = $refAttr->newInstance();

                if ($asMessage->serializedTypeName) {
                    return $this->classToTypeMap[$messageClass] = $asMessage->serializedTypeName;
                }
            }
        }

        return $this->classToTypeMap[$messageClass] = $messageClass;
    }
}


Review comment (suggestion):
for extra safety:
```suggestion
        if (isset($this->classToTypeMap[$messageClass])) {
```

Code after:
    }

    private function getContentTypeHeader(): array
    {
        $mimeType = $this->getMimeTypeForFormat();

        return null === $mimeType ? [] : ['Content-Type' => $mimeType];
    }

    private function getMimeTypeForFormat(): ?string
    {
        return match ($this->format) {
            'json' => 'application/json',
            'xml' => 'application/xml',
            'yml',
            'yaml' => 'application/yaml',
            'csv' => 'text/csv',
            default => null,
        };
    }

    private function getTypeFromEnvelope(Envelope $envelope): string
    {
        $messageClass = $envelope->getMessage()::class;

        if (isset($this->classToTypeMap[$messageClass])) {
            return $this->classToTypeMap[$messageClass];
        }

        foreach ([$messageClass] + class_parents($messageClass) + class_implements($messageClass) as $class) {
            foreach ((new \ReflectionClass($class))->getAttributes(AsMessage::class, \ReflectionAttribute::IS_INSTANCEOF) as $refAttr) {
                $asMessage = $refAttr->newInstance();

                if ($asMessage->serializedTypeName) {
                    return $this->classToTypeMap[$messageClass] = $asMessage->serializedTypeName;
                }
            }
        }

        return $this->classToTypeMap[$messageClass] = $messageClass;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/HttpKernel/Attribute/Cache.php (PHP)

Code before:
         * stale-if-error Cache-Control header
         * It can be expressed in seconds or with a relative time format (1 day, 2 weeks, ...).
         */
        public int|string|null $staleIfError = null,

        /**
         * Add the "no-store" Cache-Control directive when set to true.
         *
         * This directive indicates that no part of the response can be cached
         * in any cache (not in a shared cache, nor in a private cache).
         *
         * Supersedes the "$public" and "$smaxage" values.
         *
         * @see https://datatracker.ietf.org/doc/html/rfc7234#section-5.2.2.3
         */
        public ?bool $noStore = null,

        /**
         * A value evaluated to determine whether the cache attribute should be applied.
         *
         * The value may be either an ExpressionLanguage expression or a Closure and
         * receives all the request attributes and the resolved controller arguments.
         *
         * The result must be a boolean. If true the attribute is applied, if false it is ignored.
         *
         * @var string|Expression|\Closure(array<string, mixed>, Request): bool|bool
         */
        public string|Expression|\Closure|bool $if = true,
    ) {
    }
}


Review comment (suggestion):
```suggestion
         * @var string|Expression|bool|\Closure(array<string, mixed>, Request): bool
```

Switching the order of types makes the code less confusing for people not aware of the precedence rules for `|` vs the callable signature notation.

Code after:
         * stale-if-error Cache-Control header
         * It can be expressed in seconds or with a relative time format (1 day, 2 weeks, ...).
         */
        public int|string|null $staleIfError = null,

        /**
         * Add the "no-store" Cache-Control directive when set to true.
         *
         * This directive indicates that no part of the response can be cached
         * in any cache (not in a shared cache, nor in a private cache).
         *
         * Supersedes the "$public" and "$smaxage" values.
         *
         * @see https://datatracker.ietf.org/doc/html/rfc7234#section-5.2.2.3
         */
        public ?bool $noStore = null,

        /**
         * A value evaluated to determine whether the cache attribute should be applied.
         *
         * The value may be either an ExpressionLanguage expression or a Closure and
         * receives all the request attributes and the resolved controller arguments.
         *
         * The result must be a boolean. If true the attribute is applied, if false it is ignored.
         *
         * @var bool|string|Expression|\Closure(array<string, mixed>, Request):bool
         */
        public bool|string|Expression|\Closure $if = true,
    ) {
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/HttpKernel/Event/ControllerMetadata.php (PHP)

Code before:
    }

    /**
     * @template T of object
     *
     * @param class-string<T>|'*'|null $className
     *
     * @return ($className is null ? array<class-string, list<object>> : ($className is '*' ? list<object> : list<T>))
     */
    public function getAttributes(?string $className = null): array
    {
        return $this->controllerEvent->getAttributes($className);
    }

    /**
     * Returns the controller arguments if they have been resolved, null otherwise.
     */
    public function getArguments(): ?array
    {
        return $this->controllerArgumentsEvent?->getArguments();
    }

    /**
     * Returns the controller arguments mapped to parameter names if they have been resolved, null otherwise.
     */
    public function getNamedArguments(): ?array
    {
        return $this->controllerArgumentsEvent?->getNamedArguments();
    }
}


Review comment (suggestion):
this should have a phpdoc `@return array<string, mixed>|null` here.

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

/**
 * Provides read-only access to controller metadata.
 *
 * @author Nicolas Grekas <p@tchwork.com>
 */
class ControllerMetadata
{
    public function __construct(
        private ControllerEvent $controllerEvent,
    ) {
    }

    public function getController(): callable
    {
        return $this->controllerEvent->getController();
    }

    public function getReflector(): \ReflectionFunctionAbstract
    {
        return $this->controllerEvent->getControllerReflector();
    }

    /**
     * @template T of object
     *
     * @param class-string<T>|'*'|null $className
     *
     * @return ($className is null ? array<class-string, list<object>> : ($className is '*' ? list<object> : list<T>))
     */
    public function getAttributes(?string $className = null): array
    {
        return $this->controllerEvent->getAttributes($className);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Security/Csrf/SameOriginCsrfListener.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\Security\Csrf;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\HttpKernel\Event\ResponseEvent;

final class SameOriginCsrfListener
{
    public function __construct(private readonly string $cookieName = 'csrf-token')
    {
    }

    public function onKernelResponse(ResponseEvent $event): void
    {
        if (!$event->isMainRequest()) {
            return;
        }

        $this->clearCookies($event->getRequest(), $event->getResponse());
        $this->persistStrategy($event->getRequest());
    }

    private function clearCookies(Request $request, Response $response): void
    {
        if (!$request->attributes->has($this->cookieName)) {
            return;
        }

        $cookieName = ($request->isSecure() ? '__Host-' : '').$this->cookieName;

        foreach ($request->cookies->all() as $name => $value) {
            if ($this->cookieName === $value && str_starts_with($name, $cookieName.'_')) {
                $response->headers->clearCookie($name, '/', null, $request->isSecure(), false, 'strict');
            }
        }

Review comment (security):
```suggestion
    public function __construct(
        private readonly string $cookieName = 'csrf-token',
    ) {
```

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\Security\Csrf;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\HttpKernel\Event\ResponseEvent;

final class SameOriginCsrfListener
{
    public function __construct(
        private readonly string $cookieName = 'csrf-token')
    {
    }

    public function onKernelResponse(ResponseEvent $event): void
    {
        if (!$event->isMainRequest()) {
            return;
        }

        $this->clearCookies($event->getRequest(), $event->getResponse());
        $this->persistStrategy($event->getRequest());
    }

    private function clearCookies(Request $request, Response $response): void
    {
        if (!$request->attributes->has($this->cookieName)) {
            return;
        }

        $cookieName = ($request->isSecure() ? '__Host-' : '').$this->cookieName;

        foreach ($request->cookies->all() as $name => $value) {
            if ($this->cookieName === $value && str_starts_with($name, $cookieName.'_')) {
                $response->headers->clearCookie($name, '/', null, $request->isSecure(), false, 'strict');
            }
Repository: symfony/symfony
File: src/Symfony/Component/JsonStreamer/StreamerDumper.php (PHP)

Code before:
        }

        if (!$this->fs->exists($path)) {
            $this->fs->dumpFile($path, $generateContent());
        }
    }

    /**
     * Retrieves resource class names (as keys) required for caching based on the provided type.
     *
     * @param array<class-string, true> $classNames
     * @param array<string, mixed>      $context
     *
     * @return array<class-string, true>
     */
    private function getResourceClassNames(Type $type, array $classNames = [], array $context = []): array
    {
        $context['original_type'] ??= $type;

        foreach ($type->traverse() as $t) {
            if ($t instanceof ObjectType) {
                if (isset($classNames[$t->getClassName()])) {
                    continue;
                }

                $classNames[$t->getClassName()] = true;

                foreach ($this->propertyMetadataLoader->load($t->getClassName(), [], $context) as $property) {
                    $classNames += $this->getResourceClassNames($property->getType(), $classNames, $context);
                }
            }

            if ($t instanceof GenericType) {
                foreach ($t->getVariableTypes() as $variableType) {
                    $classNames += $this->getResourceClassNames($variableType, $classNames, $context);
                }
            }
        }

        return $classNames;
    }
}


Review comment (suggestion):
```suggestion
                $classNames[$c] = $c;
```

Code after:
        }

        if (!$this->fs->exists($path)) {
            $this->fs->dumpFile($path, $generateContent());
        }
    }

    /**
     * Retrieves resource class names required for caching based on the provided type.
     *
     * @param array<class-string, class-string> $classNames
     * @param array<string, mixed>              $context
     *
     * @return array<class-string, class-string>
     */
    private function getResourceClassNames(Type $type, array $classNames = [], array $context = []): array
    {
        $context['original_type'] ??= $type;

        foreach ($type->traverse() as $t) {
            if ($t instanceof ObjectType) {
                if (isset($classNames[$className = $t->getClassName()])) {
                    continue;
                }

                $classNames[$className] = $className;

                foreach ($this->propertyMetadataLoader->load($className, [], $context) as $property) {
                    $classNames += $this->getResourceClassNames($property->getType(), $classNames, $context);
                }
            }

            if ($t instanceof GenericType) {
                foreach ($t->getVariableTypes() as $variableType) {
                    $classNames += $this->getResourceClassNames($variableType, $classNames, $context);
                }
            }
        }

        return $classNames;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Console/Input/InputArgument.php (PHP)

Code before:
    }

    /**
     * Returns true if the argument is required.
     *
     * @return bool true if parameter mode is self::REQUIRED, false otherwise
     */
    public function isRequired(): bool
    {
        return self::REQUIRED === (self::REQUIRED & $this->mode);
    }

    /**
     * Returns true if the argument can take multiple values.
     *
     * @return bool true if mode is self::IS_ARRAY, false otherwise
     */
    public function isArray(): bool
    {
        return self::IS_ARRAY === (self::IS_ARRAY & $this->mode);
    }

    /**
     * Sets the default value.
     */
    public function setDefault(string|bool|int|float|array|object|null $default): void
    {
        if ($this->isRequired() && null !== $default) {
            throw new LogicException('Cannot set a default value except for InputArgument::OPTIONAL mode.');
        }

        if ($this->isArray()) {
            if (null === $default) {
                $default = [];
            } elseif (!\is_array($default)) {
                throw new LogicException('A default value for an array argument must be an array.');
            }
        }

        $this->default = $default;
    }

    /**
     * Returns the default value.
     */
    public function getDefault(): string|bool|int|float|array|object|null
    {
        return $this->default;
    }

    /**

Review comment (suggestion):
ref https://symfony.com/doc/current/contributing/code/bc.html#changing-classes

`Change argument type` is not allowed to keep BC promise in place

Code after:
    }

    /**
     * Returns true if the argument is required.
     *
     * @return bool true if parameter mode is self::REQUIRED, false otherwise
     */
    public function isRequired(): bool
    {
        return self::REQUIRED === (self::REQUIRED & $this->mode);
    }

    /**
     * Returns true if the argument can take multiple values.
     *
     * @return bool true if mode is self::IS_ARRAY, false otherwise
     */
    public function isArray(): bool
    {
        return self::IS_ARRAY === (self::IS_ARRAY & $this->mode);
    }

    /**
     * Sets the default value.
     */
    public function setDefault(mixed $default): void
    {
        if ($this->isRequired() && null !== $default) {
            throw new LogicException('Cannot set a default value except for InputArgument::OPTIONAL mode.');
        }

        if ($this->isArray()) {
            if (null === $default) {
                $default = [];
            } elseif (!\is_array($default)) {
                throw new LogicException('A default value for an array argument must be an array.');
            }
        }

        $this->default = $default;
    }

    /**
     * Returns the default value.
     */
    public function getDefault(): mixed
    {
        return $this->default;
    }

    /**
Repository: symfony/symfony
File: src/Symfony/Component/Mailer/Bridge/Resend/Transport/ResendApiTransport.php (PHP)

Code before:
    private function prepareHeadersAndTags(Headers $headers): array
    {
        $headersAndTags = [];
        $headersToBypass = ['from', 'to', 'cc', 'bcc', 'subject', 'reply_to'];
        foreach ($headers->all() as $name => $header) {
            if (\in_array($name, $headersToBypass, true)) {
                continue;
            }

            if ($header instanceof TagHeader) {
                $headersAndTags['tags'][] = [$header->getName() => $header->getValue()];

                continue;
            }

            if ('templateid' === $name) {
                $headersAndTags['template']['id'] = $header->getValue();

                continue;
            }

            if ('variables' === $name) {
                $headersAndTags['template'][$header->getName()] = $header->getParameters();

                continue;
            }

            $headersAndTags['headers'][$header->getName()] = $header->getBodyAsString();
        }

        return $headersAndTags;
    }

    private function formatAddress(Address $address): string
    {
        $formattedAddress = $address->getEncodedAddress();

        if ($address->getName()) {
            $formattedAddress = $address->getName().' <'.$formattedAddress.'>';
        }

        return $formattedAddress;
    }

    private function getEndpoint(): ?string
    {
        return ($this->host ?: 'api.resend.com').($this->port ? ':'.$this->port : '');
    }
}


Review comment (suggestion):
To not break current implementation, I'd also handle `params` name to transform it into a `variables` one. (And add tests to cover it)

Code after:
    private function prepareHeadersAndTags(Headers $headers): array
    {
        $headersAndTags = [];
        $headersToBypass = ['from', 'to', 'cc', 'bcc', 'subject', 'reply_to'];
        foreach ($headers->all() as $name => $header) {
            if (\in_array($name, $headersToBypass, true)) {
                continue;
            }

            if ($header instanceof TagHeader) {
                $headersAndTags['tags'][] = [$header->getName() => $header->getValue()];

                continue;
            }

            $headersAndTags['headers'][$header->getName()] = $header->getBodyAsString();
        }

        return $headersAndTags;
    }

    private function formatAddress(Address $address): string
    {
        $formattedAddress = $address->getEncodedAddress();

        if ($address->getName()) {
            $formattedAddress = $address->getName().' <'.$formattedAddress.'>';
        }

        return $formattedAddress;
    }

    private function getEndpoint(): ?string
    {
        return ($this->host ?: 'api.resend.com').($this->port ? ':'.$this->port : '');
    }
}