Skip to content

Commit e6cb7e2

Browse files
emmetogemmetog
committed
Dont treat case changes in method names as breaking
1 parent 27f477c commit e6cb7e2

File tree

2 files changed

+217
-114
lines changed

2 files changed

+217
-114
lines changed

‎src/PHPSemVerChecker/Analyzer/ClassMethodAnalyzer.php‎

Lines changed: 175 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -19,107 +19,179 @@
1919
use PHPSemVerChecker\Operation\ClassMethodRemoved;
2020
use PHPSemVerChecker\Report\Report;
2121

22-
class ClassMethodAnalyzer {
23-
protected $context;
24-
protected $fileBefore;
25-
protected $fileAfter;
26-
27-
/**
28-
* @param string $context
29-
* @param string $fileBefore
30-
* @param string $fileAfter
31-
*/
32-
public function __construct($context, $fileBefore = null, $fileAfter = null)
33-
{
34-
$this->context = $context;
35-
$this->fileBefore = $fileBefore;
36-
$this->fileAfter = $fileAfter;
37-
}
38-
39-
public function analyze(Stmt $contextBefore, Stmt $contextAfter)
40-
{
41-
$report = new Report();
42-
43-
$methodsBefore = $contextBefore->getMethods();
44-
$methodsAfter = $contextAfter->getMethods();
45-
46-
$methodsBeforeKeyed = [];
47-
foreach ($methodsBefore as $method) {
48-
$methodsBeforeKeyed[$method->name] = $method;
49-
}
50-
51-
$methodsAfterKeyed = [];
52-
foreach ($methodsAfter as $method) {
53-
$methodsAfterKeyed[$method->name] = $method;
54-
}
55-
56-
$methodNamesBefore = array_keys($methodsBeforeKeyed);
57-
$methodNamesAfter = array_keys($methodsAfterKeyed);
58-
$methodsAdded = array_diff($methodNamesAfter, $methodNamesBefore);
59-
$methodsRemoved = array_diff($methodNamesBefore, $methodNamesAfter);
60-
$methodsToVerify = array_intersect($methodNamesBefore, $methodNamesAfter);
61-
62-
// Here we only care about public methods as they are the only part of the API we care about
63-
64-
// Removed methods can either be implemented in parent classes or not exist anymore
65-
foreach ($methodsRemoved as $method) {
66-
$methodBefore = $methodsBeforeKeyed[$method];
67-
$data = new ClassMethodRemoved($this->context, $this->fileBefore, $contextBefore, $methodBefore);
68-
$report->add($this->context, $data);
69-
}
70-
71-
foreach ($methodsToVerify as $method) {
72-
/** @var \PhpParser\Node\Stmt\ClassMethod $methodBefore */
73-
$methodBefore = $methodsBeforeKeyed[$method];
74-
/** @var \PhpParser\Node\Stmt\ClassMethod $methodAfter */
75-
$methodAfter = $methodsAfterKeyed[$method];
76-
77-
// Leave non-strict comparison here
78-
if ($methodBefore != $methodAfter) {
79-
$paramsBefore = $methodBefore->params;
80-
$paramsAfter = $methodAfter->params;
81-
82-
$signatureResult = Signature::analyze($paramsBefore, $paramsAfter);
83-
84-
$changes = [
85-
'parameter_added' => ClassMethodParameterAdded::class,
86-
'parameter_removed' => ClassMethodParameterRemoved::class,
87-
'parameter_renamed' => ClassMethodParameterNameChanged::class,
88-
'parameter_typing_added' => ClassMethodParameterTypingAdded::class,
89-
'parameter_typing_removed' => ClassMethodParameterTypingRemoved::class,
90-
'parameter_default_added' => ClassMethodParameterDefaultAdded::class,
91-
'parameter_default_removed' => ClassMethodParameterDefaultRemoved::class,
92-
'parameter_default_value_changed' => ClassMethodParameterDefaultValueChanged::class,
93-
];
94-
95-
foreach ($changes as $changeType => $class) {
96-
if ( ! $signatureResult[$changeType]) {
97-
continue;
98-
}
99-
if (is_a($class, ClassMethodOperationUnary::class, true)) {
100-
$data = new $class($this->context, $this->fileAfter, $contextAfter, $methodAfter);
101-
} else {
102-
$data = new $class($this->context, $this->fileBefore, $contextBefore, $methodBefore, $this->fileAfter, $contextAfter, $methodAfter);
103-
}
104-
$report->add($this->context, $data);
105-
}
106-
107-
// Difference in source code
108-
// Cast to array because interfaces do not have stmts (= null)
109-
if ( ! Implementation::isSame((array)$methodBefore->stmts, (array)$methodAfter->stmts)) {
110-
$data = new ClassMethodImplementationChanged($this->context, $this->fileBefore, $contextBefore, $methodBefore, $this->fileAfter, $contextAfter, $methodAfter);
111-
$report->add($this->context, $data);
112-
}
113-
}
114-
}
115-
116-
// Added methods implies MINOR BUMP
117-
foreach ($methodsAdded as $method) {
118-
$methodAfter = $methodsAfterKeyed[$method];
119-
$data = new ClassMethodAdded($this->context, $this->fileAfter, $contextAfter, $methodAfter);
120-
$report->add($this->context, $data);
121-
}
122-
123-
return $report;
124-
}
22+
class ClassMethodAnalyzer
23+
{
24+
protected $context;
25+
protected $fileBefore;
26+
protected $fileAfter;
27+
28+
/**
29+
* @param string $context
30+
* @param string $fileBefore
31+
* @param string $fileAfter
32+
*/
33+
public function __construct($context, $fileBefore = null, $fileAfter = null)
34+
{
35+
$this->context = $context;
36+
$this->fileBefore = $fileBefore;
37+
$this->fileAfter = $fileAfter;
38+
}
39+
40+
public function analyze(Stmt $contextBefore, Stmt $contextAfter)
41+
{
42+
$report = new Report();
43+
44+
$methodsBefore = $contextBefore->getMethods();
45+
$methodsAfter = $contextAfter->getMethods();
46+
47+
$methodsBeforeKeyed = [];
48+
foreach ($methodsBefore as $method) {
49+
$methodsBeforeKeyed[strtolower($method->name)] = $method;
50+
}
51+
52+
$methodsAfterKeyed = [];
53+
foreach ($methodsAfter as $method) {
54+
$methodsAfterKeyed[strtolower($method->name)] = $method;
55+
}
56+
57+
// Here we only care about public methods as they are the only part of the API we care about
58+
59+
$methodNamesNotAddedOrRemoved = [];
60+
61+
foreach($methodsBefore as $methodBefore)
62+
{
63+
// Removed methods can either be implemented in parent classes or not exist anymore
64+
if($this->wasMethodRemoved($methodBefore, $methodsAfter))
65+
{
66+
$data = new ClassMethodRemoved($this->context, $this->fileBefore, $contextBefore, $methodBefore);
67+
$report->add($this->context, $data);
68+
} else {
69+
$methodNamesNotAddedOrRemoved[strtolower($methodBefore->name)] = true;
70+
}
71+
}
72+
73+
foreach($methodsAfter as $methodAfter)
74+
{
75+
// Added methods implies MINOR BUMP
76+
if($this->wasMethodAdded($methodAfter, $methodsBefore))
77+
{
78+
$data = new ClassMethodAdded($this->context, $this->fileAfter, $contextAfter, $methodAfter);
79+
$report->add($this->context, $data);
80+
} else {
81+
$methodNamesNotAddedOrRemoved[strtolower($methodAfter->name)] = true;
82+
}
83+
}
84+
85+
foreach (array_keys($methodNamesNotAddedOrRemoved) as $methodName) {
86+
87+
/** @var \PhpParser\Node\Stmt\ClassMethod $methodBefore */
88+
$methodBefore = $methodsBeforeKeyed[$methodName];
89+
/** @var \PhpParser\Node\Stmt\ClassMethod $methodAfter */
90+
$methodAfter = $methodsAfterKeyed[$methodName];
91+
92+
if (!$this->areMethodsEqual($methodBefore, $methodAfter)) {
93+
94+
$paramsBefore = $methodBefore->params;
95+
$paramsAfter = $methodAfter->params;
96+
97+
$signatureResult = Signature::analyze($paramsBefore, $paramsAfter);
98+
99+
$changes = [
100+
'parameter_added' => ClassMethodParameterAdded::class,
101+
'parameter_removed' => ClassMethodParameterRemoved::class,
102+
'parameter_renamed' => ClassMethodParameterNameChanged::class,
103+
'parameter_typing_added' => ClassMethodParameterTypingAdded::class,
104+
'parameter_typing_removed' => ClassMethodParameterTypingRemoved::class,
105+
'parameter_default_added' => ClassMethodParameterDefaultAdded::class,
106+
'parameter_default_removed' => ClassMethodParameterDefaultRemoved::class,
107+
'parameter_default_value_changed' => ClassMethodParameterDefaultValueChanged::class,
108+
];
109+
110+
foreach ($changes as $changeType => $class) {
111+
if (!$signatureResult[$changeType]) {
112+
continue;
113+
}
114+
if (is_a($class, ClassMethodOperationUnary::class, true)) {
115+
$data = new $class($this->context, $this->fileAfter, $contextAfter, $methodAfter);
116+
} else {
117+
$data = new $class(
118+
$this->context,
119+
$this->fileBefore,
120+
$contextBefore,
121+
$methodBefore,
122+
$this->fileAfter,
123+
$contextAfter,
124+
$methodAfter
125+
);
126+
}
127+
$report->add($this->context, $data);
128+
}
129+
130+
// Difference in source code
131+
// Cast to array because interfaces do not have stmts (= null)
132+
if (!Implementation::isSame((array)$methodBefore->stmts, (array)$methodAfter->stmts)) {
133+
$data = new ClassMethodImplementationChanged(
134+
$this->context,
135+
$this->fileBefore,
136+
$contextBefore,
137+
$methodBefore,
138+
$this->fileAfter,
139+
$contextAfter,
140+
$methodAfter
141+
);
142+
$report->add($this->context, $data);
143+
}
144+
}
145+
}
146+
147+
return $report;
148+
}
149+
150+
private function areMethodsEqual(Stmt\ClassMethod $methodBefore, Stmt\ClassMethod $methodAfter)
151+
{
152+
if ($methodBefore == $methodAfter) {
153+
return true;
154+
};
155+
156+
return strtolower($methodBefore->name) === strtolower($methodAfter->name)
157+
&& $methodBefore->isPrivate() === $methodAfter->isPrivate()
158+
&& $methodBefore->isAbstract() === $methodAfter->isAbstract()
159+
&& $methodBefore->isFinal() === $methodAfter->isFinal()
160+
&& $methodBefore->isProtected() === $methodAfter->isProtected()
161+
&& $methodBefore->isPublic() === $methodAfter->isPublic()
162+
&& $methodBefore->isStatic() === $methodAfter->isStatic()
163+
&& $methodBefore->getReturnType() === $methodAfter->getReturnType()
164+
// statements are objects, cannot be compared with ===
165+
&& $methodBefore->getStmts() == $methodAfter->getStmts()
166+
&& $methodBefore->getParams() === $methodAfter->getParams()
167+
&& $methodBefore->returnsByRef() === $methodAfter->returnsByRef()
168+
&& $methodBefore->getType() === $methodAfter->getType()
169+
&& $methodBefore->getAttributes() === $methodAfter->getAttributes();
170+
}
171+
172+
private function wasMethodAdded(Stmt\ClassMethod $method, $methodsAfter)
173+
{
174+
foreach($methodsAfter as $methodAfter)
175+
{
176+
if(strtolower($method->name) == strtolower($methodAfter->name))
177+
{
178+
return false;
179+
}
180+
}
181+
182+
return true;
183+
}
184+
185+
private function wasMethodRemoved(Stmt\ClassMethod $method, $methodsBefore)
186+
{
187+
foreach($methodsBefore as $methodBefore)
188+
{
189+
if(strtolower($method->name) == strtolower($methodBefore->name))
190+
{
191+
return false;
192+
}
193+
}
194+
195+
return true;
196+
}
125197
}

‎tests/PHPSemVerChecker/Analyzer/ClassMethodAnalyzerTest.php‎

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -697,15 +697,46 @@ public function testClassMethodImplementationChanged($context, $visibility, $cod
697697
$this->assertSame('tmp::tmpMethod', $report[$context][$expectedLevel][0]->getTarget());
698698
}
699699

700-
public function providerImplementationChanged()
701-
{
702-
return [
703-
['class', 'public', 'V023'],
704-
['class', 'protected', 'V024'],
705-
['class', 'private', 'V025'],
706-
['trait', 'public', 'V052'],
707-
['trait', 'protected', 'V053'],
708-
['trait', 'private', 'V054'],
709-
];
710-
}
700+
public function providerImplementationChanged()
701+
{
702+
return [
703+
['class', 'public', 'V023'],
704+
['class', 'protected', 'V024'],
705+
['class', 'private', 'V025'],
706+
['trait', 'public', 'V052'],
707+
['trait', 'protected', 'V053'],
708+
['trait', 'private', 'V054'],
709+
];
710+
}
711+
712+
public function testClassMethodCaseChangeIsIgnored()
713+
{
714+
$constructor = $this->getConstructorForContext('class');
715+
$classBefore = new $constructor('tmp', [
716+
'stmts' => [
717+
new ClassMethod('tmpMethod', [
718+
'type' => Visibility::getModifier('public'),
719+
'stmts' => [
720+
new MethodCall(new Variable('test'), 'someMethod'),
721+
],
722+
]),
723+
],
724+
]);
725+
726+
$classAfter = new $constructor('tmp', [
727+
'stmts' => [
728+
new ClassMethod('tmpmethod', [
729+
'type' => Visibility::getModifier('public'),
730+
'stmts' => [
731+
new MethodCall(new Variable('test'), 'someMethod'),
732+
],
733+
]),
734+
],
735+
]);
736+
737+
$analyzer = new ClassMethodAnalyzer('class');
738+
$report = $analyzer->analyze($classBefore, $classAfter);
739+
740+
Assert::assertNoDifference($report);
741+
}
711742
}

0 commit comments

Comments
 (0)