From 02b3a709fe44b7ec872dead4f8463c40a46bf4e7 Mon Sep 17 00:00:00 2001 From: Christian Sciberras Date: Thu, 11 Jun 2026 09:27:49 +0200 Subject: [PATCH 1/2] Refactor config internals --- composer.json | 2 +- src/Fixer/BlockStringFixer.php | 40 +++++++++++++++------- src/Formatter/AbstractFormatter.php | 11 +++++- tests/Unit/Fixer/BlockStringFixerTest.php | 41 +++++++++++++++++++---- 4 files changed, 73 insertions(+), 21 deletions(-) diff --git a/composer.json b/composer.json index a10dc6d..a6ef666 100644 --- a/composer.json +++ b/composer.json @@ -22,12 +22,12 @@ ], "require": { "php": "^7.4 || ^8.0", + "ext-json": "*", "symfony/process": "^5 || ^6 || ^7 || ^8", "friendsofphp/php-cs-fixer": "^3", "symfony/deprecation-contracts": "^2 || ^3" }, "require-dev": { - "ext-json": "*", "ergebnis/composer-normalize": "^2.7", "phpunit/phpunit": "^9.5", "phpstan/phpstan": "^2.1", diff --git a/src/Fixer/BlockStringFixer.php b/src/Fixer/BlockStringFixer.php index 25b70dc..de7843e 100644 --- a/src/Fixer/BlockStringFixer.php +++ b/src/Fixer/BlockStringFixer.php @@ -32,7 +32,9 @@ final class BlockStringFixer implements FixerInterface, ConfigurableFixerInterfa /** * @var TDeserializedConfig */ - private array $configuration; + private array $configuration = [ + 'formatters' => [], + ]; public function isRisky(): bool { @@ -79,19 +81,31 @@ public function getConfigurationDefinition(): FixerConfigurationResolverInterfac public function configure(array $configuration): void { - if (!is_string($formatters = $configuration['formatters'] ?? 'a:0:{}') - || ($formatters = @unserialize($formatters, ['allowed_classes' => true])) === false - ) { - throw new InvalidArgumentException( - 'BlockStringFixer configuration is not valid. ' - . 'Did you set it up in your PHP CS Fixer config with `BlockStringFixer::config()`?' - ); + $formatters = $configuration['formatters'] ?? []; + if (!is_array($formatters)) { + throw new InvalidArgumentException(sprintf( + 'BlockStringFixer configuration is not valid: formatters must be an array, "%s" given.', + is_object($formatters) ? get_class($formatters) : gettype($formatters) + )); } + foreach ($formatters as $key => $formatter) { + if (is_int($key) && $key !== 0) { + throw new InvalidArgumentException(sprintf( + 'BlockStringFixer configuration is not valid: formatter for integer key %s will never be used.', + $key + )); + } - // @phpstan-ignore assign.propertyType - $this->configuration = [ - 'formatters' => $formatters, - ]; + if (!$formatter instanceof AbstractFormatter) { + throw new InvalidArgumentException(sprintf( + 'BlockStringFixer configuration is not valid: formatter for key %s must be an instance of %s, %s was given instead.', + $key, + AbstractFormatter::class, + is_object($formatter) ? get_class($formatter) : gettype($formatter) + )); + } + } + $this->configuration['formatters'] = $formatters; } public function fix(SplFileInfo $file, Tokens $tokens): void @@ -117,7 +131,7 @@ public function fix(SplFileInfo $file, Tokens $tokens): void public static function config(array $formatters): array { return [ - 'formatters' => serialize($formatters), + 'formatters' => $formatters, ]; } } diff --git a/src/Formatter/AbstractFormatter.php b/src/Formatter/AbstractFormatter.php index de51b3c..f3faea8 100644 --- a/src/Formatter/AbstractFormatter.php +++ b/src/Formatter/AbstractFormatter.php @@ -2,6 +2,7 @@ namespace uuf6429\PhpCsFixerBlockstring\Formatter; +use JsonSerializable; use uuf6429\PhpCsFixerBlockstring\BlockString\BlockString; use uuf6429\PhpCsFixerBlockstring\CacheFingerprintableInterface; use uuf6429\PhpCsFixerBlockstring\InterpolationCodec\CodecInterface; @@ -17,7 +18,7 @@ * 2. Or if, for whatever reason, the {@see CodecInterface} concept does not work for you and you want to write * something from scratch. */ -abstract class AbstractFormatter implements CacheFingerprintableInterface +abstract class AbstractFormatter implements CacheFingerprintableInterface, JsonSerializable { /** * @var mixed @@ -52,4 +53,12 @@ final public function getCacheFingerprint() { return $this->cacheFingerprint; } + + /** + * @return array{cacheFingerprint: mixed, ...} + */ + public function jsonSerialize(): array + { + return ['cacheFingerprint' => $this->getCacheFingerprint()]; + } } diff --git a/tests/Unit/Fixer/BlockStringFixerTest.php b/tests/Unit/Fixer/BlockStringFixerTest.php index eb173e8..c89fc7f 100644 --- a/tests/Unit/Fixer/BlockStringFixerTest.php +++ b/tests/Unit/Fixer/BlockStringFixerTest.php @@ -2,6 +2,7 @@ namespace uuf6429\PhpCsFixerBlockstringTests\Unit\Fixer; +use Exception; use InvalidArgumentException; use PhpCsFixer\Tokenizer\Tokens; use PHPUnit\Framework\TestCase; @@ -47,20 +48,48 @@ public function testGetConfigurationDefinition(): void } /** - * @testWith ["not a serialized string"] - * [[{"some": "invalid", "config": "structure"}]] * @param mixed $formatters + * @dataProvider invalidConfigurationProvider */ - public function testConfigureWithInvalidConfiguration($formatters): void + public function testConfigureWithInvalidConfiguration($formatters, Exception $expectedException): void { - $this->expectExceptionObject(new InvalidArgumentException('BlockStringFixer configuration is not valid.')); + $this->expectExceptionObject($expectedException); (new BlockStringFixer())->configure(['formatters' => $formatters]); } - public function testConfig(): void + /** + * @return iterable + */ + public static function invalidConfigurationProvider(): iterable { - $this->assertSame(['formatters' => 'a:0:{}'], BlockStringFixer::config([])); + yield 'invalid value for formatters' => [ + 'formatters' => 'invalid', + 'expectedException' => new InvalidArgumentException( + 'BlockStringFixer configuration is not valid: formatters must be an array, "string" given.' + ), + ]; + + yield 'invalid numeric key' => [ + 'formatters' => [1 => 'test'], + 'expectedException' => new InvalidArgumentException( + 'BlockStringFixer configuration is not valid: formatter for integer key 1 will never be used.' + ), + ]; + + yield 'invalid default formatter' => [ + 'formatters' => [0 => 'invalid'], + 'expectedException' => new InvalidArgumentException( + 'BlockStringFixer configuration is not valid: formatter for key 0 must be an instance of uuf6429\PhpCsFixerBlockstring\Formatter\AbstractFormatter, string was given instead.', + ), + ]; + + yield 'invalid JSON formatter' => [ + 'formatters' => ['JSON' => 'invalid'], + 'expectedException' => new InvalidArgumentException( + 'BlockStringFixer configuration is not valid: formatter for key JSON must be an instance of uuf6429\PhpCsFixerBlockstring\Formatter\AbstractFormatter, string was given instead.', + ) + ]; } /** From 3f93cc9550f16b8aa26fa5665503b4f067b9ff26 Mon Sep 17 00:00:00 2001 From: Christian Sciberras Date: Thu, 11 Jun 2026 09:32:11 +0200 Subject: [PATCH 2/2] Improve coverage --- tests/Unit/Formatter/AbstractFormatterTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Formatter/AbstractFormatterTest.php b/tests/Unit/Formatter/AbstractFormatterTest.php index ffad18f..8dc216f 100644 --- a/tests/Unit/Formatter/AbstractFormatterTest.php +++ b/tests/Unit/Formatter/AbstractFormatterTest.php @@ -13,13 +13,14 @@ final class AbstractFormatterTest extends TestCase { public function testThatAbstractFormatterIsCacheable(): void { - $formatter = new class('fingerprint') extends AbstractFormatter { + $formatter = new class('the fingerprint') extends AbstractFormatter { public function formatBlock(BlockString $blockString): BlockString { return $blockString; } }; - $this->assertSame('fingerprint', $formatter->getCacheFingerprint()); + $this->assertSame('the fingerprint', $formatter->getCacheFingerprint()); + $this->assertSame(['cacheFingerprint' => 'the fingerprint'], $formatter->jsonSerialize()); } }