Skip to content

Commit 382f294

Browse files
helyakinMarvin Courciercmgmyr
authored
fix readonly final class architecture side effect issue PHP8.2 (#645)
* fix readonly final class architecture side effect issue PHP8.2 * add readonly also for abstract * add tests * bump min codesniffer for T_READONLY * tests v2 * test again * update phpunit config --------- Co-authored-by: Marvin Courcier <[email protected]> Co-authored-by: Chris Gmyr <[email protected]>
1 parent a71a20b commit 382f294

File tree

7 files changed

+93
-13
lines changed

7 files changed

+93
-13
lines changed

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@
44
.idea/*
55
.env
66
.phpunit.result.cache
7+
.phpunit.cache/
78
/composer.lock
8-
.php_cs.cache
9+
.php_cs.cache

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"psr/simple-cache": "^1.0|^2.0|^3.0",
3333
"sebastian/diff": "^4.0|^5.0",
3434
"slevomat/coding-standard": "^7.0.8|^8.0",
35-
"squizlabs/php_codesniffer": "^3.5",
35+
"squizlabs/php_codesniffer": "^3.7",
3636
"symfony/cache": "^4.4|^5.0|^6.0",
3737
"symfony/console": "^4.2.12|^5.0|^6.0",
3838
"symfony/finder": "^4.2.12|^5.0|^6.0",

‎phpunit.xml‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" backupGlobals="false" beStrictAboutTestsThatDoNotTestAnything="true" beStrictAboutOutputDuringTests="true" bootstrap="vendor/squizlabs/php_codesniffer/tests/bootstrap.php" colors="true" failOnRisky="true" failOnWarning="true" processIsolation="false" stopOnError="false" stopOnFailure="false" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.0/phpunit.xsd" cacheDirectory=".phpunit.cache" backupStaticProperties="false">
3-
<coverage>
4-
<include>
5-
<directory suffix=".php">./src</directory>
6-
</include>
7-
</coverage>
2+
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" backupGlobals="false" beStrictAboutTestsThatDoNotTestAnything="true" beStrictAboutOutputDuringTests="true" bootstrap="vendor/squizlabs/php_codesniffer/tests/bootstrap.php" colors="true" failOnRisky="true" failOnWarning="true" processIsolation="false" stopOnError="false" stopOnFailure="false" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.3/phpunit.xsd" cacheDirectory=".phpunit.cache" backupStaticProperties="false">
3+
<coverage/>
84
<testsuites>
95
<testsuite name="Test Suite">
106
<directory suffix="Test.php">./tests</directory>
117
</testsuite>
128
</testsuites>
9+
<source>
10+
<include>
11+
<directory suffix=".php">./src</directory>
12+
</include>
13+
</source>
1314
</phpunit>

‎src/Domain/Analyser.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ private function analyseFile(Collector $collector, string $filename): void
177177
$collector->incrementInterfaces();
178178
} elseif (isset($tokens[$i - 2]) &&
179179
\is_array($tokens[$i - 2])) {
180-
if ($tokens[$i - 2][0] === \T_ABSTRACT) {
180+
if ($tokens[$i - 2][0] === \T_ABSTRACT || $tokens[$i - 2][0] === \T_READONLY && $tokens[$i - 4][0] === \T_ABSTRACT) {
181181
$collector->addAbstractClass($filename);
182-
} elseif ($tokens[$i - 2][0] === \T_FINAL) {
182+
} elseif ($tokens[$i - 2][0] === \T_FINAL || $tokens[$i - 2][0] === \T_READONLY && $tokens[$i - 4][0] === \T_FINAL) {
183183
$collector->addConcreteFinalClass($filename);
184184
} else {
185185
$collector->addConcreteNonFinalClass($filename);

‎tests/Domain/Insights/Fixtures/ReadonlyClass.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace Tests\Domain\Insights\Fixtures;
4+
5+
/**
6+
* @see \Tests\Domain\Insights\ReadonlyClassTest
7+
*/
8+
readonly class ReadonlyClass {
9+
public function __construct(
10+
private string $foo,
11+
private string $bar,
12+
) {
13+
}
14+
}

‎tests/Domain/Insights/ReadonlyClassTest.php‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Domain\Insights;
6+
7+
use NunoMaduro\PhpInsights\Application\Console\Formatters\PathShortener;
8+
use NunoMaduro\PhpInsights\Domain\Analyser;
9+
use NunoMaduro\PhpInsights\Domain\Insights\ForbiddenNormalClasses;
10+
use NunoMaduro\PhpInsights\Domain\Metrics\Architecture\Classes;
11+
use Tests\TestCase;
12+
13+
final class ReadonlyClassTest extends TestCase
14+
{
15+
public function testHasIssue(): void
16+
{
17+
if (PHP_VERSION_ID < 80200) {
18+
self::markTestSkipped('Readonly classes are only available in PHP 8.2+');
19+
}
20+
21+
$files = [
22+
__DIR__ . '/Fixtures/ReadonlyClass.php',
23+
];
24+
25+
$analyzer = new Analyser();
26+
$collector = $analyzer->analyse([__DIR__ . '/Fixtures/'], $files, PathShortener::extractCommonPath($files));
27+
$insight = new ForbiddenNormalClasses($collector, []);
28+
29+
self::assertTrue($insight->hasIssue());
30+
self::assertIsArray($insight->getDetails());
31+
self::assertNotEmpty($insight->getDetails());
32+
}
33+
34+
public function testSkipFile(): void
35+
{
36+
$fileLocation = __DIR__ . '/Fixtures/ReadonlyClass.php';
37+
38+
$collection = $this->runAnalyserOnConfig(
39+
[
40+
'add' => [
41+
Classes::class => [
42+
ForbiddenNormalClasses::class,
43+
],
44+
],
45+
'config' => [
46+
ForbiddenNormalClasses::class => [
47+
'exclude' => [$fileLocation],
48+
],
49+
],
50+
],
51+
[$fileLocation]
52+
);
53+
54+
$classErrors = 0;
55+
56+
foreach ($collection->allFrom(new Classes()) as $insight) {
57+
if ($insight->hasIssue() && $insight->getInsightClass() === ForbiddenNormalClasses::class) {
58+
$classErrors++;
59+
}
60+
}
61+
62+
self::assertEquals(0, $classErrors);
63+
}
64+
}

‎tests/Domain/Insights/SyntaxCheckTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,10 @@ public function testHasIssueOnDirectory(): void
4949
usort($details, static fn (Details $a, Details $b): int => $a->getFile() <=> $b->getFile());
5050

5151
self::assertTrue($insight->hasIssue());
52-
if (PHP_MAJOR_VERSION === 7) {
53-
self::assertCount(2, $details);
54-
} else {
52+
if (PHP_MAJOR_VERSION === 7 || PHP_VERSION_ID >= 80200) {
5553
self::assertCount(3, $details);
54+
} else {
55+
self::assertCount(4, $details);
5656
}
5757

5858
/** @var \NunoMaduro\PhpInsights\Domain\Details $detail */

0 commit comments

Comments
 (0)