Detect BC breaks due to parameter name changes, in line with PHP 8.0 named arguments requirements

bin/roave-backward-compatibility-check.php

@@ -149,7 +149,8 @@ static function (string $installationPath) use ($composerIo): Installer {
                                                     new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
                                                     new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
                                                     new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
-                                                    new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged())
+                                                    new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
+                                                    new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
                                                 )
                                             ))
                                         )
@@ -170,7 +171,8 @@ static function (string $installationPath) use ($composerIo): Installer {
                                                     new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
                                                     new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
                                                     new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
-                                                    new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged())
+                                                    new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
+                                                    new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
                                                 )
                                             ))
                                         )
@@ -251,7 +253,8 @@ static function (string $installationPath) use ($composerIo): Installer {
                                         new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
                                         new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
                                         new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
-                                        new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged())
+                                        new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
+                                        new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
                                     )
                                 ))
                             )
@@ -294,7 +297,8 @@ static function (string $installationPath) use ($composerIo): Installer {
                                             new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
                                             new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
                                             new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
-                                            new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged())
+                                            new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
+                                            new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
                                         )
                                     ))
                                 )

src/DetectChanges/BCBreak/FunctionBased/ParameterNameChanged.php

@@ -0,0 +1,118 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Roave\BackwardCompatibility\DetectChanges\BCBreak\FunctionBased;
+
+use Roave\BackwardCompatibility\Change;
+use Roave\BackwardCompatibility\Changes;
+use Roave\BackwardCompatibility\Formatter\FunctionName;
+use Roave\BetterReflection\Reflection\ReflectionFunction;
+use Roave\BetterReflection\Reflection\ReflectionMethod;
+use Roave\BetterReflection\Reflection\ReflectionParameter;
+
+use function array_intersect_key;
+use function Safe\sprintf;
+use function str_contains;
+
+/**
+ * Detects a change in a parameter name, which must now be considered a BC break as of PHP 8 (specifically, since the
+ * named parameters feature was introduced). This check can be prevented with the @no-named-arguments annotation.
+ *
+ * This is mostly useful for methods, where a change in a parameter name is not allowed in
+ * inheritance/interface scenarios, except if annotated with `no-named-arguments`
+ */
+final class ParameterNameChanged implements FunctionBased
+{
+    private const NO_NAMED_ARGUMENTS_ANNOTATION = '@no-named-arguments';
+
+    private FunctionName $formatFunction;
+
+    public function __construct()
+    {
+        $this->formatFunction = new FunctionName();
+    }
+
+    public function __invoke(
+        ReflectionMethod|ReflectionFunction $fromFunction,
+        ReflectionMethod|ReflectionFunction $toFunction
+    ): Changes {
+        $fromHadNoNamedArgumentsAnnotation = $this->methodHasNoNamedArgumentsAnnotation($fromFunction);
+        $toHasNoNamedArgumentsAnnotation   = $this->methodHasNoNamedArgumentsAnnotation($toFunction);
+
+        if ($fromHadNoNamedArgumentsAnnotation && ! $toHasNoNamedArgumentsAnnotation) {
+            return Changes::fromList(
+                Change::removed(
+                    sprintf(
+                        'The %s annotation was removed from %s',
+                        self::NO_NAMED_ARGUMENTS_ANNOTATION,
+                        $this->formatFunction->__invoke($fromFunction),
+                    ),
+                    true
+                )
+            );
+        }
+
+        if (! $fromHadNoNamedArgumentsAnnotation && $toHasNoNamedArgumentsAnnotation) {
+            return Changes::fromList(
+                Change::added(
+                    sprintf(
+                        'The %s annotation was added from %s',
+                        self::NO_NAMED_ARGUMENTS_ANNOTATION,
+                        $this->formatFunction->__invoke($fromFunction),
+                    ),
+                    true
+                )
+            );
+        }
+
+        if ($toHasNoNamedArgumentsAnnotation) {
+            return Changes::empty();
+        }
+
+        return Changes::fromIterator($this->checkSymbols(
+            $fromFunction->getParameters(),
+            $toFunction->getParameters()
+        ));
+    }
+
+    /**
+     * @param list<ReflectionParameter> $from
+     * @param list<ReflectionParameter> $to
+     *
+     * @return iterable<int, Change>
+     */
+    private function checkSymbols(array $from, array $to): iterable
+    {
+        foreach (array_intersect_key($from, $to) as $index => $commonParameter) {
+            yield from $this->compareParameter($commonParameter, $to[$index]);
+        }
+    }
+
+    /** @return iterable<int, Change> */
+    private function compareParameter(ReflectionParameter $fromParameter, ReflectionParameter $toParameter): iterable
+    {
+        $fromName = $fromParameter->getName();
+        $toName   = $toParameter->getName();
+
+        if ($fromName === $toName) {
+            return;
+        }
+
+        yield Change::changed(
+            sprintf(
+                'Parameter %d of %s changed name from %s to %s',
+                $fromParameter->getPosition(),
+                $this->formatFunction->__invoke($fromParameter->getDeclaringFunction()),
+                $fromName,
+                $toName
+            ),
+            true
+        );
+    }
+
+    private function methodHasNoNamedArgumentsAnnotation(ReflectionMethod|ReflectionFunction $function): bool
+    {
+        return str_contains($function->getDocComment(), self::NO_NAMED_ARGUMENTS_ANNOTATION);
+    }
+}

test/unit/DetectChanges/BCBreak/FunctionBased/ParameterNameChangedTest.php

@@ -0,0 +1,123 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RoaveTest\BackwardCompatibility\DetectChanges\BCBreak\FunctionBased;
+
+use PHPUnit\Framework\TestCase;
+use Roave\BackwardCompatibility\Change;
+use Roave\BackwardCompatibility\DetectChanges\BCBreak\FunctionBased\ParameterNameChanged;
+use Roave\BetterReflection\BetterReflection;
+use Roave\BetterReflection\Reflection\ReflectionFunction;
+use Roave\BetterReflection\Reflection\ReflectionMethod;
+use Roave\BetterReflection\Reflector\DefaultReflector;
+use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;
+
+use function array_combine;
+use function array_keys;
+use function array_map;
+use function iterator_to_array;
+
+/**
+ * @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\FunctionBased\ParameterNameChanged
+ */
+final class ParameterNameChangedTest extends TestCase
+{
+    /**
+     * @param string[] $expectedMessages
+     *
+     * @dataProvider functionsToBeTested
+     */
+    public function testDiffs(
+        ReflectionMethod|ReflectionFunction $fromFunction,
+        ReflectionMethod|ReflectionFunction $toFunction,
+        array $expectedMessages
+    ): void {
+        $changes = (new ParameterNameChanged())
+            ->__invoke($fromFunction, $toFunction);
+
+        self::assertSame(
+            $expectedMessages,
+            array_map(static function (Change $change): string {
+                return $change->__toString();
+            }, iterator_to_array($changes))
+        );
+    }
+
+    /**
+     * @return array<string, array{
+     *     0: ReflectionMethod|ReflectionFunction,
+     *     1: ReflectionMethod|ReflectionFunction,
+     *     2: list<string>
+     * }>
+     */
+    public function functionsToBeTested(): array
+    {
+        $astLocator = (new BetterReflection())->astLocator();
+
+        $fromLocator = new StringSourceLocator(
+            <<<'PHP'
+<?php
+
+function changed(int $a, int $b) {}
+function untouched(int $a, int $b) {}
+/** @no-named-arguments */
+function changedButAnnotated(int $a, int $b) {}
+/** @no-named-arguments */
+function removingAnnotation(int $a, int $b) {}
+function addingAnnotation(int $a, int $b) {}
+function addedArgumentsShouldNotBeDetected($a, $b) {}
+function removedArgumentsShouldNotBeDetected($a, $b, $c) {}
+PHP
+            ,
+            $astLocator
+        );
+
+        $toLocator = new StringSourceLocator(
+            <<<'PHP'
+<?php
+
+function changed(int $c, int $d) {}
+function untouched(int $a, int $b) {}
+/** @no-named-arguments */
+function changedButAnnotated(int $c, int $d) {}
+function removingAnnotation(int $a, int $b) {}
+/** @no-named-arguments */
+function addingAnnotation(int $a, int $b) {}
+function addedArgumentsShouldNotBeDetected($a, $b, $c) {}
+function removedArgumentsShouldNotBeDetected($a, $b) {}
+PHP
+            ,
+            $astLocator
+        );
+
+        $fromReflector = new DefaultReflector($fromLocator);
+        $toReflector   = new DefaultReflector($toLocator);
+
+        $functions = [
+            'changed'                             => [
+                '[BC] CHANGED: Parameter 0 of changed() changed name from a to c',
+                '[BC] CHANGED: Parameter 1 of changed() changed name from b to d',
+            ],
+            'untouched'                           => [],
+            'changedButAnnotated'                 => [],
+            'removingAnnotation'                  => ['[BC] REMOVED: The @no-named-arguments annotation was removed from removingAnnotation()'],
+            'addingAnnotation'                    => ['[BC] ADDED: The @no-named-arguments annotation was added from addingAnnotation()'],
+            'addedArgumentsShouldNotBeDetected'   => [],
+            'removedArgumentsShouldNotBeDetected' => [],
+        ];
+
+        return array_combine(
+            array_keys($functions),
+            array_map(
+                static fn (string $function, array $errorMessages): array => [
+                    $fromReflector->reflectFunction($function),
+                    $toReflector->reflectFunction($function),
+                    $errorMessages,
+                ],
+                array_keys($functions),
+                $functions
+            )
+        );
+    }
+}