Skip to content

Commit b142ee6

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

File tree

3 files changed

+230
-0
lines changed

3 files changed

+230
-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: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
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+
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+
}

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

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
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 addedTwoParameters() {}
108+
public function addedAnotherParameter(int $one) {}
109+
public function noParameters() {}
110+
public function twoParams(int $one, int $two) {}
111+
private function privateMethod() {}
112+
public function removedParameter(int $one, array $options = []) {}
113+
}
114+
PHP
115+
,
116+
$astLocator,
117+
);
118+
119+
$toLocator = new StringSourceLocator(
120+
<<<'PHP'
121+
<?php
122+
123+
class TheClass {
124+
public function addedParameter(array $options = []) {}
125+
public function addedTwoParameters(int $one, array $options = []) {}
126+
public function addedAnotherParameter(int $one, array $options = []) {}
127+
public function noParameters() {}
128+
public function twoParams(int $one, int $two) {}
129+
private function privateMethod(array $options = []) {}
130+
public function removedParameter(int $one) {}
131+
}
132+
PHP
133+
,
134+
$astLocator,
135+
);
136+
137+
$fromClassReflector = new DefaultReflector($fromLocator);
138+
$toClassReflector = new DefaultReflector($toLocator);
139+
$fromClass = $fromClassReflector->reflectClass('TheClass');
140+
$toClass = $toClassReflector->reflectClass('TheClass');
141+
142+
$methods = [
143+
'addedParameter' => ['[BC] ADDED: Parameter options was added to Method addedParameter() of class TheClass'],
144+
'addedTwoParameters' => [
145+
'[BC] ADDED: Parameter one was added to Method addedTwoParameters() of class TheClass',
146+
'[BC] ADDED: Parameter options was added to Method addedTwoParameters() of class TheClass',
147+
],
148+
'addedAnotherParameter' => ['[BC] ADDED: Parameter options was added to Method addedAnotherParameter() of class TheClass'],
149+
'noParameters' => [],
150+
'privateMethod' => [],
151+
'removedParameter' => [],
152+
'twoParams' => [],
153+
];
154+
155+
return array_combine(
156+
array_keys($methods),
157+
array_map(
158+
static fn (string $methodName, array $errors): array => [
159+
self::getMethod($fromClass, $methodName),
160+
self::getMethod($toClass, $methodName),
161+
$errors,
162+
],
163+
array_keys($methods),
164+
$methods,
165+
),
166+
);
167+
}
168+
169+
/** @param non-empty-string $name */
170+
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
171+
{
172+
$method = $class->getMethod($name);
173+
174+
assert($method !== null);
175+
176+
return $method;
177+
}
178+
}

0 commit comments

Comments
 (0)