Skip to content

Commit 7e0be48

Browse files
floriankraemerfloriankraemer
committed
Refactor property detection classes to improve readability and maintainability. Introduced helper methods for detecting property changes, including type changes, readonly status, static status, and removals. Enhanced the PropertyRemovedDetector to handle renames more effectively.
1 parent c4586bb commit 7e0be48

6 files changed

Lines changed: 525 additions & 184 deletions

‎src/Detector/MethodSignatureChangedDetector.php‎

Lines changed: 167 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Phauthentic\BcCheck\ValueObject\ClassInfo;
2222
use Phauthentic\BcCheck\ValueObject\MethodInfo;
2323
use Phauthentic\BcCheck\ValueObject\ParameterInfo;
24+
use Phauthentic\BcCheck\ValueObject\TypeInfo;
2425

2526
final readonly class MethodSignatureChangedDetector implements BcBreakDetectorInterface
2627
{
@@ -32,14 +33,12 @@ public function detect(ClassInfo $before, ClassInfo $after): array
3233
$afterMethod = $after->getMethod($beforeMethod->name);
3334

3435
if ($afterMethod === null) {
35-
// Method removed - handled by MethodRemovedDetector
36+
// * Method removed - handled by MethodRemovedDetector
3637
continue;
3738
}
3839

3940
$methodBreaks = $this->compareSignatures($before->name, $beforeMethod, $afterMethod);
40-
foreach ($methodBreaks as $break) {
41-
$breaks[] = $break;
42-
}
41+
$breaks = [...$breaks, ...$methodBreaks];
4342
}
4443

4544
return $breaks;
@@ -52,26 +51,41 @@ private function compareSignatures(string $className, MethodInfo $before, Method
5251
{
5352
$breaks = [];
5453

55-
// Check if new required parameters were added
54+
$requiredParamBreak = $this->checkRequiredParametersAdded($className, $before, $after);
55+
if ($requiredParamBreak !== null) {
56+
$breaks[] = $requiredParamBreak;
57+
}
58+
59+
$paramBreaks = $this->compareExistingParameters($className, $before, $after);
60+
foreach ($paramBreaks as $break) {
61+
$breaks[] = $break;
62+
}
63+
64+
return $breaks;
65+
}
66+
67+
private function checkRequiredParametersAdded(string $className, MethodInfo $before, MethodInfo $after): ?BcBreak
68+
{
5669
$beforeRequired = $before->getRequiredParameterCount();
5770
$afterRequired = $after->getRequiredParameterCount();
5871

5972
if ($afterRequired > $beforeRequired) {
60-
$breaks[] = new BcBreak(
61-
message: sprintf(
62-
'Method %s::%s() has more required parameters (%d -> %d)',
63-
$className,
64-
$before->name,
65-
$beforeRequired,
66-
$afterRequired,
67-
),
73+
return $this->createBreak(
74+
message: $this->formatRequiredParametersAddedMessage($className, $before->name, $beforeRequired, $afterRequired),
6875
className: $className,
6976
memberName: $before->name,
70-
type: BcBreakType::MethodSignatureChanged,
7177
);
7278
}
7379

74-
// Check parameter type changes for existing parameters
80+
return null;
81+
}
82+
83+
/**
84+
* @return list<BcBreak>
85+
*/
86+
private function compareExistingParameters(string $className, MethodInfo $before, MethodInfo $after): array
87+
{
88+
$breaks = [];
7589
$minCount = min(count($before->parameters), count($after->parameters));
7690

7791
for ($i = 0; $i < $minCount; $i++) {
@@ -87,6 +101,21 @@ className: $className,
87101
return $breaks;
88102
}
89103

104+
private function formatRequiredParametersAddedMessage(
105+
string $className,
106+
string $methodName,
107+
int $beforeCount,
108+
int $afterCount,
109+
): string {
110+
return sprintf(
111+
'Method %s::%s() has more required parameters (%d -> %d)',
112+
$className,
113+
$methodName,
114+
$beforeCount,
115+
$afterCount,
116+
);
117+
}
118+
90119
/**
91120
* @return list<BcBreak>
92121
*/
@@ -99,67 +128,149 @@ private function compareParameters(
99128
): array {
100129
$breaks = [];
101130

102-
// Parameter type changed (added or made more restrictive)
131+
$typeBreak = $this->checkParameterTypeChange($className, $methodName, $before, $after);
132+
if ($typeBreak !== null) {
133+
$breaks[] = $typeBreak;
134+
}
135+
136+
$defaultBreak = $this->checkParameterDefaultRemoved($className, $methodName, $before, $after);
137+
if ($defaultBreak !== null) {
138+
$breaks[] = $defaultBreak;
139+
}
140+
141+
$referenceBreak = $this->checkParameterByReferenceChange($className, $methodName, $before, $after);
142+
if ($referenceBreak !== null) {
143+
$breaks[] = $referenceBreak;
144+
}
145+
146+
return $breaks;
147+
}
148+
149+
private function checkParameterTypeChange(
150+
string $className,
151+
string $methodName,
152+
ParameterInfo $before,
153+
ParameterInfo $after,
154+
): ?BcBreak {
155+
// Type was added (was untyped, now has type)
103156
if ($before->type === null && $after->type !== null) {
104-
$breaks[] = new BcBreak(
105-
message: sprintf(
106-
'Parameter $%s of %s::%s() now has type %s',
107-
$before->name,
108-
$className,
109-
$methodName,
110-
$after->type->toString(),
111-
),
157+
return $this->createBreak(
158+
message: $this->formatTypeAddedMessage($className, $methodName, $before->name, $after->type),
112159
className: $className,
113160
memberName: $methodName,
114-
type: BcBreakType::MethodSignatureChanged,
115161
);
116-
} elseif ($before->type !== null && $after->type !== null && !$before->type->equals($after->type)) {
117-
$breaks[] = new BcBreak(
118-
message: sprintf(
119-
'Parameter $%s of %s::%s() changed type from %s to %s',
120-
$before->name,
121-
$className,
122-
$methodName,
123-
$before->type->toString(),
124-
$after->type->toString(),
125-
),
162+
}
163+
164+
// Type changed (both had types, but different)
165+
if ($before->type !== null && $after->type !== null && !$before->type->equals($after->type)) {
166+
return $this->createBreak(
167+
message: $this->formatTypeChangedMessage($className, $methodName, $before->name, $before->type, $after->type),
126168
className: $className,
127169
memberName: $methodName,
128-
type: BcBreakType::MethodSignatureChanged,
129170
);
130171
}
131172

132-
// Default removed (was optional, now required)
173+
return null;
174+
}
175+
176+
private function checkParameterDefaultRemoved(
177+
string $className,
178+
string $methodName,
179+
ParameterInfo $before,
180+
ParameterInfo $after,
181+
): ?BcBreak {
133182
if ($before->hasDefault && !$after->hasDefault) {
134-
$breaks[] = new BcBreak(
135-
message: sprintf(
136-
'Parameter $%s of %s::%s() is no longer optional',
137-
$before->name,
138-
$className,
139-
$methodName,
140-
),
183+
return $this->createBreak(
184+
message: $this->formatDefaultRemovedMessage($className, $methodName, $before->name),
141185
className: $className,
142186
memberName: $methodName,
143-
type: BcBreakType::MethodSignatureChanged,
144187
);
145188
}
146189

147-
// By-reference changed
190+
return null;
191+
}
192+
193+
private function checkParameterByReferenceChange(
194+
string $className,
195+
string $methodName,
196+
ParameterInfo $before,
197+
ParameterInfo $after,
198+
): ?BcBreak {
148199
if ($before->isByReference !== $after->isByReference) {
149-
$breaks[] = new BcBreak(
150-
message: sprintf(
151-
'Parameter $%s of %s::%s() %s',
152-
$before->name,
153-
$className,
154-
$methodName,
155-
$after->isByReference ? 'is now passed by reference' : 'is no longer passed by reference',
156-
),
200+
return $this->createBreak(
201+
message: $this->formatByReferenceChangedMessage($className, $methodName, $before->name, $after->isByReference),
157202
className: $className,
158203
memberName: $methodName,
159-
type: BcBreakType::MethodSignatureChanged,
160204
);
161205
}
162206

163-
return $breaks;
207+
return null;
208+
}
209+
210+
private function createBreak(string $message, string $className, string $memberName): BcBreak
211+
{
212+
return new BcBreak(
213+
message: $message,
214+
className: $className,
215+
memberName: $memberName,
216+
type: BcBreakType::MethodSignatureChanged,
217+
);
218+
}
219+
220+
private function formatTypeAddedMessage(string $className, string $methodName, string $paramName, TypeInfo $type): string
221+
{
222+
return sprintf(
223+
'Parameter $%s of %s::%s() now has type %s',
224+
$paramName,
225+
$className,
226+
$methodName,
227+
$type->toString(),
228+
);
229+
}
230+
231+
private function formatTypeChangedMessage(
232+
string $className,
233+
string $methodName,
234+
string $paramName,
235+
TypeInfo $beforeType,
236+
TypeInfo $afterType,
237+
): string {
238+
return sprintf(
239+
'Parameter $%s of %s::%s() changed type from %s to %s',
240+
$paramName,
241+
$className,
242+
$methodName,
243+
$beforeType->toString(),
244+
$afterType->toString(),
245+
);
246+
}
247+
248+
private function formatDefaultRemovedMessage(string $className, string $methodName, string $paramName): string
249+
{
250+
return sprintf(
251+
'Parameter $%s of %s::%s() is no longer optional',
252+
$paramName,
253+
$className,
254+
$methodName,
255+
);
256+
}
257+
258+
private function formatByReferenceChangedMessage(
259+
string $className,
260+
string $methodName,
261+
string $paramName,
262+
bool $isNowByReference,
263+
): string {
264+
$changeDescription = $isNowByReference
265+
? 'is now passed by reference'
266+
: 'is no longer passed by reference';
267+
268+
return sprintf(
269+
'Parameter $%s of %s::%s() %s',
270+
$paramName,
271+
$className,
272+
$methodName,
273+
$changeDescription,
274+
);
164275
}
165276
}

‎src/Detector/ParentChangedDetector.php‎

Lines changed: 60 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -24,36 +24,67 @@
2424
{
2525
public function detect(ClassInfo $before, ClassInfo $after): array
2626
{
27-
// If before had a parent and after doesn't, or they're different
28-
if ($before->parentClass !== null && $before->parentClass !== $after->parentClass) {
29-
if ($after->parentClass === null) {
30-
return [
31-
new BcBreak(
32-
message: sprintf(
33-
'Class %s no longer extends %s',
34-
$before->name,
35-
$before->parentClass,
36-
),
37-
className: $before->name,
38-
type: BcBreakType::ParentChanged,
39-
),
40-
];
41-
}
42-
43-
return [
44-
new BcBreak(
45-
message: sprintf(
46-
'Class %s changed parent from %s to %s',
47-
$before->name,
48-
$before->parentClass,
49-
$after->parentClass,
50-
),
51-
className: $before->name,
52-
type: BcBreakType::ParentChanged,
53-
),
54-
];
27+
if (!$this->hasParentChanged($before, $after)) {
28+
return [];
5529
}
5630

57-
return [];
31+
return [$this->detectParentChange($before, $after)];
32+
}
33+
34+
private function hasParentChanged(ClassInfo $before, ClassInfo $after): bool
35+
{
36+
return $before->parentClass !== null
37+
&& $before->parentClass !== $after->parentClass;
38+
}
39+
40+
private function detectParentChange(ClassInfo $before, ClassInfo $after): BcBreak
41+
{
42+
if ($after->parentClass === null) {
43+
return $this->createParentRemovedBreak($before);
44+
}
45+
46+
return $this->createParentChangedBreak($before, $after);
47+
}
48+
49+
private function createParentRemovedBreak(ClassInfo $before): BcBreak
50+
{
51+
assert($before->parentClass !== null);
52+
53+
return new BcBreak(
54+
message: $this->formatParentRemovedMessage($before->name, $before->parentClass),
55+
className: $before->name,
56+
type: BcBreakType::ParentChanged,
57+
);
58+
}
59+
60+
private function createParentChangedBreak(ClassInfo $before, ClassInfo $after): BcBreak
61+
{
62+
assert($before->parentClass !== null);
63+
assert($after->parentClass !== null);
64+
65+
return new BcBreak(
66+
message: $this->formatParentChangedMessage($before->name, $before->parentClass, $after->parentClass),
67+
className: $before->name,
68+
type: BcBreakType::ParentChanged,
69+
);
70+
}
71+
72+
private function formatParentRemovedMessage(string $className, string $parentClass): string
73+
{
74+
return sprintf(
75+
'Class %s no longer extends %s',
76+
$className,
77+
$parentClass,
78+
);
79+
}
80+
81+
private function formatParentChangedMessage(string $className, string $beforeParent, string $afterParent): string
82+
{
83+
return sprintf(
84+
'Class %s changed parent from %s to %s',
85+
$className,
86+
$beforeParent,
87+
$afterParent,
88+
);
5889
}
5990
}

0 commit comments

Comments
 (0)