Skip to content

Commit c24bfb5

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

File tree

3 files changed

+221
-0
lines changed

3 files changed

+221
-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: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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+
* Any child classes which extend public and protected methods will be incompatible with the new method signature.
19+
*/
20+
final class MethodParameterAdded implements MethodBased
21+
{
22+
public function __invoke(ReflectionMethod $fromMethod, ReflectionMethod $toMethod): Changes
23+
{
24+
if ($fromMethod->getDeclaringClass()->isFinal() || $fromMethod->isPrivate()) {
25+
return Changes::empty();
26+
}
27+
28+
$added = array_diff(
29+
array_map(static fn (ReflectionParameter $param) => $param->getName(), $toMethod->getParameters()),
30+
array_map(static fn (ReflectionParameter $param) => $param->getName(), $fromMethod->getParameters()),
31+
);
32+
33+
if ($toMethod->getNumberOfParameters() > $fromMethod->getNumberOfParameters()) {
34+
return Changes::fromList(
35+
...array_map(
36+
static fn (string $paramName): Change => Change::added(
37+
Str\format(
38+
'Parameter %s was added to Method %s() of class %s',
39+
$paramName,
40+
$fromMethod->getName(),
41+
$fromMethod->getDeclaringClass()->getName(),
42+
),
43+
),
44+
$added,
45+
),
46+
);
47+
}
48+
49+
return Changes::empty();
50+
}
51+
}

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

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
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+
/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodParameterAdded */
25+
final class MethodParameterAddedTest extends TestCase
26+
{
27+
private MethodBased $methodCheck;
28+
29+
protected function setUp(): void
30+
{
31+
parent::setUp();
32+
33+
$this->methodCheck = new MethodParameterAdded();
34+
}
35+
36+
public function testWillSkipCheckingPrivateMethods(): void
37+
{
38+
$from = $this->createMock(ReflectionMethod::class);
39+
$to = $this->createMock(ReflectionMethod::class);
40+
41+
$from
42+
->method('isPrivate')
43+
->willReturn(true);
44+
45+
self::assertEquals(Changes::empty(), ($this->methodCheck)($from, $to));
46+
}
47+
48+
public function testWillSkipCheckingMethodsOnFinalClasses(): void
49+
{
50+
$from = $this->createMock(ReflectionMethod::class);
51+
$to = $this->createMock(ReflectionMethod::class);
52+
53+
$from
54+
->method('isPrivate')
55+
->willReturn(false);
56+
57+
self::assertEquals(Changes::empty(), ($this->methodCheck)($from, $to));
58+
}
59+
60+
public function testWillSkipCheckingPrivateMethodsOnFinalClasses(): void
61+
{
62+
$from = $this->createMock(ReflectionMethod::class);
63+
$to = $this->createMock(ReflectionMethod::class);
64+
65+
$from
66+
->method('isPrivate')
67+
->willReturn(true);
68+
69+
$from
70+
->method('isFinal')
71+
->willReturn(true);
72+
73+
self::assertEquals(Changes::empty(), ($this->methodCheck)($from, $to));
74+
}
75+
76+
/**
77+
* @param string[] $expectedMessages
78+
*
79+
* @dataProvider methodsToBeTested
80+
*/
81+
public function testDiffs(
82+
ReflectionMethod $fromMethod,
83+
ReflectionMethod $toMethod,
84+
array $expectedMessages,
85+
): void {
86+
$changes = (new MethodParameterAdded())($fromMethod, $toMethod);
87+
88+
self::assertSame(
89+
$expectedMessages,
90+
array_map(static function (Change $change): string {
91+
return $change->__toString();
92+
}, iterator_to_array($changes)),
93+
);
94+
}
95+
96+
/** @return array<string, array{0: ReflectionMethod, 1: ReflectionMethod, 2: list<string>}> */
97+
public function methodsToBeTested(): array
98+
{
99+
$astLocator = (new BetterReflection())->astLocator();
100+
101+
$fromLocator = new StringSourceLocator(
102+
<<<'PHP'
103+
<?php
104+
105+
class TheClass {
106+
public function addedParameter() {}
107+
public function noParameters() {}
108+
public function twoParams(int $one, int $two) {}
109+
private function privateMethod() {}
110+
public function removedParameter(array $options = []) {}
111+
}
112+
PHP
113+
,
114+
$astLocator,
115+
);
116+
117+
$toLocator = new StringSourceLocator(
118+
<<<'PHP'
119+
<?php
120+
121+
class TheClass {
122+
public function addedParameter(array $options = []) {}
123+
public function noParameters() {}
124+
public function twoParams(int $one, int $two) {}
125+
private function privateMethod(array $options = []) {}
126+
public function removedParameter() {}
127+
}
128+
PHP
129+
,
130+
$astLocator,
131+
);
132+
133+
$fromClassReflector = new DefaultReflector($fromLocator);
134+
$toClassReflector = new DefaultReflector($toLocator);
135+
$fromClass = $fromClassReflector->reflectClass('TheClass');
136+
$toClass = $toClassReflector->reflectClass('TheClass');
137+
138+
$methods = [
139+
'addedParameter' => ['[BC] ADDED: Parameter options was added to Method addedParameter() of class TheClass'],
140+
];
141+
142+
return array_combine(
143+
array_keys($methods),
144+
array_map(
145+
static fn (string $methodName, array $errors): array => [
146+
self::getMethod($fromClass, $methodName),
147+
self::getMethod($toClass, $methodName),
148+
$errors,
149+
],
150+
array_keys($methods),
151+
$methods,
152+
),
153+
);
154+
}
155+
156+
/** @param non-empty-string $name */
157+
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
158+
{
159+
$method = $class->getMethod($name);
160+
161+
assert($method !== null);
162+
163+
return $method;
164+
}
165+
}

0 commit comments

Comments
 (0)