From 279d7f424159637c464ee0aa94398b83cbc9e3eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 15 Feb 2023 23:40:48 +0100 Subject: [PATCH 1/6] Implement regex-based baseline for changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds (lazy) filtering capabilities to detected changes. Signed-off-by: Luís Cobucci --- src/Baseline.php | 41 +++++++++++++++++++++++++++++++++++++++ src/Changes.php | 17 ++++++++++++++++ test/unit/ChangesTest.php | 38 ++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 src/Baseline.php diff --git a/src/Baseline.php b/src/Baseline.php new file mode 100644 index 00000000..1efa23ff --- /dev/null +++ b/src/Baseline.php @@ -0,0 +1,41 @@ + $ignoredChanges */ + private function __construct(private readonly array $ignoredChanges = []) + { + } + + public static function empty(): self + { + return new self(); + } + + /** @psalm-param list $ignoredChanges */ + public static function fromList(string ...$ignoredChanges): self + { + return new self(array_values($ignoredChanges)); + } + + public function ignores(Change $change): bool + { + $changeDescription = $change->__toString(); + + foreach ($this->ignoredChanges as $ignoredChangeRegex) { + if (preg_match($ignoredChangeRegex, $changeDescription) === 1) { + return true; + } + } + + return false; + } +} diff --git a/src/Changes.php b/src/Changes.php index 199652d7..d4770975 100644 --- a/src/Changes.php +++ b/src/Changes.php @@ -67,6 +67,23 @@ public function mergeWith(self $other): self return $instance; } + public function applyBaseline(Baseline $baseline): self + { + $instance = new self([]); + + $instance->unBufferedChanges = (function () use ($baseline): Generator { + foreach ($this as $change) { + if ($baseline->ignores($change)) { + continue; + } + + yield $change; + } + })(); + + return $instance; + } + /** * {@inheritDoc} * diff --git a/test/unit/ChangesTest.php b/test/unit/ChangesTest.php index 8e2b5283..67662c58 100644 --- a/test/unit/ChangesTest.php +++ b/test/unit/ChangesTest.php @@ -7,6 +7,7 @@ use Generator; use PHPUnit\Framework\TestCase; use Psl\Type; +use Roave\BackwardCompatibility\Baseline; use Roave\BackwardCompatibility\Change; use Roave\BackwardCompatibility\Changes; @@ -118,4 +119,41 @@ public function testCount(): void Changes::fromList(...array_fill(0, $count, Change::added('foo', true))), ); } + + /** @psalm-suppress UnusedVariable by-ref assignments are in place to override the behavior of the spy/callback */ + public function testApplyBaselineWillApplyTheFilterWithoutLoadingChanges(): void + { + $stopProducingValues = static function (): void { + self::fail('No values should have been produced'); + }; + + $changesProvider = static function () use (&$stopProducingValues): Generator { + /** @psalm-var callable(): void $stopProducingValues */ + $stopProducingValues(); + + yield Change::changed('a', true); + + $stopProducingValues(); + + yield Change::changed('b', false); + }; + + $changes = Changes::fromIterator($changesProvider()); + $filtered = $changes->applyBaseline(Baseline::fromList('#a#')); + + self::assertNotSame($changes, $filtered, 'New instance should be created'); + + $stopProducingValues = static function (): void { + }; + + $expectedChanges = [Change::changed('b', false)]; + + self::assertEquals($expectedChanges, iterator_to_array($filtered)); + self::assertEquals( + $expectedChanges, + iterator_to_array($filtered), + 'Changes can be iterated upon more than once (they are buffered)', + ); + self::assertCount(1, $filtered); + } } From 8734285a54b8d3572f24b1a0370e643a6e485fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 15 Feb 2023 23:47:03 +0100 Subject: [PATCH 2/6] Parse and apply baseline from configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This integrates the changes filter into the backward compatibility check command, parsing the configuration file prior to any process to ease the integration of new properties. Signed-off-by: Luís Cobucci --- README.md | 17 +++- src/Command/AssertBackwardsCompatible.php | 31 ++++++- src/Command/Configuration.php | 44 ++++++++++ .../Command/AssertBackwardsCompatibleTest.php | 22 +++++ test/unit/Command/ConfigurationTest.php | 82 +++++++++++++++++++ 5 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 src/Command/Configuration.php create mode 100644 test/unit/Command/ConfigurationTest.php diff --git a/README.md b/README.md index 0796b46c..59f107a7 100644 --- a/README.md +++ b/README.md @@ -122,4 +122,19 @@ vendor/bin/roave-backward-compatibility-check --help ## Configuration -There are currently no configuration options available. +The file `.roave-backward-compatibility-check.json` is read from the current working directory (when it exists) and sets configuration for the command. + +It's expected to be a JSON encoded file that, optionally, contains the following properties: + +* `baseline`: list of regexes used to filter detected changes; useful to avoid detection of known BC-breaks + +**Example:** + +```json +{ + "baseline": [ + "#\\[BC\\] CHANGED: The parameter \\$a of TestArtifact\\\\TheClass\\#method()#", + "#\\[BC\\] CHANGED: The parameter \\$b of TestArtifact\\\\TheClass\\#method2()#" + ] +} +``` diff --git a/src/Command/AssertBackwardsCompatible.php b/src/Command/AssertBackwardsCompatible.php index 75dca68a..fad23c08 100644 --- a/src/Command/AssertBackwardsCompatible.php +++ b/src/Command/AssertBackwardsCompatible.php @@ -6,6 +6,7 @@ use Psl; use Psl\Env; +use Psl\File; use Psl\Iter; use Psl\Str; use Psl\Type; @@ -35,6 +36,8 @@ final class AssertBackwardsCompatible extends Command { + private const CONFIGURATION_FILENAME = '.roave-backward-compatibility-check.json'; + /** @throws LogicException */ public function __construct( private PerformCheckoutOfRevision $git, @@ -114,7 +117,9 @@ public function execute(InputInterface $input, OutputInterface $output): int $stdErr = $output->getErrorOutput(); // @todo fix flaky assumption about the path of the source repo... - $sourceRepo = CheckedOutRepository::fromPath(Env\current_dir()); + $currentDirectory = Env\current_dir(); + + $sourceRepo = CheckedOutRepository::fromPath($currentDirectory); $fromRevision = $input->getOption('from') !== null ? $this->parseRevisionFromInput($input, $sourceRepo) @@ -126,6 +131,8 @@ public function execute(InputInterface $input, OutputInterface $output): int $toRevision = $this->parseRevision->fromStringForRepository($to, $sourceRepo); + $configuration = $this->determineConfiguration($currentDirectory, $stdErr); + $stdErr->writeln(Str\format( 'Comparing from %s to %s...', Type\string()->coerce($fromRevision), @@ -149,7 +156,7 @@ public function execute(InputInterface $input, OutputInterface $output): int $toPath->__toString(), ($this->locateDependencies)($toPath->__toString(), $includeDevelopmentDependencies), ), - ); + )->applyBaseline($configuration->baseline); $formatters = [ 'console' => new SymfonyConsoleTextFormatter($stdErr), @@ -216,4 +223,24 @@ private function determineFromRevisionFromRepository( $repository, ); } + + private function determineConfiguration( + string $currentDirectory, + OutputInterface $stdErr, + ): Configuration { + $fileName = $currentDirectory . '/' . self::CONFIGURATION_FILENAME; + + try { + $configContents = File\read($fileName); + } catch (File\Exception\InvalidArgumentException) { + return Configuration::default(); + } + + $stdErr->writeln(Str\format( + 'Using "%s" as configuration file', + Type\string()->coerce($fileName), + )); + + return Configuration::fromJson($configContents); + } } diff --git a/src/Command/Configuration.php b/src/Command/Configuration.php new file mode 100644 index 00000000..38c9dfad --- /dev/null +++ b/src/Command/Configuration.php @@ -0,0 +1,44 @@ + Type\optional(Type\vec(Type\string()))], + ), + ); + } catch (Json\Exception\DecodeException $exception) { + throw new RuntimeException( + 'It was not possible to parse the configuration', + previous: $exception, + ); + } + + $baseline = $configuration['baseline'] ?? []; + + return new self(Baseline::fromList(...$baseline)); + } +} diff --git a/test/e2e/Command/AssertBackwardsCompatibleTest.php b/test/e2e/Command/AssertBackwardsCompatibleTest.php index 5bd62902..8222755c 100644 --- a/test/e2e/Command/AssertBackwardsCompatibleTest.php +++ b/test/e2e/Command/AssertBackwardsCompatibleTest.php @@ -28,6 +28,13 @@ final class AssertBackwardsCompatibleTest extends TestCase ] } +JSON; + + private const BASELINE_CONFIGURATION = <<<'JSON' +{ + "baseline": ["#\\[BC\\] CHANGED: The parameter \\$a of TestArtifact\\\\TheClass\\#method()#"] +} + JSON; private const CLASS_VERSIONS = [ @@ -264,6 +271,21 @@ public function testWillPickLatestTaggedVersionOnNoGivenFrom(): void } } + public function testWillAllowSpecifyingBaselineConfiguration(): void + { + File\write($this->sourcesRepository . '/.roave-backward-compatibility-check.json', self::BASELINE_CONFIGURATION); + + $output = Shell\execute(__DIR__ . '/../../../bin/roave-backward-compatibility-check', [ + '--from=' . $this->versions[0], + '--to=' . $this->versions[1], + ], $this->sourcesRepository, [], Shell\ErrorOutputBehavior::Append); + + self::assertStringContainsString( + '.roave-backward-compatibility-check.json" as configuration file', + $output, + ); + } + private function tagOnVersion(string $tagName, int $version): void { Shell\execute('git', ['checkout', $this->versions[$version]], $this->sourcesRepository); diff --git a/test/unit/Command/ConfigurationTest.php b/test/unit/Command/ConfigurationTest.php new file mode 100644 index 00000000..c1957261 --- /dev/null +++ b/test/unit/Command/ConfigurationTest.php @@ -0,0 +1,82 @@ +baseline); + } + + /** @dataProvider validConfigurations */ + public function testBaselineShouldBeReadFromJsonContents( + string $jsonContents, + Baseline $expectedBaseline, + ): void { + $config = Configuration::fromJson($jsonContents); + + self::assertEquals($expectedBaseline, $config->baseline); + } + + /** @psalm-return iterable */ + public function validConfigurations(): iterable + { + yield 'empty object' => ['{}', Baseline::empty()]; + yield 'empty array' => ['[]', Baseline::empty()]; + yield 'empty baseline property' => ['{"baseline":[]}', Baseline::empty()]; + + yield 'baseline with strings' => [ + <<<'JSON' +{"baseline": ["#\\[BC\\] CHANGED: The parameter \\$a#"]} +JSON, + Baseline::fromList('#\[BC\] CHANGED: The parameter \$a#'), + ]; + + yield 'random properties are ignored' => [ + <<<'JSON' +{ + "baseline": ["#\\[BC\\] CHANGED: The parameter \\$a#"], + "random": false +} +JSON, + Baseline::fromList('#\[BC\] CHANGED: The parameter \$a#'), + ]; + } + + /** @dataProvider invalidConfigurations */ + public function testExceptionShouldBeTriggeredOnInvalidConfiguration( + string $jsonContents, + ): void { + $this->expectException(RuntimeException::class); + + Configuration::fromJson($jsonContents); + } + + /** @psalm-return iterable */ + public function invalidConfigurations(): iterable + { + yield 'empty content' => ['']; + yield 'empty string' => ['""']; + yield 'int' => ['0']; + yield 'float' => ['0.1']; + yield 'boolean' => ['false']; + yield 'baseline with string' => ['{"baseline": "this should be a list"}']; + yield 'baseline with int' => ['{"baseline": 0}']; + yield 'baseline with float' => ['{"baseline": 0.0}']; + yield 'baseline with bool' => ['{"baseline": true}']; + yield 'baseline with array of float' => ['{"baseline": [0.0]}']; + yield 'baseline with array of bool' => ['{"baseline": [false]}']; + yield 'baseline with array of object' => ['{"baseline": [{}]}']; + } +} From b29a141e412cf83fa8f14af2daecb3dd48d2b872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Thu, 23 Feb 2023 23:19:33 +0100 Subject: [PATCH 3/6] Make configuration parsing more flexible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This moves responsibilities around and introduces abstractions to support different file formats with less effort. Signed-off-by: Luís Cobucci --- src/Command/AssertBackwardsCompatible.php | 25 +----- src/Command/Configuration.php | 44 ---------- .../DetermineConfigurationFromFilesystem.php | 37 +++++++++ src/Configuration/Configuration.php | 27 ++++++ src/Configuration/ParseConfigurationFile.php | 9 ++ .../ParseJsonConfigurationFile.php | 48 +++++++++++ test/unit/Command/ConfigurationTest.php | 82 ------------------- test/unit/Configuration/ConfigurationTest.php | 21 +++++ 8 files changed, 143 insertions(+), 150 deletions(-) delete mode 100644 src/Command/Configuration.php create mode 100644 src/Command/DetermineConfigurationFromFilesystem.php create mode 100644 src/Configuration/Configuration.php create mode 100644 src/Configuration/ParseConfigurationFile.php create mode 100644 src/Configuration/ParseJsonConfigurationFile.php delete mode 100644 test/unit/Command/ConfigurationTest.php create mode 100644 test/unit/Configuration/ConfigurationTest.php diff --git a/src/Command/AssertBackwardsCompatible.php b/src/Command/AssertBackwardsCompatible.php index fad23c08..590d5eea 100644 --- a/src/Command/AssertBackwardsCompatible.php +++ b/src/Command/AssertBackwardsCompatible.php @@ -6,7 +6,6 @@ use Psl; use Psl\Env; -use Psl\File; use Psl\Iter; use Psl\Str; use Psl\Type; @@ -36,8 +35,6 @@ final class AssertBackwardsCompatible extends Command { - private const CONFIGURATION_FILENAME = '.roave-backward-compatibility-check.json'; - /** @throws LogicException */ public function __construct( private PerformCheckoutOfRevision $git, @@ -131,7 +128,7 @@ public function execute(InputInterface $input, OutputInterface $output): int $toRevision = $this->parseRevision->fromStringForRepository($to, $sourceRepo); - $configuration = $this->determineConfiguration($currentDirectory, $stdErr); + $configuration = (new DetermineConfigurationFromFilesystem())($currentDirectory, $stdErr); $stdErr->writeln(Str\format( 'Comparing from %s to %s...', @@ -223,24 +220,4 @@ private function determineFromRevisionFromRepository( $repository, ); } - - private function determineConfiguration( - string $currentDirectory, - OutputInterface $stdErr, - ): Configuration { - $fileName = $currentDirectory . '/' . self::CONFIGURATION_FILENAME; - - try { - $configContents = File\read($fileName); - } catch (File\Exception\InvalidArgumentException) { - return Configuration::default(); - } - - $stdErr->writeln(Str\format( - 'Using "%s" as configuration file', - Type\string()->coerce($fileName), - )); - - return Configuration::fromJson($configContents); - } } diff --git a/src/Command/Configuration.php b/src/Command/Configuration.php deleted file mode 100644 index 38c9dfad..00000000 --- a/src/Command/Configuration.php +++ /dev/null @@ -1,44 +0,0 @@ - Type\optional(Type\vec(Type\string()))], - ), - ); - } catch (Json\Exception\DecodeException $exception) { - throw new RuntimeException( - 'It was not possible to parse the configuration', - previous: $exception, - ); - } - - $baseline = $configuration['baseline'] ?? []; - - return new self(Baseline::fromList(...$baseline)); - } -} diff --git a/src/Command/DetermineConfigurationFromFilesystem.php b/src/Command/DetermineConfigurationFromFilesystem.php new file mode 100644 index 00000000..3b17d458 --- /dev/null +++ b/src/Command/DetermineConfigurationFromFilesystem.php @@ -0,0 +1,37 @@ +parser->parse($currentDirectory); + + if ($configuration->filename !== null) { + $stdErr->writeln(Str\format( + 'Using "%s" as configuration file', + Type\string()->coerce($configuration->filename), + )); + } + + return $configuration; + } +} diff --git a/src/Configuration/Configuration.php b/src/Configuration/Configuration.php new file mode 100644 index 00000000..e5a7bbc5 --- /dev/null +++ b/src/Configuration/Configuration.php @@ -0,0 +1,27 @@ + Type\optional(Type\vec(Type\string()))], + ), + ); + } catch (Json\Exception\DecodeException $exception) { + throw new RuntimeException( + 'It was not possible to parse the configuration', + previous: $exception, + ); + } + + $baseline = $configuration['baseline'] ?? []; + + return Configuration::fromFile( + Baseline::fromList(...$baseline), + $filename, + ); + } +} diff --git a/test/unit/Command/ConfigurationTest.php b/test/unit/Command/ConfigurationTest.php deleted file mode 100644 index c1957261..00000000 --- a/test/unit/Command/ConfigurationTest.php +++ /dev/null @@ -1,82 +0,0 @@ -baseline); - } - - /** @dataProvider validConfigurations */ - public function testBaselineShouldBeReadFromJsonContents( - string $jsonContents, - Baseline $expectedBaseline, - ): void { - $config = Configuration::fromJson($jsonContents); - - self::assertEquals($expectedBaseline, $config->baseline); - } - - /** @psalm-return iterable */ - public function validConfigurations(): iterable - { - yield 'empty object' => ['{}', Baseline::empty()]; - yield 'empty array' => ['[]', Baseline::empty()]; - yield 'empty baseline property' => ['{"baseline":[]}', Baseline::empty()]; - - yield 'baseline with strings' => [ - <<<'JSON' -{"baseline": ["#\\[BC\\] CHANGED: The parameter \\$a#"]} -JSON, - Baseline::fromList('#\[BC\] CHANGED: The parameter \$a#'), - ]; - - yield 'random properties are ignored' => [ - <<<'JSON' -{ - "baseline": ["#\\[BC\\] CHANGED: The parameter \\$a#"], - "random": false -} -JSON, - Baseline::fromList('#\[BC\] CHANGED: The parameter \$a#'), - ]; - } - - /** @dataProvider invalidConfigurations */ - public function testExceptionShouldBeTriggeredOnInvalidConfiguration( - string $jsonContents, - ): void { - $this->expectException(RuntimeException::class); - - Configuration::fromJson($jsonContents); - } - - /** @psalm-return iterable */ - public function invalidConfigurations(): iterable - { - yield 'empty content' => ['']; - yield 'empty string' => ['""']; - yield 'int' => ['0']; - yield 'float' => ['0.1']; - yield 'boolean' => ['false']; - yield 'baseline with string' => ['{"baseline": "this should be a list"}']; - yield 'baseline with int' => ['{"baseline": 0}']; - yield 'baseline with float' => ['{"baseline": 0.0}']; - yield 'baseline with bool' => ['{"baseline": true}']; - yield 'baseline with array of float' => ['{"baseline": [0.0]}']; - yield 'baseline with array of bool' => ['{"baseline": [false]}']; - yield 'baseline with array of object' => ['{"baseline": [{}]}']; - } -} diff --git a/test/unit/Configuration/ConfigurationTest.php b/test/unit/Configuration/ConfigurationTest.php new file mode 100644 index 00000000..4510fb44 --- /dev/null +++ b/test/unit/Configuration/ConfigurationTest.php @@ -0,0 +1,21 @@ +baseline); + self::assertNull($config->filename); + } +} From 024306bced3e73cd3159299e4a6cb59a09806053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Sun, 26 Feb 2023 21:40:55 +0100 Subject: [PATCH 4/6] Implement XML configuration parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ships a parser and the expected schema to ease the validation of XML documents. Signed-off-by: Luís Cobucci --- Resources/schema.xsd | 30 +++ box.json.dist | 1 + composer.json | 4 + composer.lock | 8 +- .../InvalidConfigurationStructure.php | 33 +++ src/Configuration/ParseConfigurationFile.php | 2 + .../ParseXmlConfigurationFile.php | 75 +++++++ .../ParseXmlConfigurationFileTest.php | 193 ++++++++++++++++++ 8 files changed, 344 insertions(+), 2 deletions(-) create mode 100644 Resources/schema.xsd create mode 100644 src/Configuration/InvalidConfigurationStructure.php create mode 100644 src/Configuration/ParseXmlConfigurationFile.php create mode 100644 test/unit/Configuration/ParseXmlConfigurationFileTest.php diff --git a/Resources/schema.xsd b/Resources/schema.xsd new file mode 100644 index 00000000..5a4cb174 --- /dev/null +++ b/Resources/schema.xsd @@ -0,0 +1,30 @@ + + + + + + + This schema file defines the structure for the XML configuration file of roave/backward-compatibility-check. + + + + + + + + + + + + + + + + + + + + + + + diff --git a/box.json.dist b/box.json.dist index 425ea7e1..7a89b72a 100644 --- a/box.json.dist +++ b/box.json.dist @@ -10,6 +10,7 @@ "main": "bin/roave-backward-compatibility-check.php", "output": "dist/roave-backward-compatibility-check.phar", "files-bin": [ + "Resources/schema.xsd", "LICENSE", "vendor/composer/composer/LICENSE" ], diff --git a/composer.json b/composer.json index 424038da..09a1a7c3 100644 --- a/composer.json +++ b/composer.json @@ -3,6 +3,10 @@ "description": "Tool to compare two revisions of a public API to check for BC breaks", "require": { "php": "~8.2.0 || ~8.3.0", + "ext-dom": "*", + "ext-json": "*", + "ext-libxml": "*", + "ext-simplexml": "*", "azjezz/psl": "^3.0.2", "composer/composer": "^2.7.6", "nikic/php-parser": "^4.19.1", diff --git a/composer.lock b/composer.lock index e00e1efd..f7f22bbd 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "4044c984df3464d5e8e83aafddf39fed", + "content-hash": "b45db2c9f3fcb8b5a29bab64852c0a36", "packages": [ { "name": "azjezz/psl", @@ -7350,7 +7350,11 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": "~8.2.0 || ~8.3.0" + "php": "~8.2.0 || ~8.3.0", + "ext-dom": "*", + "ext-json": "*", + "ext-libxml": "*", + "ext-simplexml": "*" }, "platform-dev": {}, "platform-overrides": { diff --git a/src/Configuration/InvalidConfigurationStructure.php b/src/Configuration/InvalidConfigurationStructure.php new file mode 100644 index 00000000..8365351f --- /dev/null +++ b/src/Configuration/InvalidConfigurationStructure.php @@ -0,0 +1,33 @@ + $errors */ + public static function fromLibxmlErrors(array $errors): self + { + $message = 'The provided configuration is invalid, errors:' . PHP_EOL; + + foreach ($errors as $error) { + $message .= sprintf( + ' - [Line %d] %s' . PHP_EOL, + $error->line, + trim($error->message), + ); + } + + return new self($message); + } +} diff --git a/src/Configuration/ParseConfigurationFile.php b/src/Configuration/ParseConfigurationFile.php index 4717b848..ffc3723d 100644 --- a/src/Configuration/ParseConfigurationFile.php +++ b/src/Configuration/ParseConfigurationFile.php @@ -1,9 +1,11 @@ validateStructure($xmlContents); + } catch (File\Exception\InvalidArgumentException) { + return Configuration::default(); + } + + $configuration = new SimpleXMLElement($xmlContents); + + return Configuration::fromFile( + $this->parseBaseline($configuration), + $filename, + ); + } + + private function validateStructure(string $xmlContents): void + { + $previousConfiguration = libxml_use_internal_errors(true); + + $xmlDocument = new DOMDocument(); + $xmlDocument->loadXML($xmlContents); + + $configurationIsValid = $xmlDocument->schemaValidate(self::SCHEMA); + + $parsingErrors = libxml_get_errors(); + libxml_use_internal_errors($previousConfiguration); + + if ($configurationIsValid) { + return; + } + + throw InvalidConfigurationStructure::fromLibxmlErrors($parsingErrors); + } + + private function parseBaseline(SimpleXMLElement $element): Baseline + { + $ignoredItems = []; + + foreach ($element->xpath('baseline/ignored-regex') ?? [] as $node) { + $ignoredItem = (string) $node; + + assert($ignoredItem !== ''); + $ignoredItems[] = $ignoredItem; + } + + return Baseline::fromList(...$ignoredItems); + } +} diff --git a/test/unit/Configuration/ParseXmlConfigurationFileTest.php b/test/unit/Configuration/ParseXmlConfigurationFileTest.php new file mode 100644 index 00000000..661fb51a --- /dev/null +++ b/test/unit/Configuration/ParseXmlConfigurationFileTest.php @@ -0,0 +1,193 @@ +temporaryDirectory = Filesystem\create_temporary_file( + Env\temp_dir(), + 'roave-backward-compatibility-xml-config-test', + ); + + self::assertNotEmpty($this->temporaryDirectory); + self::assertFileExists($this->temporaryDirectory); + + Filesystem\delete_file($this->temporaryDirectory); + Filesystem\create_directory($this->temporaryDirectory); + } + + /** @after */ + public function cleanUpFilesystem(): void + { + Shell\execute('rm', ['-rf', $this->temporaryDirectory]); + } + + /** @test */ + public function defaultConfigurationShouldBeUsedWhenFileDoesNotExist(): void + { + $config = (new ParseXmlConfigurationFile())->parse($this->temporaryDirectory); + + self::assertEquals(Configuration::default(), $config); + } + + /** + * @test + * @dataProvider invalidConfiguration + */ + public function exceptionShouldBeRaisedWhenStructureIsInvalid( + string $xmlContents, + string $expectedError, + ): void { + File\write($this->temporaryDirectory . '/.roave-backward-compatibility-check.xml', $xmlContents); + + $this->expectException(InvalidConfigurationStructure::class); + $this->expectExceptionMessage($expectedError); + + (new ParseXmlConfigurationFile())->parse($this->temporaryDirectory); + } + + /** @return iterable */ + public static function invalidConfiguration(): iterable + { + yield 'invalid root element' => [ + <<<'XML' + + +XML, + '[Line 2] Element \'anything\': No matching global declaration available for the validation root', + ]; + + yield 'invalid root child' => [ + <<<'XML' + + + + +XML, + '[Line 3] Element \'something\': This element is not expected. Expected is ( baseline )', + ]; + + yield 'multiple baseline tags' => [ + <<<'XML' + + + + + +XML, + '[Line 4] Element \'baseline\': This element is not expected', + ]; + + yield 'invalid baseline child' => [ + <<<'XML' + + + + + + +XML, + '[Line 4] Element \'nothing\': This element is not expected. Expected is ( ignored-regex )', + ]; + + yield 'invalid ignored item type' => [ + <<<'XML' + + + + + + + + +XML, + '[Line 4] Element \'ignored-regex\': Element content is not allowed, because the type definition is simple', + ]; + } + + /** + * @test + * @dataProvider validConfiguration + */ + public function baselineShouldBeParsed( + string $xmlContents, + Baseline $expectedBaseline, + ): void { + File\write($this->temporaryDirectory . '/.roave-backward-compatibility-check.xml', $xmlContents); + + self::assertEquals( + Configuration::fromFile( + $expectedBaseline, + $this->temporaryDirectory . '/.roave-backward-compatibility-check.xml', + ), + (new ParseXmlConfigurationFile())->parse($this->temporaryDirectory), + ); + } + + /** @return iterable */ + public static function validConfiguration(): iterable + { + yield 'no baseline' => [ + <<<'XML' + + +XML, + Baseline::empty(), + ]; + + yield 'empty baseline' => [ + <<<'XML' + + + + +XML, + Baseline::empty(), + ]; + + yield 'baseline with single element' => [ + <<<'XML' + + + + #\[BC\] CHANGED: The parameter \$a of TestArtifact\\TheClass\#method.*# + + +XML, + Baseline::fromList('#\[BC\] CHANGED: The parameter \$a of TestArtifact\\\\TheClass\#method.*#'), + ]; + + yield 'baseline with multiple elements' => [ + <<<'XML' + + + + #\[BC\] CHANGED: The parameter \$a of TestArtifact\\TheClass\#method.*# + #\[BC\] ADDED: Method .*\(\) was added to interface TestArtifact\\TheInterface.*# + + +XML, + Baseline::fromList( + '#\[BC\] CHANGED: The parameter \$a of TestArtifact\\\\TheClass\#method.*#', + '#\[BC\] ADDED: Method .*\(\) was added to interface TestArtifact\\\\TheInterface.*#', + ), + ]; + } +} From f824a0bcc0f6ebb13aa0ceaa87ac063a2b06868e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Sun, 26 Feb 2023 21:59:12 +0100 Subject: [PATCH 5/6] Make XML the default configuration format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Luís Cobucci --- README.md | 23 ++++++++++--------- .../DetermineConfigurationFromFilesystem.php | 4 ++-- .../Command/AssertBackwardsCompatibleTest.php | 18 ++++++++------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 59f107a7..3acef8aa 100644 --- a/README.md +++ b/README.md @@ -122,19 +122,20 @@ vendor/bin/roave-backward-compatibility-check --help ## Configuration -The file `.roave-backward-compatibility-check.json` is read from the current working directory (when it exists) and sets configuration for the command. +The file `.roave-backward-compatibility-check.xml` is read from the current working directory (when it exists) and sets configuration for the command. -It's expected to be a JSON encoded file that, optionally, contains the following properties: - -* `baseline`: list of regexes used to filter detected changes; useful to avoid detection of known BC-breaks +It's expected to be an XML file that follows our [schema](resources/schema.xsd): **Example:** -```json -{ - "baseline": [ - "#\\[BC\\] CHANGED: The parameter \\$a of TestArtifact\\\\TheClass\\#method()#", - "#\\[BC\\] CHANGED: The parameter \\$b of TestArtifact\\\\TheClass\\#method2()#" - ] -} +```xml + + + + #\[BC\] CHANGED: The parameter \$a of of TestArtifact\\TheClass\#method\(\)# + #\[BC\] CHANGED: The parameter \$b of of TestArtifact\\TheClass\#method2\(\)# + + ``` diff --git a/src/Command/DetermineConfigurationFromFilesystem.php b/src/Command/DetermineConfigurationFromFilesystem.php index 3b17d458..2be952f4 100644 --- a/src/Command/DetermineConfigurationFromFilesystem.php +++ b/src/Command/DetermineConfigurationFromFilesystem.php @@ -8,14 +8,14 @@ use Psl\Type; use Roave\BackwardCompatibility\Configuration\Configuration; use Roave\BackwardCompatibility\Configuration\ParseConfigurationFile; -use Roave\BackwardCompatibility\Configuration\ParseJsonConfigurationFile; +use Roave\BackwardCompatibility\Configuration\ParseXmlConfigurationFile; use Symfony\Component\Console\Output\OutputInterface; /** @internal */ final class DetermineConfigurationFromFilesystem { public function __construct( - private readonly ParseConfigurationFile $parser = new ParseJsonConfigurationFile(), + private readonly ParseConfigurationFile $parser = new ParseXmlConfigurationFile(), ) { } diff --git a/test/e2e/Command/AssertBackwardsCompatibleTest.php b/test/e2e/Command/AssertBackwardsCompatibleTest.php index 8222755c..becc433e 100644 --- a/test/e2e/Command/AssertBackwardsCompatibleTest.php +++ b/test/e2e/Command/AssertBackwardsCompatibleTest.php @@ -30,12 +30,14 @@ final class AssertBackwardsCompatibleTest extends TestCase JSON; - private const BASELINE_CONFIGURATION = <<<'JSON' -{ - "baseline": ["#\\[BC\\] CHANGED: The parameter \\$a of TestArtifact\\\\TheClass\\#method()#"] -} - -JSON; + private const BASELINE_CONFIGURATION = <<<'XML' + + + + #\[BC\] CHANGED: The parameter \$a of TestArtifact\\TheClass\#method.*# + + +XML; private const CLASS_VERSIONS = [ <<<'PHP' @@ -273,7 +275,7 @@ public function testWillPickLatestTaggedVersionOnNoGivenFrom(): void public function testWillAllowSpecifyingBaselineConfiguration(): void { - File\write($this->sourcesRepository . '/.roave-backward-compatibility-check.json', self::BASELINE_CONFIGURATION); + File\write($this->sourcesRepository . '/.roave-backward-compatibility-check.xml', self::BASELINE_CONFIGURATION); $output = Shell\execute(__DIR__ . '/../../../bin/roave-backward-compatibility-check', [ '--from=' . $this->versions[0], @@ -281,7 +283,7 @@ public function testWillAllowSpecifyingBaselineConfiguration(): void ], $this->sourcesRepository, [], Shell\ErrorOutputBehavior::Append); self::assertStringContainsString( - '.roave-backward-compatibility-check.json" as configuration file', + '.roave-backward-compatibility-check.xml" as configuration file', $output, ); } From f6bea18cecad13e683f08f3bf8435b483d3fcc9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Sun, 26 Feb 2023 21:59:52 +0100 Subject: [PATCH 6/6] Remove unused JSON parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Luís Cobucci --- .../ParseJsonConfigurationFile.php | 48 ------------------- 1 file changed, 48 deletions(-) delete mode 100644 src/Configuration/ParseJsonConfigurationFile.php diff --git a/src/Configuration/ParseJsonConfigurationFile.php b/src/Configuration/ParseJsonConfigurationFile.php deleted file mode 100644 index 371cb164..00000000 --- a/src/Configuration/ParseJsonConfigurationFile.php +++ /dev/null @@ -1,48 +0,0 @@ - Type\optional(Type\vec(Type\string()))], - ), - ); - } catch (Json\Exception\DecodeException $exception) { - throw new RuntimeException( - 'It was not possible to parse the configuration', - previous: $exception, - ); - } - - $baseline = $configuration['baseline'] ?? []; - - return Configuration::fromFile( - Baseline::fromList(...$baseline), - $filename, - ); - } -}