Skip to content

Commit

Permalink
fix readonly final class architecture side effect issue PHP8.2 (#645)
Browse files Browse the repository at this point in the history
* 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 <courciermarvin@gmail.com>
Co-authored-by: Chris Gmyr <cmgmyr@gmail.com>
  • Loading branch information
3 people authored Sep 28, 2023
1 parent a71a20b commit 382f294
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 13 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
.idea/*
.env
.phpunit.result.cache
.phpunit.cache/
/composer.lock
.php_cs.cache
.php_cs.cache
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"psr/simple-cache": "^1.0|^2.0|^3.0",
"sebastian/diff": "^4.0|^5.0",
"slevomat/coding-standard": "^7.0.8|^8.0",
"squizlabs/php_codesniffer": "^3.5",
"squizlabs/php_codesniffer": "^3.7",
"symfony/cache": "^4.4|^5.0|^6.0",
"symfony/console": "^4.2.12|^5.0|^6.0",
"symfony/finder": "^4.2.12|^5.0|^6.0",
Expand Down
13 changes: 7 additions & 6 deletions phpunit.xml
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
<?xml version="1.0" encoding="UTF-8"?>
<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">
<coverage>
<include>
<directory suffix=".php">./src</directory>
</include>
</coverage>
<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">
<coverage/>
<testsuites>
<testsuite name="Test Suite">
<directory suffix="Test.php">./tests</directory>
</testsuite>
</testsuites>
<source>
<include>
<directory suffix=".php">./src</directory>
</include>
</source>
</phpunit>
4 changes: 2 additions & 2 deletions src/Domain/Analyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ private function analyseFile(Collector $collector, string $filename): void
$collector->incrementInterfaces();
} elseif (isset($tokens[$i - 2]) &&
\is_array($tokens[$i - 2])) {
if ($tokens[$i - 2][0] === \T_ABSTRACT) {
if ($tokens[$i - 2][0] === \T_ABSTRACT || $tokens[$i - 2][0] === \T_READONLY && $tokens[$i - 4][0] === \T_ABSTRACT) {
$collector->addAbstractClass($filename);
} elseif ($tokens[$i - 2][0] === \T_FINAL) {
} elseif ($tokens[$i - 2][0] === \T_FINAL || $tokens[$i - 2][0] === \T_READONLY && $tokens[$i - 4][0] === \T_FINAL) {
$collector->addConcreteFinalClass($filename);
} else {
$collector->addConcreteNonFinalClass($filename);
Expand Down
14 changes: 14 additions & 0 deletions tests/Domain/Insights/Fixtures/ReadonlyClass.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace Tests\Domain\Insights\Fixtures;

/**
* @see \Tests\Domain\Insights\ReadonlyClassTest
*/
readonly class ReadonlyClass {
public function __construct(
private string $foo,
private string $bar,
) {
}
}
64 changes: 64 additions & 0 deletions tests/Domain/Insights/ReadonlyClassTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

declare(strict_types=1);

namespace Tests\Domain\Insights;

use NunoMaduro\PhpInsights\Application\Console\Formatters\PathShortener;
use NunoMaduro\PhpInsights\Domain\Analyser;
use NunoMaduro\PhpInsights\Domain\Insights\ForbiddenNormalClasses;
use NunoMaduro\PhpInsights\Domain\Metrics\Architecture\Classes;
use Tests\TestCase;

final class ReadonlyClassTest extends TestCase
{
public function testHasIssue(): void
{
if (PHP_VERSION_ID < 80200) {
self::markTestSkipped('Readonly classes are only available in PHP 8.2+');
}

$files = [
__DIR__ . '/Fixtures/ReadonlyClass.php',
];

$analyzer = new Analyser();
$collector = $analyzer->analyse([__DIR__ . '/Fixtures/'], $files, PathShortener::extractCommonPath($files));
$insight = new ForbiddenNormalClasses($collector, []);

self::assertTrue($insight->hasIssue());
self::assertIsArray($insight->getDetails());
self::assertNotEmpty($insight->getDetails());
}

public function testSkipFile(): void
{
$fileLocation = __DIR__ . '/Fixtures/ReadonlyClass.php';

$collection = $this->runAnalyserOnConfig(
[
'add' => [
Classes::class => [
ForbiddenNormalClasses::class,
],
],
'config' => [
ForbiddenNormalClasses::class => [
'exclude' => [$fileLocation],
],
],
],
[$fileLocation]
);

$classErrors = 0;

foreach ($collection->allFrom(new Classes()) as $insight) {
if ($insight->hasIssue() && $insight->getInsightClass() === ForbiddenNormalClasses::class) {
$classErrors++;
}
}

self::assertEquals(0, $classErrors);
}
}
6 changes: 3 additions & 3 deletions tests/Domain/Insights/SyntaxCheckTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public function testHasIssueOnDirectory(): void
usort($details, static fn (Details $a, Details $b): int => $a->getFile() <=> $b->getFile());

self::assertTrue($insight->hasIssue());
if (PHP_MAJOR_VERSION === 7) {
self::assertCount(2, $details);
} else {
if (PHP_MAJOR_VERSION === 7 || PHP_VERSION_ID >= 80200) {
self::assertCount(3, $details);
} else {
self::assertCount(4, $details);
}

/** @var \NunoMaduro\PhpInsights\Domain\Details $detail */
Expand Down

0 comments on commit 382f294

Please sign in to comment.