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

Code before:
            return \DateTimeImmutable::createFromFormat('U', time())->add($expiration)->format('U');
        }

        return (string) $expiration;
    }

    /**
     * @return self::STATUS_*
     */
    private function doVerify(string $uri): int
    {
        $url = parse_url($uri);
        $params = [];

        if (isset($url['query'])) {
            parse_str($url['query'], $params);
        }

        if (empty($params[$this->hashParameter])) {
            return self::STATUS_MISSING;
        }

        $hash = $params[$this->hashParameter];
        unset($params[$this->hashParameter]);

        // In 8.0, remove support for non-url-safe tokens
        if (!hash_equals($this->computeHash($this->buildUrl($url, $params)), strtr(rtrim($hash, '='), ['/' => '_', '+' => '-']))) {
            return self::STATUS_INVALID;
        }

        if (!$expiration = $params[$this->expirationParameter] ?? false) {
            return self::STATUS_VALID;
        }

        if (time() < $expiration) {
            return self::STATUS_VALID;
        }

        return self::STATUS_EXPIRED;
    }

    private static function normalize(Request|string $uri): string
    {
        if ($uri instanceof Request) {
            $qs = ($qs = $uri->server->get('QUERY_STRING')) ? '?'.$qs : '';
            $uri = $uri->getSchemeAndHttpHost().$uri->getBaseUrl().$uri->getPathInfo().$qs;
        }

        return $uri;
    }
}

Review comment (suggestion):
to make it easier to spot when preparing 8.0
```suggestion
        // @deprecated: In 8.0, remove support for non-url-safe tokens
```

Code after:

        return (string) $expiration;
    }

    private function now(): \DateTimeImmutable
    {
        return $this->clock?->now() ?? \DateTimeImmutable::createFromFormat('U', time());
    }

    /**
     * @return self::STATUS_*
     */
    private function doVerify(string $uri): int
    {
        $url = parse_url($uri);
        $params = [];

        if (isset($url['query'])) {
            parse_str($url['query'], $params);
        }

        if (empty($params[$this->hashParameter])) {
            return self::STATUS_MISSING;
        }

        $hash = $params[$this->hashParameter];
        unset($params[$this->hashParameter]);

        if (!hash_equals($this->computeHash($this->buildUrl($url, $params)), strtr(rtrim($hash, '='), ['/' => '_', '+' => '-']))) {
            return self::STATUS_INVALID;
        }

        if (!$expiration = $params[$this->expirationParameter] ?? false) {
            return self::STATUS_VALID;
        }

        if ($this->now()->getTimestamp() < $expiration) {
            return self::STATUS_VALID;
        }

        return self::STATUS_EXPIRED;
    }

    private static function normalize(Request|string $uri): string
    {
        if ($uri instanceof Request) {
            $qs = ($qs = $uri->server->get('QUERY_STRING')) ? '?'.$qs : '';
            $uri = $uri->getSchemeAndHttpHost().$uri->getBaseUrl().$uri->getPathInfo().$qs;
        }

        return $uri;
Repository: symfony/symfony
File: src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php (PHP)

Code before:
    private function formatDateTimeErrors(array $errors): array
    {
        $formattedErrors = [];

        foreach ($errors as $pos => $message) {
            $formattedErrors[] = \sprintf('at position %d: %s', $pos, $message);
        }

        return $formattedErrors;
    }

    private function getTimezone(array $context): ?\DateTimeZone
    {
        $dateTimeZone = $context[self::TIMEZONE_KEY] ?? $this->defaultContext[self::TIMEZONE_KEY];

        if (null === $dateTimeZone) {
            return null;
        }

        return $dateTimeZone instanceof \DateTimeZone ? $dateTimeZone : new \DateTimeZone($dateTimeZone);
    }

    private function preserveContextTimezone(\DateTime|\DateTimeImmutable $object, array $context): \DateTime|\DateTimeImmutable
    {
        $timezone = $this->getTimezone($context);
        $preserveTimezone = $context[self::PRESERVE_CONTEXT_TIMEZONE_KEY] ?? $this->defaultContext[self::PRESERVE_CONTEXT_TIMEZONE_KEY] === true;

        if (null === $timezone || false === $preserveTimezone) {
            return $object;
        }

        return $object->setTimezone($timezone);
    }
}


Review comment (suggestion):
```suggestion
        $preserveTimezone = $context[self::PRESERVE_CONTEXT_TIMEZONE_KEY] ?? $this->defaultContext[self::PRESERVE_CONTEXT_TIMEZONE_KEY];
```
Might be enough

Code after:
        $formattedErrors = [];

        foreach ($errors as $pos => $message) {
            $formattedErrors[] = \sprintf('at position %d: %s', $pos, $message);
        }

        return $formattedErrors;
    }

    private function getTimezone(array $context): ?\DateTimeZone
    {
        $dateTimeZone = $context[self::TIMEZONE_KEY] ?? $this->defaultContext[self::TIMEZONE_KEY];

        if (null === $dateTimeZone) {
            return null;
        }

        return $dateTimeZone instanceof \DateTimeZone ? $dateTimeZone : new \DateTimeZone($dateTimeZone);
    }

    private function enforceTimezone(\DateTime|\DateTimeImmutable $object, array $context): \DateTimeInterface
    {
        $timezone = $this->getTimezone($context);
        $forceTimezone = $context[self::FORCE_TIMEZONE_KEY] ?? $this->defaultContext[self::FORCE_TIMEZONE_KEY];

        if (null === $timezone || !$forceTimezone) {
            return $object;
        }

        return $object->setTimezone($timezone);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php (PHP)

Code before:
        $formattedErrors = [];

        foreach ($errors as $pos => $message) {
            $formattedErrors[] = \sprintf('at position %d: %s', $pos, $message);
        }

        return $formattedErrors;
    }

    private function getTimezone(array $context): ?\DateTimeZone
    {
        $dateTimeZone = $context[self::TIMEZONE_KEY] ?? $this->defaultContext[self::TIMEZONE_KEY];

        if (null === $dateTimeZone) {
            return null;
        }

        return $dateTimeZone instanceof \DateTimeZone ? $dateTimeZone : new \DateTimeZone($dateTimeZone);
    }

    private function preserveContextTimezone(\DateTime|\DateTimeImmutable $object, array $context): \DateTime|\DateTimeImmutable
    {
        $timezone = $this->getTimezone($context);
        $preserveTimezone = $context[self::PRESERVE_CONTEXT_TIMEZONE_KEY] ?? $this->defaultContext[self::PRESERVE_CONTEXT_TIMEZONE_KEY] === true;

        if (null === $timezone || false === $preserveTimezone) {
            return $object;
        }

        return $object->setTimezone($timezone);
    }
}


Review comment (suggestion):
```suggestion
        if (null === $timezone || !$preserveTimezone) {
```

Code after:
        foreach ($errors as $pos => $message) {
            $formattedErrors[] = \sprintf('at position %d: %s', $pos, $message);
        }

        return $formattedErrors;
    }

    private function getTimezone(array $context): ?\DateTimeZone
    {
        $dateTimeZone = $context[self::TIMEZONE_KEY] ?? $this->defaultContext[self::TIMEZONE_KEY];

        if (null === $dateTimeZone) {
            return null;
        }

        return $dateTimeZone instanceof \DateTimeZone ? $dateTimeZone : new \DateTimeZone($dateTimeZone);
    }

    private function enforceTimezone(\DateTime|\DateTimeImmutable $object, array $context): \DateTimeInterface
    {
        $timezone = $this->getTimezone($context);
        $forceTimezone = $context[self::FORCE_TIMEZONE_KEY] ?? $this->defaultContext[self::FORCE_TIMEZONE_KEY];

        if (null === $timezone || !$forceTimezone) {
            return $object;
        }

        return $object->setTimezone($timezone);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php (PHP)

Code before:
        $formattedErrors = [];

        foreach ($errors as $pos => $message) {
            $formattedErrors[] = \sprintf('at position %d: %s', $pos, $message);
        }

        return $formattedErrors;
    }

    private function getTimezone(array $context): ?\DateTimeZone
    {
        $dateTimeZone = $context[self::TIMEZONE_KEY] ?? $this->defaultContext[self::TIMEZONE_KEY];

        if (null === $dateTimeZone) {
            return null;
        }

        return $dateTimeZone instanceof \DateTimeZone ? $dateTimeZone : new \DateTimeZone($dateTimeZone);
    }

    private function forceTimezone(\DateTime|\DateTimeImmutable $object, array $context): \DateTimeInterface
    {
        $timezone = $this->getTimezone($context);
        $preserveTimezone = $context[self::FORCE_TIMEZONE_KEY] ?? $this->defaultContext[self::FORCE_TIMEZONE_KEY];

        if (null === $timezone || !$preserveTimezone) {
            return $object;
        }

        return $object->setTimezone($timezone);
    }
}


Review comment (suggestion):
```suggestion
        $forceTimezone = $context[self::FORCE_TIMEZONE_KEY] ?? $this->defaultContext[self::FORCE_TIMEZONE_KEY];

        if (null === $timezone || !$forceTimezone) {
```

Code after:
        $formattedErrors = [];

        foreach ($errors as $pos => $message) {
            $formattedErrors[] = \sprintf('at position %d: %s', $pos, $message);
        }

        return $formattedErrors;
    }

    private function getTimezone(array $context): ?\DateTimeZone
    {
        $dateTimeZone = $context[self::TIMEZONE_KEY] ?? $this->defaultContext[self::TIMEZONE_KEY];

        if (null === $dateTimeZone) {
            return null;
        }

        return $dateTimeZone instanceof \DateTimeZone ? $dateTimeZone : new \DateTimeZone($dateTimeZone);
    }

    private function enforceTimezone(\DateTime|\DateTimeImmutable $object, array $context): \DateTimeInterface
    {
        $timezone = $this->getTimezone($context);
        $forceTimezone = $context[self::FORCE_TIMEZONE_KEY] ?? $this->defaultContext[self::FORCE_TIMEZONE_KEY];

        if (null === $timezone || !$forceTimezone) {
            return $object;
        }

        return $object->setTimezone($timezone);
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Notifier/Bridge/Discord/DiscordOptions.php (PHP)

Code before:
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\Notifier\Bridge\Discord;

use Symfony\Component\Notifier\Bridge\Discord\Embeds\DiscordEmbedInterface;
use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Message\MessageOptionsInterface;

/**
 * @author Karoly Gossler <connor@connor.hu>
 * @author Tomas Norkūnas <norkunas.tom@gmail.com>
 */
final class DiscordOptions implements MessageOptionsInterface
{
    public function __construct(
        private array $options = [],
    ) {
    }

    public function toArray(): array
    {
        $options = $this->options;
        unset($options['recipient_id']);

        return $options;
    }

    public function recipient(string $id): static
    {
        $this->options['recipient_id'] = $id;

        return $this;
    }

    public function getRecipientId(): ?string
    {
        return $this->options['recipient_id'] ?? null;
    }

    /**
     * @return $this
     */
    public function username(string $username): static
    {
        $this->options['username'] = $username;

        return $this;
    }

Review comment (suggestion):
if the recipient id is not meant to be part of the array of options, I would suggest storing it as a separate property instead.

Code after:
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\Notifier\Bridge\Discord;

use Symfony\Component\Notifier\Bridge\Discord\Embeds\DiscordEmbedInterface;
use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Message\MessageOptionsInterface;

/**
 * @author Karoly Gossler <connor@connor.hu>
 * @author Tomas Norkūnas <norkunas.tom@gmail.com>
 */
final class DiscordOptions implements MessageOptionsInterface
{
    /**
     * @var non-empty-string|null
     */
    private ?string $recipientId = null;

    public function __construct(
        private array $options = [],
    ) {
    }

    public function toArray(): array
    {
        return $this->options;
    }

    /**
     * @param non-empty-string $id
     *
     * @return $this
     */
    public function recipient(string $id): static
    {
        $this->recipientId = $id;

        return $this;
    }

    /**
     * @return non-empty-string|null
     */
    public function getRecipientId(): ?string
    {
        return $this->recipientId;
    }
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Transform/MapCollection.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\ObjectMapper\Transform;

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

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

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

        foreach ($value as &$v) {
            $v = $this->objectMapper->map($v);
        }

        return $value;
    }
}


Review comment (suggestion):
```suggestion
    public function __construct(private ObjectMapperInterface $objectMapper = new ObjectMapper())
```
this can be removed then. Sorry, I forgot in my suggestion

Code after:
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Transform;

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

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

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

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

        return $values;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Transform/MapCollection.php (PHP)

Code before:
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Transform;

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

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

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

        foreach ($value as &$v) {
            $v = $this->objectMapper->map($v);
        }

        return $value;
    }
}


Review comment (suggestion):
Name "$value" does not say anything about what this parameter means from a semantical POV.
"$sourceProperty" might be a better name. Calling a field/property of a class a "variable" is very much confusing to me.

Code after:
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\ObjectMapper\Transform;

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

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

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

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

        return $values;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Transform/MapCollection.php (PHP)

Code before:
use Symfony\Component\ObjectMapper\ObjectMapper;
use Symfony\Component\ObjectMapper\ObjectMapperInterface;
use Symfony\Component\ObjectMapper\TransformCallableInterface;

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

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

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

        return $values;
    }
}


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

Code after:
use Symfony\Component\ObjectMapper\ObjectMapper;
use Symfony\Component\ObjectMapper\ObjectMapperInterface;
use Symfony\Component\ObjectMapper\TransformCallableInterface;

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

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

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

        return $values;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Transform/MapCollection.php (PHP)

Code before:

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

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

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

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

        return $values;
    }
}


Review comment (suggestion):
```suggestion
        foreach ($value as $k => $v) {
            $values[$k] = $this->objectMapper->map($v);
```

Code after:

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

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

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

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

        return $values;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Tests/Fixtures/TransformCollection/TransformCollectionC.php (PHP)

Code before:
<?php

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

use Symfony\Component\ObjectMapper\Attribute\Map;

#[Map(target: TransformCollectionD::class)]
class TransformCollectionC
{
    public function __construct(
        #[Map(target: 'baz')]
        public string $bar
    ) {}
}


Review comment (suggestion):
```suggestion
        public string $bar,
    ) {
    }
```

Code after:
<?php

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

use Symfony\Component\ObjectMapper\Attribute\Map;

#[Map(target: TransformCollectionD::class)]
class TransformCollectionC
{
    public function __construct(
        #[Map(target: 'baz')]
        public string $bar,
    ) {
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/ObjectMapper/Tests/Fixtures/TransformCollection/TransformCollectionD.php (PHP)

Code before:
<?php

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

class TransformCollectionD
{
    public function __construct(
        public string $baz
    ) {}
}


Review comment (suggestion):
```suggestion
        public string $baz,
    ) {
    }
```

Code after:
<?php

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

class TransformCollectionD
{
    public function __construct(
        public string $baz,
    ) {
    }
}