ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Component/HttpKernel/Attribute/Serialize.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\Attribute;

/**
 * Controller tag to serialize response.
 *
 * @author Konstantin Myakshin <molodchick@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_METHOD)]
final class Serialize
{
    public function __construct(
        public readonly int $code = 200,
        /**
         * @var array<string, string>
         */
        public readonly array $headers = [],
        /**
         * @var array<string, string>
         */
        public readonly array $serializationContext = [],
    ) {
    }
}


Review comment (suggestion):
Shouldn't this `#[Serialize]` attribute allow to pass the groups to be used during the serialization process?

Also, the `SerializerInterface` and its `serialize` method allows us to pass the preferred format (JSON, XML, ...) so making it an argument (even with a predefined value) makes sense to me 🤔 

==== EDIT ====

By the way: with "passing groups" I meant that they seem to be a relevant setting so maybe it's a good idea to add them with a shortcut (instead of passing them via context), but that's just a secondary matter - the primary concern here is the missing format:

```php
$data = $this->serializer->serialize(
            data: $output,
            format: 'json',
            context: ['groups' => ['Group1', 'Group2']]
        );
```

so it could be like this:

```php
#[Serialize(code: 201, format: 'json', headers: [], serializationContext: ['groups' => ['Group1', 'Group2']])]
```

or even

```php
#[Serialize(code: 201, format: 'json', headers: [], groups: ['Group1', 'Group2'])]
```

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

/**
 * Controller tag to serialize response.
 *
 * @author Konstantin Myakshin <molodchick@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_METHOD)]
final class Serialize
{
    /**
     * @param int                  $code    The HTTP status code (200 "OK" by default)
     * @param array<string, mixed> $headers Extra headers to set on the response
     * @param array<string, mixed> $context The serialization context passed to the serializer
     */
    public function __construct(
        public readonly int $code = 200,
        public readonly array $headers = [],
        public readonly array $context = [],
    ) {
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/HttpKernel/Attribute/Serialize.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\HttpKernel\Attribute;

/**
 * Controller tag to serialize response.
 *
 * @author Konstantin Myakshin <molodchick@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_METHOD)]
final class Serialize
{
    /**
     * @param int                  $code                 The HTTP status code (200 "OK" by default)
     * @param array<string, mixed> $headers              Extra headers to set on the response
     * @param array<string, mixed> $serializationContext The serialization context passed to the serializer
     */
    public function __construct(
        public readonly int $code = 200,
        public readonly array $headers = [],
        public readonly array $serializationContext = [],
    ) {
    }
}


Review comment (refactor):
maybe simplify?
```suggestion
        public readonly array $context = [],
```

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

/**
 * Controller tag to serialize response.
 *
 * @author Konstantin Myakshin <molodchick@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_METHOD)]
final class Serialize
{
    /**
     * @param int                  $code    The HTTP status code (200 "OK" by default)
     * @param array<string, mixed> $headers Extra headers to set on the response
     * @param array<string, mixed> $context The serialization context passed to the serializer
     */
    public function __construct(
        public readonly int $code = 200,
        public readonly array $headers = [],
        public readonly array $context = [],
    ) {
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Scheduler/Schedule.php (PHP)

Code before:

        return $this;
    }

    public function getState(): ?CacheInterface
    {
        return $this->state;
    }

    /**
     * @return array<RecurringMessage>
     */
    public function getRecurringMessages(): array
    {
        return array_values($this->messages);
    }

    /**
     * @return $this
     */
    public function getSchedule(): static
    {
        return $this;
    }

    public function restart(bool $restart = null): bool
    {
        return $this->restart = $restart ?? $this->restart;
    }
}


Review comment (refactor):
For more clarity, I would split this method: `setRestart(bool)` and `shouldRestart(): bool`

Code after:
     * @return $this
     */
    public function stateful(CacheInterface $state): static
    {
        $this->state = $state;

        return $this;
    }

    public function getState(): ?CacheInterface
    {
        return $this->state;
    }

    /**
     * @return array<RecurringMessage>
     */
    public function getRecurringMessages(): array
    {
        return array_values($this->messages);
    }

    /**
     * @return $this
     */
    public function getSchedule(): static
    {
        return $this;
    }

    public function shouldRestart(): bool
    {
        return $this->shouldRestart;
    }

    public function setRestart(bool $shouldRestart): bool
    {
        return $this->shouldRestart = $shouldRestart;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Scheduler/Schedule.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\Scheduler;

use Symfony\Component\Lock\LockInterface;
use Symfony\Component\Scheduler\Exception\LogicException;
use Symfony\Contracts\Cache\CacheInterface;

final class Schedule implements ScheduleProviderInterface
{
    /** @var array<string,RecurringMessage> */
    private array $messages = [];
    private ?LockInterface $lock = null;
    private ?CacheInterface $state = null;

    private bool $restart = false;

    public static function with(RecurringMessage $message, RecurringMessage ...$messages): static
    {
        return static::doAdd(new self(), $message, ...$messages);
    }

    /**
     * @return $this
     */
    public function add(RecurringMessage $message, RecurringMessage ...$messages): static
    {
        $this->restart(true);

        return static::doAdd($this, $message, ...$messages);
    }

    private static function doAdd(self $schedule, RecurringMessage $message, RecurringMessage ...$messages): static
    {
        foreach ([$message, ...$messages] as $m) {
            if (isset($schedule->messages[$m->getId()])) {
                throw new LogicException('Duplicated schedule message.');
            }

            $schedule->messages[$m->getId()] = $m;
        }

Review comment (suggestion):
```suggestion
    private bool $shouldRestart = false;
```

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

use Symfony\Component\Lock\LockInterface;
use Symfony\Component\Scheduler\Exception\LogicException;
use Symfony\Contracts\Cache\CacheInterface;

final class Schedule implements ScheduleProviderInterface
{
    /** @var array<string,RecurringMessage> */
    private array $messages = [];
    private ?LockInterface $lock = null;
    private ?CacheInterface $state = null;
    private bool $shouldRestart = false;

    public static function with(RecurringMessage $message, RecurringMessage ...$messages): static
    {
        return static::doAdd(new self(), $message, ...$messages);
    }

    /**
     * @return $this
     */
    public function add(RecurringMessage $message, RecurringMessage ...$messages): static
    {
        $this->setRestart(true);

        return static::doAdd($this, $message, ...$messages);
    }

    private static function doAdd(self $schedule, RecurringMessage $message, RecurringMessage ...$messages): static
    {
        foreach ([$message, ...$messages] as $m) {
            if (isset($schedule->messages[$m->getId()])) {
                throw new LogicException('Duplicated schedule message.');
            }

            $schedule->messages[$m->getId()] = $m;
        }
Repository: symfony/symfony
File: src/Symfony/Component/Scheduler/Schedule.php (PHP)

Code before:
    /**
     * @return $this
     */
    public function add(RecurringMessage $message, RecurringMessage ...$messages): static
    {
        $this->restart(true);

        return static::doAdd($this, $message, ...$messages);
    }

    private static function doAdd(self $schedule, RecurringMessage $message, RecurringMessage ...$messages): static
    {
        foreach ([$message, ...$messages] as $m) {
            if (isset($schedule->messages[$m->getId()])) {
                throw new LogicException('Duplicated schedule message.');
            }

            $schedule->messages[$m->getId()] = $m;
        }

        return $schedule;
    }

    public function remove(RecurringMessage $message): static
    {
        unset($this->messages[$message->getId()]);
        $this->restart(true);

        return $this;
    }

    public function clear(): static
    {
        $this->messages = [];
        $this->restart(true);

        return $this;
    }

    /**
     * @return $this
     */
    public function lock(LockInterface $lock): static
    {
        $this->lock = $lock;

        return $this;
    }

    public function getLock(): ?LockInterface
    {

Review comment (suggestion):
```suggestion
    /**
     * @return $this
     */
    public function remove(RecurringMessage $message): static
```

Code after:
     * @return $this
     */
    public function add(RecurringMessage $message, RecurringMessage ...$messages): static
    {
        $this->setRestart(true);

        return static::doAdd($this, $message, ...$messages);
    }

    private static function doAdd(self $schedule, RecurringMessage $message, RecurringMessage ...$messages): static
    {
        foreach ([$message, ...$messages] as $m) {
            if (isset($schedule->messages[$m->getId()])) {
                throw new LogicException('Duplicated schedule message.');
            }

            $schedule->messages[$m->getId()] = $m;
        }

        return $schedule;
    }

    /**
     * @return $this
     */
    public function remove(RecurringMessage $message): static
    {
        unset($this->messages[$message->getId()]);
        $this->setRestart(true);

        return $this;
    }

    /**
     * @return $this
     */
    public function clear(): static
    {
        $this->messages = [];
        $this->setRestart(true);

        return $this;
    }

    /**
     * @return $this
     */
    public function lock(LockInterface $lock): static
    {
        $this->lock = $lock;
Repository: symfony/symfony
File: src/Symfony/Component/Security/Core/Role/RoleHierarchy.php (PHP)

Code before:
    }

    public function getReachableRoleNames(array $roles): array
    {
        $reachableRoles = $roles;

        foreach ($roles as $role) {
            if (!isset($this->map[$role])) {
                continue;
            }

            foreach ($this->map[$role] as $r) {
                $reachableRoles[] = $r;
            }
        }

        return array_values(array_unique($reachableRoles));
    }

    public function getEncompassingRoleNames(array $roles): array
    {
        $encompassingRoles = $roles;

        foreach ($roles as $role) {
            foreach ($this->map as $parent => $children) {
                if (\in_array($role, $children)) {
                    $encompassingRoles[] = $parent;
                }
            }
        }

        return array_values(array_unique($encompassingRoles));
    }

    protected function buildRoleMap(): void
    {
        $this->map = [];
        foreach ($this->hierarchy as $main => $roles) {
            $this->map[$main] = $roles;
            $visited = [];
            $additionalRoles = $roles;
            while ($role = array_shift($additionalRoles)) {
                if (!isset($this->hierarchy[$role])) {
                    continue;
                }

                $visited[] = $role;

                foreach ($this->hierarchy[$role] as $roleToAdd) {
                    $this->map[$main][] = $roleToAdd;
                }

Review comment (suggestion):
```suggestion
                if (\in_array($role, $children, true)) {
```

Code after:

        foreach ($roles as $role) {
            $reachableRoles[$role] = $role;

            if (!isset($this->map[$role])) {
                continue;
            }

            foreach ($this->map[$role] as $r) {
                $reachableRoles[$r] = $r;
            }
        }

        return $reachableRoles;
    }

    /**
     * @param string[] $roles
     *
     * @return array<string, string>
     */
    public function getParentRoleNames(array $roles): array
    {
        $parentRoles = [];

        foreach ($roles as $role) {
            $parentRoles[$role] = $role;

            foreach ($this->map as $parent => $children) {
                if (isset($children[$role])) {
                    $parentRoles[$parent] = $parent;
                }
            }
        }

        return $parentRoles;
    }

    protected function buildRoleMap(): void
    {
        $this->map = [];
        foreach ($this->hierarchy as $main => $roles) {
            $map = [];
            $visited = [];
            $additionalRoles = $roles;
            while (null !== $role = key($additionalRoles)) {
                $role = $additionalRoles[$role];
                $map[$role] = $role;

                if (!isset($this->hierarchy[$role])) {
                    next($additionalRoles);
Repository: symfony/symfony
File: src/Symfony/Component/Form/Util/FormUtil.php (PHP)

Code before:
        }

        if (\is_array($paramsValue)) {
            return $paramsValue; // params has the precedence
        }

        if (self::isFileUpload($filesValue)) {
            return $filesValue; // if the array is a file upload field, it has the precedence
        }

        return $paramsValue;
    }

    private static function getValueAtPath(array $params, array $path)
    {
        foreach ($path as $key) {
            if (null === $params = $params[$key] ?? null) {
                return null;
            }
        }

        return $params;
    }

    /**
     * @param UploadedFile|array $value
     */
    private static function isFileUpload($value): bool
    {
        if ($value instanceof UploadedFile) {
            return true;
        }

        if (!\is_array($value) || !\in_array(\count($value), [5, 6], true)) {
            return false;
        }

        if (\array_key_exists('full_path', $value)) {
            unset($value['full_path']);
        }

        $keys = array_keys($value);
        sort($keys);

        return ['error', 'name', 'size', 'tmp_name', 'type'] === $keys;
    }

    private static function doesNotContainNonFileUploadArray(array $array): bool
    {
        foreach ($array as $value) {
            if (\is_array($value) && !self::isFileUpload($value)) {

Review comment (bug):
this type is wrong. The value can be anything (otherwise, the `is_array` check on line 122 would be totally useless btw)

Code after:
        }

        return $params;
    }

    private static function isFileUpload(mixed $value): bool
    {
        if ($value instanceof UploadedFile) {
            return true;
        }

        if (!\is_array($value) || !\in_array(\count($value), [5, 6], true)) {
            return false;
        }

        if (\array_key_exists('full_path', $value)) {
            unset($value['full_path']);
        }

        $keys = array_keys($value);
        sort($keys);

        return ['error', 'name', 'size', 'tmp_name', 'type'] === $keys;
    }

    private static function doesNotContainNonFileUploadArray(array $array): bool
    {
        foreach ($array as $value) {
            if (\is_array($value) && !self::isFileUpload($value)) {
                return false;
            }
        }

        return true;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Validator/Constraints/Xml.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\Constraints;

use Symfony\Component\Validator\Attribute\HasNamedArguments;
use Symfony\Component\Validator\Constraint;

/**
 * Validates that a value is a valid xml format.
 *
 * @author Mokhtar Tlili <tlili.mokhtar@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
class Xml extends Constraint
{
    public const INVALID_XML_ERROR = '0355230a-97b8-49da-b8cd-985bf3345bcf';

    protected const ERROR_NAMES = [
        self::INVALID_XML_ERROR => 'INVALID_XML_ERROR',
    ];

    #[HasNamedArguments]
    public function __construct(
        public string $message = 'This value is not valid XML.',
        ?array $groups = null,
        mixed $payload = null,
    ) {
        parent::__construct(null, $groups, $payload);
    }
}


Review comment (suggestion):
```suggestion
 * Validates that a value is a valid XML string.
```

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

use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\Exception\InvalidArgumentException;

/**
 * Validates that a value is a valid XML string.
 *
 * @author Mokhtar Tlili <tlili.mokhtar@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
class Xml extends Constraint
{
    public const INVALID_XML_ERROR = '0355230a-97b8-49da-b8cd-985bf3345bcf';

    protected const ERROR_NAMES = [
        self::INVALID_XML_ERROR => 'INVALID_XML_ERROR',
    ];

    public function __construct(
        public string $formatMessage = 'This value is not valid XML.',
        public string $schemaMessage = 'This value is not conform to the expected XSD schema.',
        public ?string $schemaPath = null,
        public int $schemaFlags = 0,
        ?array $groups = null,
        mixed $payload = null,
    ) {
        parent::__construct(null, $groups, $payload);

        if (null !== $this->schemaPath && !is_readable($this->schemaPath)) {
            throw new InvalidArgumentException(\sprintf('The XSD schema file "%s" does not exist or is unreadable.', $this->schemaPath));
        }
Repository: symfony/symfony
File: src/Symfony/Component/Validator/Constraints/Xml.php (PHP)

Code before:
/**
 * Validates that a value is a valid XML string.
 *
 * @author Mokhtar Tlili <tlili.mokhtar@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
class Xml extends Constraint
{
    public const INVALID_XML_ERROR = '0355230a-97b8-49da-b8cd-985bf3345bcf';

    protected const ERROR_NAMES = [
        self::INVALID_XML_ERROR => 'INVALID_XML_ERROR',
    ];

    #[HasNamedArguments]
    public function __construct(
        public string $formatMessage = 'This value is not valid XML.',
        public string $schemaMessage = 'This value is not conform to the XML Schema.',
        public ?string $schemaPath = null,
        public int $schemaFlags = 0,
        ?array $groups = null,
        mixed $payload = null,
    ) {
        parent::__construct(null, $groups, $payload);

        if (null !== $this->schemaPath && !file_exists($this->schemaPath)) {
            throw new InvalidArgumentException(sprintf('The XSD schema file "%s" does not exist.', $this->schemaPath));
        }
    }
}


Review comment (suggestion):
Let's use `is_readable()` to ensure that the file exists and can actually be read:

```suggestion
        if (null !== $this->schemaPath && !is_readable($this->schemaPath)) {
```

Code after:
 * Validates that a value is a valid XML string.
 *
 * @author Mokhtar Tlili <tlili.mokhtar@gmail.com>
 */
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
class Xml extends Constraint
{
    public const INVALID_XML_ERROR = '0355230a-97b8-49da-b8cd-985bf3345bcf';

    protected const ERROR_NAMES = [
        self::INVALID_XML_ERROR => 'INVALID_XML_ERROR',
    ];

    public function __construct(
        public string $formatMessage = 'This value is not valid XML.',
        public string $schemaMessage = 'This value is not conform to the expected XSD schema.',
        public ?string $schemaPath = null,
        public int $schemaFlags = 0,
        ?array $groups = null,
        mixed $payload = null,
    ) {
        parent::__construct(null, $groups, $payload);

        if (null !== $this->schemaPath && !is_readable($this->schemaPath)) {
            throw new InvalidArgumentException(\sprintf('The XSD schema file "%s" does not exist or is unreadable.', $this->schemaPath));
        }
    }
}