Skip to content

Commit a212fbf

Browse files
AydinHassanAydinHassan
committed
Add check for adding param to method
1 parent 8443e58 commit a212fbf

File tree

3 files changed

+195
-0
lines changed

3 files changed

+195
-0
lines changed

‎bin/roave-backward-compatibility-check.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ static function (string $installationPath) use ($composerIo): Installer {
145145
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
146146
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
147147
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
148+
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodParameterAdded()),
148149
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
149150
new FunctionBased\MultipleChecksOnAFunction(
150151
new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
@@ -167,6 +168,7 @@ static function (string $installationPath) use ($composerIo): Installer {
167168
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
168169
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
169170
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
171+
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodParameterAdded()),
170172
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
171173
new FunctionBased\MultipleChecksOnAFunction(
172174
new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
@@ -214,6 +216,7 @@ static function (string $installationPath) use ($composerIo): Installer {
214216
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodBecameFinal()),
215217
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
216218
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
219+
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodParameterAdded()),
217220
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
218221
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
219222
new FunctionBased\MultipleChecksOnAFunction(
@@ -249,6 +252,7 @@ static function (string $installationPath) use ($composerIo): Installer {
249252
new ClassBased\SkipClassBasedErrors(new ClassBased\MethodChanged(
250253
new MethodBased\MultipleChecksOnAMethod(
251254
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
255+
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodParameterAdded()),
252256
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
253257
new FunctionBased\MultipleChecksOnAFunction(
254258
new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
@@ -293,6 +297,7 @@ static function (string $installationPath) use ($composerIo): Installer {
293297
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
294298
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
295299
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
300+
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodParameterAdded()),
296301
new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
297302
new FunctionBased\MultipleChecksOnAFunction(
298303
new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),

‎src/DetectChanges/BCBreak/MethodBased/MethodParameterAdded.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased;
6+
7+
use Psl\Str;
8+
use Roave\BackwardCompatibility\Change;
9+
use Roave\BackwardCompatibility\Changes;
10+
use Roave\BetterReflection\Reflection\ReflectionMethod;
11+
use Roave\BetterReflection\Reflection\ReflectionParameter;
12+
13+
use function array_diff;
14+
use function array_map;
15+
16+
/**
17+
* Adding a parameter to a method on a non-final class is a BC break
18+
*/
19+
final class MethodParameterAdded implements MethodBased
20+
{
21+
public function __invoke(ReflectionMethod $fromMethod, ReflectionMethod $toMethod): Changes
22+
{
23+
if ($fromMethod->getDeclaringClass()->isFinal() || $fromMethod->isPrivate()) {
24+
return Changes::empty();
25+
}
26+
27+
$added = array_diff(
28+
array_map(static fn (ReflectionParameter $param) => $param->getName(), $toMethod->getParameters()),
29+
array_map(static fn (ReflectionParameter $param) => $param->getName(), $fromMethod->getParameters()),
30+
);
31+
32+
if ($toMethod->getNumberOfParameters() > $fromMethod->getNumberOfParameters()) {
33+
return Changes::fromList(
34+
...array_map(
35+
static fn (string $paramName): Change => Change::added(
36+
Str\format(
37+
'Parameter %s was added to Method %s() of class %s',
38+
$paramName,
39+
$fromMethod->getName(),
40+
$fromMethod->getDeclaringClass()->getName(),
41+
),
42+
),
43+
$added,
44+
),
45+
);
46+
}
47+
48+
return Changes::empty();
49+
}
50+
}

‎test/unit/DetectChanges/BCBreak/MethodBased/MethodParameterAddedTest.php

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace RoaveTest\BackwardCompatibility\DetectChanges\BCBreak\MethodBased;
6+
7+
use PHPUnit\Framework\TestCase;
8+
use Roave\BackwardCompatibility\Change;
9+
use Roave\BackwardCompatibility\Changes;
10+
use Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodBased;
11+
use Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodParameterAdded;
12+
use Roave\BetterReflection\BetterReflection;
13+
use Roave\BetterReflection\Reflection\ReflectionClass;
14+
use Roave\BetterReflection\Reflection\ReflectionMethod;
15+
use Roave\BetterReflection\Reflector\DefaultReflector;
16+
use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;
17+
18+
use function array_combine;
19+
use function array_keys;
20+
use function array_map;
21+
use function assert;
22+
use function iterator_to_array;
23+
24+
final class MethodParameterAddedTest extends TestCase
25+
{
26+
private MethodBased $methodCheck;
27+
28+
protected function setUp(): void
29+
{
30+
parent::setUp();
31+
32+
$this->methodCheck = new MethodParameterAdded();
33+
}
34+
35+
public function testWillSkipCheckingPrivateMethods(): void
36+
{
37+
$from = $this->createMock(ReflectionMethod::class);
38+
$to = $this->createMock(ReflectionMethod::class);
39+
40+
$from
41+
->method('isPrivate')
42+
->willReturn(true);
43+
44+
self::assertEquals(Changes::empty(), ($this->methodCheck)($from, $to));
45+
}
46+
47+
public function testWillSkipCheckingMethodsOnFinalClasses(): void
48+
{
49+
$from = $this->createMock(ReflectionMethod::class);
50+
$to = $this->createMock(ReflectionMethod::class);
51+
52+
$from
53+
->method('isFinal')
54+
->willReturn(true);
55+
56+
self::assertEquals(Changes::empty(), ($this->methodCheck)($from, $to));
57+
}
58+
59+
/**
60+
* @param string[] $expectedMessages
61+
*
62+
* @dataProvider methodsToBeTested
63+
*/
64+
public function testDiffs(
65+
ReflectionMethod $fromMethod,
66+
ReflectionMethod $toMethod,
67+
array $expectedMessages,
68+
): void {
69+
$changes = (new MethodParameterAdded())($fromMethod, $toMethod);
70+
71+
self::assertSame(
72+
$expectedMessages,
73+
array_map(static function (Change $change): string {
74+
return $change->__toString();
75+
}, iterator_to_array($changes)),
76+
);
77+
}
78+
79+
/** @return array<string, array{0: ReflectionMethod, 1: ReflectionMethod, 2: list<string>}> */
80+
public function methodsToBeTested(): array
81+
{
82+
$astLocator = (new BetterReflection())->astLocator();
83+
84+
$fromLocator = new StringSourceLocator(
85+
<<<'PHP'
86+
<?php
87+
88+
class TheClass {
89+
public function addedParameter() {}
90+
}
91+
PHP
92+
,
93+
$astLocator,
94+
);
95+
96+
$toLocator = new StringSourceLocator(
97+
<<<'PHP'
98+
<?php
99+
100+
class TheClass {
101+
public function addedParameter(array $options = []) {}
102+
}
103+
PHP
104+
,
105+
$astLocator,
106+
);
107+
108+
$fromClassReflector = new DefaultReflector($fromLocator);
109+
$toClassReflector = new DefaultReflector($toLocator);
110+
$fromClass = $fromClassReflector->reflectClass('TheClass');
111+
$toClass = $toClassReflector->reflectClass('TheClass');
112+
113+
$methods = [
114+
'addedParameter' => ['[BC] ADDED: Parameter options was added to Method addedParameter() of class TheClass'],
115+
];
116+
117+
return array_combine(
118+
array_keys($methods),
119+
array_map(
120+
static fn (string $methodName, array $errors): array => [
121+
self::getMethod($fromClass, $methodName),
122+
self::getMethod($toClass, $methodName),
123+
$errors,
124+
],
125+
array_keys($methods),
126+
$methods,
127+
),
128+
);
129+
}
130+
131+
/** @param non-empty-string $name */
132+
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
133+
{
134+
$method = $class->getMethod($name);
135+
136+
assert($method !== null);
137+
138+
return $method;
139+
}
140+
}

0 commit comments

Comments
 (0)