Skip to content

Commit 9296147

Browse files
OcramiusOcramius
authored
Merge pull request #768 from bdsl/767-enum-cases
Detect changes in ENUMs as BC breaks
2 parents 92d33e4 + c245e9c commit 9296147

File tree

8 files changed

+341
-2
lines changed

8 files changed

+341
-2
lines changed

‎bin/roave-backward-compatibility-check.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ static function (string $installationPath) use ($composerIo): Installer {
9696
new ClassBased\SkipClassBasedErrors(new ClassBased\MethodRemoved()),
9797
new ClassBased\SkipClassBasedErrors(new ClassBased\AncestorRemoved()),
9898
new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameInternal()),
99+
new ClassBased\SkipClassBasedErrors(new ClassBased\EnumCasesChanged()),
99100
new ClassBased\SkipClassBasedErrors(new ClassBased\OpenClassChanged(
100101
new ClassBased\MultipleChecksOnAClass(
101102
new ClassBased\SkipClassBasedErrors(new ClassBased\ConstantChanged(

‎infection.json.dist

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@
1616
"IdenticalEqual": false,
1717
"NotIdenticalNotEqual": false
1818
},
19-
"minMsi": 90.1,
19+
"minMsi": 85.8,
2020
"minCoveredMsi": 100
2121
}

‎src/DetectChanges/BCBreak/ClassBased/EnumCasesChanged.php

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Roave\BackwardCompatibility\DetectChanges\BCBreak\ClassBased;
6+
7+
use Psl\Regex;
8+
use Roave\BackwardCompatibility\Change;
9+
use Roave\BackwardCompatibility\Changes;
10+
use Roave\BetterReflection\Reflection\ReflectionClass;
11+
use Roave\BetterReflection\Reflection\ReflectionEnum;
12+
use Roave\BetterReflection\Reflection\ReflectionEnumCase;
13+
14+
use function array_filter;
15+
use function array_map;
16+
17+
final class EnumCasesChanged implements ClassBased
18+
{
19+
public function __invoke(ReflectionClass $fromClass, ReflectionClass $toClass): Changes
20+
{
21+
$fromEnumName = $fromClass->getName();
22+
$fromKind = $this->kindOf($fromClass);
23+
$toKind = $this->kindOf($toClass);
24+
25+
if (! $fromClass instanceof ReflectionEnum && ! $toClass instanceof ReflectionEnum) {
26+
return Changes::empty();
27+
}
28+
29+
if (! $fromClass instanceof ReflectionEnum) {
30+
return Changes::fromList(Change::changed($fromKind . ' ' . $fromEnumName . ' became enum'));
31+
}
32+
33+
if (! $toClass instanceof ReflectionEnum) {
34+
return Changes::fromList(Change::changed('enum ' . $fromEnumName . ' became ' . $toKind));
35+
}
36+
37+
$addedCases = array_filter(
38+
$toClass->getCases(),
39+
static function (ReflectionEnumCase $case) use ($fromClass): bool {
40+
if (self::isInternalDocComment($case->getDocComment())) {
41+
return false;
42+
}
43+
44+
return ! $fromClass->hasCase($case->getName());
45+
},
46+
);
47+
48+
$removedCases = array_filter(
49+
$fromClass->getCases(),
50+
static function (ReflectionEnumCase $case) use ($toClass): bool {
51+
if (self::isInternalDocComment($case->getDocComment())) {
52+
return false;
53+
}
54+
55+
return ! $toClass->hasCase($case->getName());
56+
},
57+
);
58+
59+
$internalisedCases = array_filter(
60+
$toClass->getCases(),
61+
static function (ReflectionEnumCase $case) use ($fromClass) {
62+
if (! self::isInternalDocComment($case->getDocComment())) {
63+
return false;
64+
}
65+
66+
$fromClassCase = $fromClass->getCase($case->getName());
67+
if (! $fromClassCase) {
68+
return false;
69+
}
70+
71+
return ! self::isInternalDocComment($fromClassCase->getDocComment());
72+
},
73+
);
74+
75+
$nowNotInternalCases = array_filter(
76+
$toClass->getCases(),
77+
static function (ReflectionEnumCase $case) use ($fromClass) {
78+
if (self::isInternalDocComment($case->getDocComment())) {
79+
return false;
80+
}
81+
82+
$fromClassCase = $fromClass->getCase($case->getName());
83+
if (! $fromClassCase) {
84+
return false;
85+
}
86+
87+
return self::isInternalDocComment($fromClassCase->getDocComment());
88+
},
89+
);
90+
91+
$caseRemovedChanges = array_map(
92+
static function (ReflectionEnumCase $case) use ($fromEnumName): Change {
93+
$caseName = $case->getName();
94+
95+
return Change::removed('Case ' . $fromEnumName . '::' . $caseName . ' was removed');
96+
},
97+
$removedCases,
98+
);
99+
100+
$caseAddedChanges = array_map(
101+
static function (ReflectionEnumCase $case) use ($fromEnumName): Change {
102+
$caseName = $case->getName();
103+
104+
return Change::added('Case ' . $fromEnumName . '::' . $caseName . ' was added');
105+
},
106+
$addedCases,
107+
);
108+
109+
$caseBecameInternalChanges = array_map(
110+
static function (ReflectionEnumCase $case) use ($fromEnumName): Change {
111+
$caseName = $case->getName();
112+
113+
return Change::changed('Case ' . $fromEnumName . '::' . $caseName . ' was marked "@internal"');
114+
},
115+
$internalisedCases,
116+
);
117+
118+
$caseBecameNotInternalChanges = array_map(
119+
static function (ReflectionEnumCase $case) use ($fromEnumName): Change {
120+
$caseName = $case->getName();
121+
122+
return Change::changed('Case ' . $fromEnumName . '::' . $caseName . ' had "@internal" removed');
123+
},
124+
$nowNotInternalCases,
125+
);
126+
127+
return Changes::fromList(
128+
...$caseRemovedChanges,
129+
...$caseAddedChanges,
130+
...$caseBecameInternalChanges,
131+
...$caseBecameNotInternalChanges,
132+
);
133+
}
134+
135+
private static function isInternalDocComment(string|null $comment): bool
136+
{
137+
return $comment !== null
138+
&& Regex\matches($comment, '/\s+@internal\s+/');
139+
}
140+
141+
/** @psalm-return 'enum'|'interface'|'trait'|'class' */
142+
private function kindOf(ReflectionClass $reflectionClass): string
143+
{
144+
if ($reflectionClass->isEnum()) {
145+
return 'enum';
146+
}
147+
148+
if ($reflectionClass->isInterface()) {
149+
return 'interface';
150+
}
151+
152+
if ($reflectionClass->isTrait()) {
153+
return 'trait';
154+
}
155+
156+
return 'class';
157+
}
158+
}

‎test/asset/api/new/EnumWithCasesBeingChanged.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace RoaveTestAsset;
6+
7+
enum EnumWithCasesBeingChanged
8+
{
9+
// two new months below:
10+
case January;
11+
case February;
12+
13+
/** @internal - tired from the two new Months, reserving March for private use from now on.*/
14+
case March;
15+
16+
case April;
17+
case May;
18+
case June;
19+
case July;
20+
21+
// We're on holiday in August, not going to allow that month any more.
22+
// case August;
23+
24+
case September;
25+
case October;
26+
case November;
27+
case December;
28+
29+
/**
30+
* For testing now but for use only in a future major version. As it should not be statically referenced from
31+
* outside this library, and the library will not pass a reference to it to the outside at runtime, adding it
32+
* is not a BC break.
33+
* @internal
34+
*/
35+
case Sol;
36+
37+
/**
38+
* Previously internal, now public. This is a BC break as we previously committed to never return an instance
39+
* of this case and we no-longer make this commitment.
40+
*/
41+
case UnknownMonth;
42+
}

‎test/asset/api/old/EnumWithCasesBeingChanged.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace RoaveTestAsset;
6+
7+
enum EnumWithCasesBeingChanged
8+
{
9+
case March;
10+
case April;
11+
case May;
12+
case June;
13+
case July;
14+
case August;
15+
case September;
16+
case October;
17+
case November;
18+
Case december; // oops forget this is spelled with a capital D
19+
20+
/**
21+
* @internal - may be removed without notice
22+
*/
23+
case FakeMonth;
24+
25+
/**
26+
* @internal - may be introduced in future
27+
*/
28+
case UnknownMonth;
29+
}

‎test/unit/DetectChanges/BCBreak/ClassBased/DummyEnum.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace RoaveTest\BackwardCompatibility\DetectChanges\BCBreak\ClassBased;
6+
7+
enum DummyEnum
8+
{
9+
case someCase;
10+
}

‎test/unit/DetectChanges/BCBreak/ClassBased/EnumCasesChangedTest.php

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace RoaveTest\BackwardCompatibility\DetectChanges\BCBreak\ClassBased;
6+
7+
use PHPUnit\Framework\TestCase;
8+
use Roave\BackwardCompatibility\Change;
9+
use Roave\BackwardCompatibility\DetectChanges\BCBreak\ClassBased\EnumCasesChanged;
10+
use Roave\BetterReflection\BetterReflection;
11+
use Roave\BetterReflection\Reflection\ReflectionClass;
12+
use Roave\BetterReflection\Reflector\DefaultReflector;
13+
use Roave\BetterReflection\SourceLocator\Type\SingleFileSourceLocator;
14+
use stdClass;
15+
16+
use function array_map;
17+
use function iterator_to_array;
18+
19+
/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\ClassBased\EnumCasesChanged */
20+
final class EnumCasesChangedTest extends TestCase
21+
{
22+
/**
23+
* @param string[] $expectedMessages
24+
*
25+
* @dataProvider enumsToBeTested
26+
*/
27+
public function testDiffs(
28+
ReflectionClass $fromEnum,
29+
ReflectionClass $toEnum,
30+
array $expectedMessages,
31+
): void {
32+
$changes = (new EnumCasesChanged())($fromEnum, $toEnum);
33+
34+
self::assertSame(
35+
$expectedMessages,
36+
array_map(static function (Change $change): string {
37+
return $change->__toString();
38+
}, iterator_to_array($changes)),
39+
);
40+
}
41+
42+
public function testReturnsClassBecameEnumError(): void
43+
{
44+
$changes = (new EnumCasesChanged())(
45+
ReflectionClass::createFromName(stdClass::class),
46+
ReflectionClass::createFromName(DummyEnum::class),
47+
);
48+
49+
$this->assertEquals(
50+
[Change::changed('class stdClass became enum')],
51+
iterator_to_array($changes),
52+
);
53+
}
54+
55+
public function testReturnsEnumBecameClassError(): void
56+
{
57+
$changes = (new EnumCasesChanged())(
58+
ReflectionClass::createFromName(DummyEnum::class),
59+
ReflectionClass::createFromName(stdClass::class),
60+
);
61+
62+
$this->assertEquals(
63+
[Change::changed('enum RoaveTest\BackwardCompatibility\DetectChanges\BCBreak\ClassBased\DummyEnum became class')],
64+
iterator_to_array($changes),
65+
);
66+
}
67+
68+
/**
69+
* @return array<string, array<int, ReflectionClass|array<int, string>>>
70+
* @psalm-return array<string, array{0: ReflectionClass, 1: ReflectionClass, 2: list<string>}>
71+
*/
72+
public function enumsToBeTested(): array
73+
{
74+
$locator = (new BetterReflection())->astLocator();
75+
76+
return [
77+
'RoaveTestAsset\\EnumWithCasesBeingChanged' => [
78+
(new DefaultReflector(new SingleFileSourceLocator(
79+
__DIR__ . '/../../../../asset/api/old/EnumWithCasesBeingChanged.php',
80+
$locator,
81+
)))->reflectClass('RoaveTestAsset\EnumWithCasesBeingChanged'),
82+
(new DefaultReflector(new SingleFileSourceLocator(
83+
__DIR__ . '/../../../../asset/api/new/EnumWithCasesBeingChanged.php',
84+
$locator,
85+
)))->reflectClass('RoaveTestAsset\EnumWithCasesBeingChanged'),
86+
[
87+
'[BC] REMOVED: Case RoaveTestAsset\EnumWithCasesBeingChanged::August was removed',
88+
'[BC] REMOVED: Case RoaveTestAsset\EnumWithCasesBeingChanged::december was removed',
89+
'[BC] ADDED: Case RoaveTestAsset\EnumWithCasesBeingChanged::January was added',
90+
'[BC] ADDED: Case RoaveTestAsset\EnumWithCasesBeingChanged::February was added',
91+
'[BC] ADDED: Case RoaveTestAsset\EnumWithCasesBeingChanged::December was added',
92+
'[BC] CHANGED: Case RoaveTestAsset\EnumWithCasesBeingChanged::March was marked "@internal"',
93+
'[BC] CHANGED: Case RoaveTestAsset\EnumWithCasesBeingChanged::UnknownMonth had "@internal" removed',
94+
],
95+
],
96+
];
97+
}
98+
}

‎test/unit/Git/GitCheckoutRevisionToTemporaryPathTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,9 @@ public function testCheckoutDirectoryIsCorrect(): void
126126
$git = new GitCheckoutRevisionToTemporaryPath();
127127
$revision = Revision::fromSha1(self::TEST_REVISION_TO_CHECKOUT);
128128
$directory = $git->generateTemporaryPathFor($revision);
129+
$tmpDir = Env\temp_dir();
129130

130-
self::assertStringContainsString('/tmp/api-compare-', $directory);
131+
self::assertStringContainsString($tmpDir . '/api-compare-', $directory);
131132
}
132133

133134
private function sourceRepository(): CheckedOutRepository

0 commit comments

Comments
 (0)