ronantakizawa/codereview-bench huggingface dataset:
Repository: symfony/symfony
File: src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php (PHP)

Code before:
        if (false === $realPath && '-' !== $path && 'Standard input code' !== $path) {
            return self::PATH_TYPE_UNDETERMINED;
        }
        foreach (self::getVendors() as $vendor) {
            if (0 === strpos($realPath, $vendor) && false !== strpbrk(substr($realPath, \strlen($vendor), 1), '/'.\DIRECTORY_SEPARATOR)) {
                return self::PATH_TYPE_VENDOR;
            }
        }

        foreach (self::$internalPaths as $internalPath) {
            if (0 === strpos($realPath, $internalPath)) {
                return self::PATH_TYPE_SELF;
            }
        }

        return self::PATH_TYPE_UNDETERMINED;
    }

    /**
     * @return string
     */
    public function toString()
    {
        $exception = new \Exception($this->message);
        $reflection = new \ReflectionProperty($exception, 'trace');
        if (\PHP_VERSION_ID > 80100) {
            $reflection->setAccessible(true);
        }
        $reflection->setValue($exception, $this->trace);

        return ($this->originatesFromAnObject() ? 'deprecation triggered by '.$this->originatingClass().'::'.$this->originatingMethod().":\n" : '')
            .$this->message."\n"
            ."Stack trace:\n"
            .str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString())."\n";
    }
}


Review comment (suggestion):
```suggestion
        if (\PHP_VERSION_ID < 80100) {
```

Code after:
        if (false === $realPath && '-' !== $path && 'Standard input code' !== $path) {
            return self::PATH_TYPE_UNDETERMINED;
        }
        foreach (self::getVendors() as $vendor) {
            if (0 === strpos($realPath, $vendor) && false !== strpbrk(substr($realPath, \strlen($vendor), 1), '/'.\DIRECTORY_SEPARATOR)) {
                return self::PATH_TYPE_VENDOR;
            }
        }

        foreach (self::$internalPaths as $internalPath) {
            if (0 === strpos($realPath, $internalPath)) {
                return self::PATH_TYPE_SELF;
            }
        }

        return self::PATH_TYPE_UNDETERMINED;
    }

    /**
     * @return string
     */
    public function toString()
    {
        $exception = new \Exception($this->message);
        $reflection = new \ReflectionProperty($exception, 'trace');
        if (\PHP_VERSION_ID < 80100) {
            $reflection->setAccessible(true);
        }
        $reflection->setValue($exception, $this->trace);

        return ($this->originatesFromAnObject() ? 'deprecation triggered by '.$this->originatingClass().'::'.$this->originatingMethod().":\n" : '')
            .$this->message."\n"
            ."Stack trace:\n"
            .str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString())."\n";
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Mailer/Bridge/Mailtrap/Transport/MailtrapApiTransport.php (PHP)

Code before:
            $filename = $headers->getHeaderParameter('Content-Disposition', 'filename');
            $disposition = $headers->getHeaderBody('Content-Disposition');

            $att = [
                'content' => $attachment->bodyToString(),
                'type' => $headers->get('Content-Type')->getBody(),
                'filename' => $filename,
                'disposition' => $disposition,
            ];

            if ('inline' === $disposition) {
                $att['content_id'] = $attachment->hasContentId() ? $attachment->getContentId() : $filename;
            }

            $attachments[] = $att;
        }

        return $attachments;
    }

    private static function encodeEmail(Address $address): array
    {
        return array_filter(['email' => $address->getEncodedAddress(), 'name' => $address->getName()]);
    }

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

    protected function getEndpointPath(): string
    {
        return self::ENDPOINT_PATH;
    }
}


Review comment (suggestion):
This one should be `protected` as it's used in the extended class.

Code after:
            $disposition = $headers->getHeaderBody('Content-Disposition');

            $att = [
                'content' => $attachment->bodyToString(),
                'type' => $headers->get('Content-Type')->getBody(),
                'filename' => $filename,
                'disposition' => $disposition,
            ];

            if ('inline' === $disposition) {
                $att['content_id'] = $attachment->hasContentId() ? $attachment->getContentId() : $filename;
            }

            $attachments[] = $att;
        }

        return $attachments;
    }

    private static function encodeEmail(Address $address): array
    {
        return array_filter(['email' => $address->getEncodedAddress(), 'name' => $address->getName()]);
    }

    protected function getEndpoint(): string
    {
        return ($this->host ?: static::HOST).($this->port ? ':'.$this->port : '').'/api/send';
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Mailer/Bridge/Mailtrap/Transport/MailtrapApiSandboxTransport.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\Mailer\Bridge\Mailtrap\Transport;

use Psr\EventDispatcher\EventDispatcherInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Contracts\HttpClient\HttpClientInterface;

/**
 * @author Kieran Cross
 */
final class MailtrapApiSandboxTransport extends MailtrapApiTransport
{
    protected const HOST = 'sandbox.api.mailtrap.io';

    public function __construct(
        #[\SensitiveParameter] private string $token,
        ?HttpClientInterface $client = null,
        ?EventDispatcherInterface $dispatcher = null,
        ?LoggerInterface $logger = null,
        private ?int $inboxId = null,
    ) {
        if (null === $inboxId) {
            throw new TransportException('Missing required "inboxId" option for Mailtrap sandbox transport.');
        }

        parent::__construct($token, $client, $dispatcher, $logger);
    }

    public function __toString(): string
    {
        return \sprintf('mailtrap+sandbox://%s%s/?inboxId=%u', $this->host ?: static::HOST, $this->port ? ':'.$this->port : '', $this->inboxId);
    }

    protected function getEndpoint(): string
    {
        return parent::getEndpoint().'/'.$this->inboxId;
    }
}


Review comment (security):
```suggestion
        #[\SensitiveParameter] private string $token,
        private int $inboxId,
        ?HttpClientInterface $client = null,
        ?EventDispatcherInterface $dispatcher = null,
        ?LoggerInterface $logger = null,
```

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\Mailer\Bridge\Mailtrap\Transport;

use Psr\EventDispatcher\EventDispatcherInterface;
use Psr\Log\LoggerInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;

/**
 * @author Kieran Cross
 */
final class MailtrapApiSandboxTransport extends MailtrapApiTransport
{
    protected const HOST = 'sandbox.api.mailtrap.io';

    public function __construct(
        #[\SensitiveParameter] private string $token,
        private int $inboxId,
        ?HttpClientInterface $client = null,
        ?EventDispatcherInterface $dispatcher = null,
        ?LoggerInterface $logger = null,
    ) {
        parent::__construct($token, $client, $dispatcher, $logger);
    }

    public function __toString(): string
    {
        return \sprintf('mailtrap+sandbox://%s%s/?inboxId=%u', $this->host ?: static::HOST, $this->port ? ':'.$this->port : '', $this->inboxId);
    }

    protected function getEndpoint(): string
    {
        return parent::getEndpoint().'/'.$this->inboxId;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/Form/ResolvedFormType.php (PHP)

Code before:
    }

    /**
     * @return void
     */
    public function buildView(FormView $view, FormInterface $form, array $options)
    {
        $this->parent?->buildView($view, $form, $options);

        $this->innerType->buildView($view, $form, $options);

        foreach ($this->typeExtensions as $extension) {
            $extension->buildView($view, $form, $options);
        }
    }

    /**
     * @return void
     */
    public function finishView(FormView $view, FormInterface $form, array $options)
    {
        $this->parent?->finishView($view, $form, $options);

        $this->innerType->finishView($view, $form, $options);

        foreach ($this->typeExtensions as $extension) {
            /** @var FormTypeExtensionInterface $extension */
            $extension->finishView($view, $form, $options);
        }
    }

    public function getOptionsResolver(): OptionsResolver
    {
        if (!isset($this->optionsResolver)) {
            if (null !== $this->parent) {
                $this->optionsResolver = clone $this->parent->getOptionsResolver();
            } else {
                $this->optionsResolver = new OptionsResolver();
            }

            $this->innerType->configureOptions($this->optionsResolver);

            foreach ($this->typeExtensions as $extension) {
                $extension->configureOptions($this->optionsResolver);
            }
        }

        return $this->optionsResolver;
    }

    /**

Review comment (suggestion):
This is redundant, `$this->typeExtensions` is already typed correctly.

Code after:
    }

    /**
     * @return void
     */
    public function buildView(FormView $view, FormInterface $form, array $options)
    {
        $this->parent?->buildView($view, $form, $options);

        $this->innerType->buildView($view, $form, $options);

        foreach ($this->typeExtensions as $extension) {
            $extension->buildView($view, $form, $options);
        }
    }

    /**
     * @return void
     */
    public function finishView(FormView $view, FormInterface $form, array $options)
    {
        $this->parent?->finishView($view, $form, $options);

        $this->innerType->finishView($view, $form, $options);

        /** @var FormTypeExtensionInterface $extension */
        foreach ($this->typeExtensions as $extension) {
            $extension->finishView($view, $form, $options);
        }
    }

    public function getOptionsResolver(): OptionsResolver
    {
        if (!isset($this->optionsResolver)) {
            if (null !== $this->parent) {
                $this->optionsResolver = clone $this->parent->getOptionsResolver();
            } else {
                $this->optionsResolver = new OptionsResolver();
            }

            $this->innerType->configureOptions($this->optionsResolver);

            foreach ($this->typeExtensions as $extension) {
                $extension->configureOptions($this->optionsResolver);
            }
        }

        return $this->optionsResolver;
    }

    /**
Repository: symfony/symfony
File: src/Symfony/Component/Routing/Attribute/Route.php (PHP)

Code before:
    public function getMethods(): array
    {
        return $this->methods;
    }

    public function setCondition(?string $condition): void
    {
        $this->condition = $condition;
    }

    public function getCondition(): ?string
    {
        return $this->condition;
    }

    public function setPriority(int $priority): void
    {
        $this->priority = $priority;
    }

    public function getPriority(): ?int
    {
        return $this->priority;
    }

    public function setEnv(string|array|null $env): void
    {
        $this->env = $env;
    }

    public function getEnv(): string|array|null
    {
        return $this->env;
    }

    /**
     * @return (string|DeprecatedAlias)[]
     */
    public function getAliases(): array
    {
        return $this->aliases;
    }

    /**
     * @param string|DeprecatedAlias|(string|DeprecatedAlias)[] $aliases
     */
    public function setAliases(string|DeprecatedAlias|array $aliases): void
    {
        $this->aliases = \is_array($aliases) ? $aliases : [$aliases];
    }
}

Review comment (suggestion):
Extending the parameter type is a breaking change.
Instead, I suggest adding new methods `getEnvs(): array` and `setEnvs(array $env)`.

For the implementation of the existing `setEnv()` and `getEnv()`, you can read the 1st value of the array property and throw an exception if there are several values.

We did something similar in https://github.com/symfony/symfony/commit/caa6d0eb5298aefaa93ab425da5e6bf3acf374a2#diff-a0f563695b61a31b6e2bee8d55d019b52326c245667db02ba878a2a878d78b98

Code after:
    public function setMethods(array|string $methods): void
    {
        $this->methods = (array) $methods;
    }

    public function getMethods(): array
    {
        return $this->methods;
    }

    public function setCondition(?string $condition): void
    {
        $this->condition = $condition;
    }

    public function getCondition(): ?string
    {
        return $this->condition;
    }

    public function setPriority(int $priority): void
    {
        $this->priority = $priority;
    }

    public function getPriority(): ?int
    {
        return $this->priority;
    }

    /**
     * @deprecated since Symfony 7.4, use the {@see setEnvs()} method instead
     */
    public function setEnv(?string $env): void
    {
        trigger_deprecation('symfony/routing', '7.4', 'The "%s()" method is deprecated, use "setEnvs()" instead.', __METHOD__);
        $this->env = (array) $env;
    }

    /**
     * @deprecated since Symfony 7.4, use {@see getEnvs()} method instead
     */
    public function getEnv(): ?string
    {
        trigger_deprecation('symfony/routing', '7.4', 'The "%s()" method is deprecated, use "getEnvs()" instead.', __METHOD__);
        if (!$this->env) {
            return null;
        }
        if (\count($this->env) > 1) {
            throw new LogicException(\sprintf('The "env" property has %d environments. Use "getEnvs()" to get all of them.', \count($this->env)));
        }
Repository: symfony/symfony
File: src/Symfony/Component/HtmlSanitizer/Parser/NativeParser.php (PHP)

Code before:
/**
 * Parser using PHP 8.4's new Dom API.
 */
final class NativeParser
{
    public function __construct()
    {
        if (\PHP_VERSION_ID < 80400) {
            throw new \LogicException(self::class.' requires PHP 8.4 or higher.');
        }
    }

    public function parse(string $html, string $context = 'body'): ?\Dom\Node
    {
        $document = @\Dom\HTMLDocument::createFromString(\sprintf('<!DOCTYPE html><%s>%s</%1$s>', $context, $html), \Dom\HTML_NO_DEFAULT_NS);
        $fragment = $document->createDocumentFragment();

        $context = $document->getElementsByTagName($context)->item(0);
        $count1 = $context->childElementCount;

        $fragment = $document->createDocumentFragment();
        while ($context->firstChild) {
            $fragment->appendChild($context->firstChild);
        }

        $context->innerHTML = $html;
        $count2 = $context->childElementCount;

        return $count2 > $count1 ? $context : ($fragment->hasChildNodes() ? $fragment : null);
    }
}


Review comment (suggestion):
Here we parse a 2nd time with a different algo and we take the results that yields the most nodes.
Without this,`'<tbody>Lorem ipsum</tbody>'` is turned into `'Lorem ipsum'`, which differs from how MastermindsParser works.


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\HtmlSanitizer\Parser;

/**
 * Parser using PHP 8.4's new Dom API.
 */
final class NativeParser implements ParserInterface
{
    public function __construct()
    {
        if (\PHP_VERSION_ID < 80400) {
            throw new \LogicException(self::class.' requires PHP 8.4 or higher.');
        }
    }

    public function parse(string $html, string $context = 'body'): ?\Dom\Node
    {
        $document = @\Dom\HTMLDocument::createFromString(\sprintf('<!DOCTYPE html><%s>%s</%1$s>', $context, $html));
        $element = $document->getElementsByTagName($context)->item(0);

        return $element->hasChildNodes() ? $element : null;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/HtmlSanitizer/Parser/NativeParser.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\HtmlSanitizer\Parser;

/**
 * Parser using PHP 8.4's new Dom API.
 */
final class NativeParser
{
    public function __construct()
    {
        if (\PHP_VERSION_ID < 80400) {
            throw new \LogicException(self::class.' requires PHP 8.4 or higher.');
        }
    }

    public function parse(string $html, string $context = 'body'): ?\Dom\Node
    {
        $document = @\Dom\HTMLDocument::createFromString(\sprintf('<!DOCTYPE html><%s>%s</%1$s>', $context, $html), \Dom\HTML_NO_DEFAULT_NS);
        $fragment = $document->createDocumentFragment();

        $context = $document->getElementsByTagName($context)->item(0);
        $count1 = $context->childElementCount;

        $fragment = $document->createDocumentFragment();
        while ($context->firstChild) {
            $fragment->appendChild($context->firstChild);
        }

        $context->innerHTML = $html;
        $count2 = $context->childElementCount;

        return $count2 > $count1 ? $context : ($fragment->hasChildNodes() ? $fragment : null);
    }
}


Review comment (suggestion):
DOM namespace classes should be imported. php-cs-fixer didn't detect this in fabbot, probably because it runs on PHP 8.3 and doesn't fix the code when the class doesn't exist.


```suggestion
    public function parse(string $html, string $context = 'body'): ?Node
```

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\HtmlSanitizer\Parser;

/**
 * Parser using PHP 8.4's new Dom API.
 */
final class NativeParser implements ParserInterface
{
    public function __construct()
    {
        if (\PHP_VERSION_ID < 80400) {
            throw new \LogicException(self::class.' requires PHP 8.4 or higher.');
        }
    }

    public function parse(string $html, string $context = 'body'): ?\Dom\Node
    {
        $document = @\Dom\HTMLDocument::createFromString(\sprintf('<!DOCTYPE html><%s>%s</%1$s>', $context, $html));
        $element = $document->getElementsByTagName($context)->item(0);

        return $element->hasChildNodes() ? $element : null;
    }
}
Repository: symfony/symfony
File: src/Symfony/Component/HtmlSanitizer/Parser/NativeParser.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\HtmlSanitizer\Parser;

/**
 * Parser using PHP 8.4's new Dom API.
 */
final class NativeParser
{
    public function __construct()
    {
        if (\PHP_VERSION_ID < 80400) {
            throw new \LogicException(self::class.' requires PHP 8.4 or higher.');
        }
    }

    public function parse(string $html, string $context = 'body'): ?\Dom\Node
    {
        $document = @\Dom\HTMLDocument::createFromString(\sprintf('<!DOCTYPE html><%s>%s</%1$s>', $context, $html), \Dom\HTML_NO_DEFAULT_NS);
        $fragment = $document->createDocumentFragment();

        $context = $document->getElementsByTagName($context)->item(0);
        $count1 = $context->childElementCount;

        $fragment = $document->createDocumentFragment();
        while ($context->firstChild) {
            $fragment->appendChild($context->firstChild);
        }

        $context->innerHTML = $html;
        $count2 = $context->childElementCount;

        return $count2 > $count1 ? $context : ($fragment->hasChildNodes() ? $fragment : null);

Review comment (security):
The class is `final` without interface, which means you cannot use it as a type. It's OK since you changed the parameter type in `HtmlSanitizer` constructor to accept a closure. It would still be more conventional to have an interface, even if it's a new one: it would provide more type safety.

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\HtmlSanitizer\Parser;

/**
 * Parser using PHP 8.4's new Dom API.
 */
final class NativeParser implements ParserInterface
{
    public function __construct()
    {
        if (\PHP_VERSION_ID < 80400) {
            throw new \LogicException(self::class.' requires PHP 8.4 or higher.');
        }
    }

    public function parse(string $html, string $context = 'body'): ?\Dom\Node
    {
        $document = @\Dom\HTMLDocument::createFromString(\sprintf('<!DOCTYPE html><%s>%s</%1$s>', $context, $html));
        $element = $document->getElementsByTagName($context)->item(0);

        return $element->hasChildNodes() ? $element : null;
    }
}