Skip to content

Commit 3087dad

Browse files
tomzxtomzx
committed
Improved function/class method parameters checking.
Different signature typehints will result in a MAJOR. Different signature variables names will result in a PATCH.
1 parent e96abf2 commit 3087dad

File tree

4 files changed

+200
-27
lines changed

4 files changed

+200
-27
lines changed

‎src/PHPSemVerChecker/Registry/Registry.php‎

Lines changed: 78 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -162,17 +162,16 @@ public function compare(Registry $registry)
162162
$paramsAfter = $functionAfter->params;
163163
// Signature
164164

165-
// Argument order is different (type mismatch)
166-
$iterations = min(count($paramsBefore), count($paramsAfter));
167-
for ($i = 0; $i < $iterations; ++$i) {
168-
$paramTypeBefore = is_object($paramsBefore[$i]->type) ? $paramsBefore[$i]->type->toString() : $paramsBefore[$i]->type;
169-
$paramTypeAfter = is_object($paramsAfter[$i]->type) ? $paramsAfter[$i]->type->toString() : $paramsAfter[$i]->type;
170-
// TODO: Allow for contravariance <tom@tomrochette.com>
171-
if ($paramTypeBefore !== $paramTypeAfter) {
172-
$data = new FunctionParameterMismatch($fileBefore, $functionBefore, $fileAfter, $functionAfter);
173-
$appendFunctionDifference(self::MAJOR, $data);
174-
continue 2;
175-
}
165+
if ( ! $this->isSameSignatureTypehint($paramsBefore, $paramsAfter)) {
166+
$data = new FunctionParameterMismatch($fileBefore, $functionBefore, $fileAfter, $functionAfter);
167+
$appendFunctionDifference(self::MAJOR, $data);
168+
continue;
169+
}
170+
171+
if ( ! $this->isSameSignatureVariables($paramsBefore, $paramsAfter)) {
172+
$data = new FunctionParameterMismatch($fileBefore, $functionBefore, $fileAfter, $functionAfter);
173+
$appendFunctionDifference(self::PATCH, $data);
174+
continue;
176175
}
177176

178177
// Different length (considering params with defaults)
@@ -270,18 +269,16 @@ public function compare(Registry $registry)
270269
$paramsAfter = $methodAfter->params;
271270
// Signature
272271

273-
// Argument order is different (type mismatch)
274-
$iterations = min(count($paramsBefore), count($paramsAfter));
275-
for ($i = 0; $i < $iterations; ++$i) {
276-
$paramTypeBefore = is_object($paramsBefore[$i]->type) ? $paramsBefore[$i]->type->toString() : $paramsBefore[$i]->type;
277-
$paramTypeAfter = is_object($paramsAfter[$i]->type) ? $paramsAfter[$i]->type->toString() : $paramsAfter[$i]->type;
278-
// TODO: Allow for contravariance <tom@tomrochette.com>
279-
// TODO: Check that this works properly with aliases <tom@tomrochette.com>
280-
if ($paramTypeBefore !== $paramTypeAfter) {
281-
$data = new ClassMethodParameterMismatch($fileBefore, $classBefore, $methodBefore, $fileAfter, $classAfter, $methodAfter);
282-
$appendClassDifference(self::MAJOR, $data);
283-
continue 2;
284-
}
272+
if ( ! $this->isSameSignatureTypehint($paramsBefore, $paramsAfter)) {
273+
$data = new ClassMethodParameterMismatch($fileBefore, $classBefore, $methodBefore, $fileAfter, $classAfter, $methodAfter);
274+
$appendClassDifference(self::MAJOR, $data);
275+
continue;
276+
}
277+
278+
if ( ! $this->isSameSignatureVariables($paramsBefore, $paramsAfter)) {
279+
$data = new ClassMethodParameterMismatch($fileBefore, $classBefore, $methodBefore, $fileAfter, $classAfter, $methodAfter);
280+
$appendClassDifference(self::PATCH, $data);
281+
continue;
285282
}
286283

287284
// Different length (considering params with defaults)
@@ -317,6 +314,64 @@ public function compare(Registry $registry)
317314
return $differences;
318315
}
319316

317+
/**
318+
* @param array $paramsA
319+
* @param array $paramsB
320+
* @return bool
321+
*/
322+
protected function isSameSignatureTypehint(array $paramsA, array $paramsB)
323+
{
324+
// TODO: Check additional parameters have defaults <tom@tomrochette.com>
325+
$iterations = min(count($paramsA), count($paramsB));
326+
for ($i = 0; $i < $iterations; ++$i) {
327+
// TODO: Allow for contravariance <tom@tomrochette.com>
328+
if ( ! $this->isSameType($paramsA[$i]->type, $paramsB[$i]->type)) {
329+
return false;
330+
}
331+
}
332+
// Only one of these will returns its remaining values, the other returning an empty array
333+
$toCheck = array_slice($paramsA, $iterations) + array_slice($paramsB, $iterations);
334+
// If any additional argument does not have a default value, the signature has changed
335+
foreach ($toCheck as $param) {
336+
if ($param->default === null) {
337+
return false;
338+
}
339+
}
340+
return true;
341+
}
342+
343+
/**
344+
* @param array $paramsA
345+
* @param array $paramsB
346+
* @return bool
347+
*/
348+
protected function isSameSignatureVariables(array $paramsA, array $paramsB)
349+
{
350+
if (count($paramsA) !== count($paramsB)) {
351+
return false;
352+
}
353+
354+
$iterations = min(count($paramsA), count($paramsB));
355+
for ($i = 0; $i < $iterations; ++$i) {
356+
if ( $paramsA[$i]->name != $paramsB[$i]->name) {
357+
return false;
358+
}
359+
}
360+
return true;
361+
}
362+
363+
/**
364+
* @param \PhpParser\Node\Name|string $typeA
365+
* @param \PhpParser\Node\Name|string $typeB
366+
* @return bool
367+
*/
368+
protected function isSameType($typeA, $typeB)
369+
{
370+
$typeA = is_object($typeA) ? $typeA->toString() : $typeA;
371+
$typeB = is_object($typeB) ? $typeB->toString() : $typeB;
372+
return $typeA === $typeB;
373+
}
374+
320375
/**
321376
* @param $level
322377
* @return string

‎tests/PHPSemVerChecker/Registry/RegistryTest.php‎

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,41 @@ public function testCompareSimilarClassMethod()
103103
$this->assertDifferences($differences);
104104
}
105105

106-
public function testCompareSimilarClassMethodWithDifferentSignature()
106+
public function testCompareSimilarClassMethodWithDifferentSignatureVariables()
107+
{
108+
$before = new Registry();
109+
$after = new Registry();
110+
111+
$beforeClass = new Class_('tmp', [
112+
'stmts' => [
113+
new ClassMethod('tmpMethod', [
114+
'params' => [
115+
new Param('a', null),
116+
],
117+
]),
118+
],
119+
]);
120+
$before->addClass($beforeClass);
121+
122+
$afterClass = new Class_('tmp', [
123+
'stmts' => [
124+
new ClassMethod('tmpMethod', [
125+
'params' => [
126+
new Param('b', null),
127+
],
128+
]),
129+
],
130+
]);
131+
$after->addClass($afterClass);
132+
133+
$differences = $before->compare($after);
134+
135+
$this->assertDifferences($differences, 0, 0, 0, 0, 0, 1);
136+
$this->assertSame('Method parameter mismatch.', $differences['class'][Registry::PATCH][0]->getReason());
137+
$this->assertSame('tmp::tmpMethod', $differences['class'][Registry::PATCH][0]->getTarget());
138+
}
139+
140+
public function testCompareSimilarClassMethodWithDifferentSignatureTypehint()
107141
{
108142
$before = new Registry();
109143
$after = new Registry();
@@ -137,6 +171,41 @@ public function testCompareSimilarClassMethodWithDifferentSignature()
137171
$this->assertSame('tmp::tmpMethod', $differences['class'][Registry::MAJOR][0]->getTarget());
138172
}
139173

174+
public function testCompareSimilarClassMethodWithDifferentSignatureLength()
175+
{
176+
$before = new Registry();
177+
$after = new Registry();
178+
179+
$beforeClass = new Class_('tmp', [
180+
'stmts' => [
181+
new ClassMethod('tmpMethod', [
182+
'params' => [
183+
new Param('a', null, 'A'),
184+
new Param('b', null, 'B'),
185+
],
186+
]),
187+
],
188+
]);
189+
$before->addClass($beforeClass);
190+
191+
$afterClass = new Class_('tmp', [
192+
'stmts' => [
193+
new ClassMethod('tmpMethod', [
194+
'params' => [
195+
new Param('b', null, 'B'),
196+
],
197+
]),
198+
],
199+
]);
200+
$after->addClass($afterClass);
201+
202+
$differences = $before->compare($after);
203+
204+
$this->assertDifferences($differences, 0, 0, 0, 0, 0, 0, 0, 1);
205+
$this->assertSame('Method parameter mismatch.', $differences['class'][Registry::MAJOR][0]->getReason());
206+
$this->assertSame('tmp::tmpMethod', $differences['class'][Registry::MAJOR][0]->getTarget());
207+
}
208+
140209
public function testCompareMissingClassMethod()
141210
{
142211
$before = new Registry();
@@ -289,14 +358,63 @@ public function testCompareSimilarFunction()
289358
$this->assertDifferences($differences);
290359
}
291360

292-
public function testCompareSimilarFunctionWithDifferentSignature()
361+
public function testCompareSimilarFunctionWithDifferentSignatureVariables()
362+
{
363+
$before = new Registry();
364+
$after = new Registry();
365+
366+
$before->addFunction(new Function_('tmp', [
367+
'params' => [
368+
new Param('a', null),
369+
],
370+
]));
371+
372+
$after->addFunction(new Function_('tmp', [
373+
'params' => [
374+
new Param('b', null),
375+
],
376+
]));
377+
378+
$differences = $before->compare($after);
379+
380+
$this->assertDifferences($differences, 0, 1);
381+
$this->assertSame('Function parameter mismatch.', $differences['function'][Registry::PATCH][0]->getReason());
382+
$this->assertSame('tmp', $differences['function'][Registry::PATCH][0]->getTarget());
383+
}
384+
385+
public function testCompareSimilarFunctionWithDifferentSignatureTypehint()
386+
{
387+
$before = new Registry();
388+
$after = new Registry();
389+
390+
$before->addFunction(new Function_('tmp', [
391+
'params' => [
392+
new Param('a', null, 'A'),
393+
],
394+
]));
395+
396+
$after->addFunction(new Function_('tmp', [
397+
'params' => [
398+
new Param('a', null, 'B'),
399+
],
400+
]));
401+
402+
$differences = $before->compare($after);
403+
404+
$this->assertDifferences($differences, 0, 0, 0, 1);
405+
$this->assertSame('Function parameter mismatch.', $differences['function'][Registry::MAJOR][0]->getReason());
406+
$this->assertSame('tmp', $differences['function'][Registry::MAJOR][0]->getTarget());
407+
}
408+
409+
public function testCompareSimilarFunctionWithDifferentSignatureLength()
293410
{
294411
$before = new Registry();
295412
$after = new Registry();
296413

297414
$before->addFunction(new Function_('tmp', [
298415
'params' => [
299416
new Param('a', null, 'A'),
417+
new Param('b', null, 'B'),
300418
],
301419
]));
302420

‎tests/fixtures/after/ClassMethodParameterMismatch.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace fixtures;
44

5-
class ClassMethod
5+
class ClassMethodParameterMismatch
66
{
77
public function newMethod($someParameter)
88
{

‎tests/fixtures/before/ClassMethodParameterMismatch.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace fixtures;
44

5-
class ClassMethod
5+
class ClassMethodParameterMismatch
66
{
77
public function newMethod()
88
{

0 commit comments

Comments
 (0)