diff --git a/.editorconfig b/.editorconfig index 041cbae6..e965c0ff 100644 --- a/.editorconfig +++ b/.editorconfig @@ -12,6 +12,10 @@ insert_final_newline = true [tabs.txt] indent_style = tab +# Text fixtures deliberately carry trailing whitespace to exercise reformatting +[*.txt.fixture] +trim_trailing_whitespace = false + # YAML-Files [{*.yml,*.yaml,*.yml.fixture,*.yaml.fixture}] indent_size = 2 diff --git a/e2e/typo3-yaml/expected-output.txt b/e2e/typo3-yaml/expected-output.txt index 2a80bcb8..a18e966f 100644 --- a/e2e/typo3-yaml/expected-output.txt +++ b/e2e/typo3-yaml/expected-output.txt @@ -9,9 +9,8 @@ - options: recipients: -- bar@domain.com: 'Bar' + foo@domain.com: Bar -+ bar@domain.com: Bar + bar@domain.com: Bar subject: 'Kontaktanfrage von Website' - recipientAddress: foo@domain.com - recipientName: Bar @@ -32,9 +31,8 @@ - options: recipients: -- bar@domain.com: 'Bar' + foo@domain.com: Bar -+ bar@domain.com: Bar + bar@domain.com: Bar subject: 'Kontaktanfrage von Website' - recipientAddress: foo@domain.com - recipientName: Bar @@ -70,8 +68,9 @@ ----------- end diff ----------- Applied rules: + * CodeFormatRule * EmailFinisherYamlFractor - [OK] 1 file has been changed by Fractor + [OK] 1 file has been changed by Fractor diff --git a/packages/fractor-composer-json/src/ComposerJsonFileProcessor.php b/packages/fractor-composer-json/src/ComposerJsonFileProcessor.php index 86e62677..3d087cd1 100644 --- a/packages/fractor-composer-json/src/ComposerJsonFileProcessor.php +++ b/packages/fractor-composer-json/src/ComposerJsonFileProcessor.php @@ -36,23 +36,28 @@ public function canHandle(File $file): bool public function handle(File $file, iterable $appliedRules): void { - $fileHasChanged = \false; + $rawContent = $file->getContent(); $composerJson = $this->composerJsonFactory->createFromFile($file); - $oldComposerJson = $this->composerJsonFactory->createFromFile($file); foreach ($appliedRules as $rule) { + $beforeArray = $composerJson->getJsonArray(); $rule->refactor($composerJson); - if ($oldComposerJson->getJsonArray() !== $composerJson->getJsonArray()) { - $file->changeFileContent($this->composerJsonPrinter->printToString($this->indent, $composerJson)); + if ($beforeArray !== $composerJson->getJsonArray()) { $file->addAppliedRule(AppliedRule::fromRule($rule)); - $fileHasChanged = \true; } } - if (! $fileHasChanged) { + // Always re-print: a formatting-only change (indentation) is then + // attributed to CodeFormatRule by the runner rather than applied silently. + $newContent = rtrim($this->composerJsonPrinter->printToString($this->indent, $composerJson)) . "\n"; + + if ($newContent === $rawContent) { $this->changedFilesDetector->addCachableFile($file->getFilePath()); + return; } + + $file->changeFileContent($newContent); } public function allowedFileExtensions(): array diff --git a/packages/fractor-composer-json/tests/ComposerJsonProcessResult/ComposerJsonProcessResultTest.php b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/ComposerJsonProcessResultTest.php new file mode 100644 index 00000000..6ddf57b8 --- /dev/null +++ b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/ComposerJsonProcessResultTest.php @@ -0,0 +1,51 @@ + $expectedAppliedRules + */ + #[DataProvider('provideData')] + public function test(string $fixture, array $expectedAppliedRules): void + { + $fractorTestResult = $this->doTestFile(__DIR__ . '/Fixtures/' . $fixture); + + self::assertSame($expectedAppliedRules, $fractorTestResult->getAppliedFractorRules()); + } + + /** + * @return \Iterator}> + */ + public static function provideData(): \Iterator + { + yield 'rule change is attributed to the real rule' => [ + 'rule-change/composer.json.fixture', + [RemovePackageComposerJsonFractor::class], + ]; + yield 'pure reformat is attributed to the virtual code-format rule' => [ + 'reformatting/composer.json.fixture', + [AppliedRule::CODE_FORMAT_RULE], + ]; + yield 'already formatted file reports no change' => ['already-formatted/composer.json.fixture', []]; + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/fractor.php'; + } +} diff --git a/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/already-formatted/composer.json.fixture b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/already-formatted/composer.json.fixture new file mode 100644 index 00000000..2e72d9e2 --- /dev/null +++ b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/already-formatted/composer.json.fixture @@ -0,0 +1,6 @@ +{ + "name": "acme/demo", + "require": { + "vendor1/keep": "^2.0" + } +} diff --git a/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/reformatting/composer.json.fixture b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/reformatting/composer.json.fixture new file mode 100644 index 00000000..033aec33 --- /dev/null +++ b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/reformatting/composer.json.fixture @@ -0,0 +1,13 @@ +{ + "name": "acme/demo", + "require": { + "vendor1/keep": "^2.0" + } +} +----- +{ + "name": "acme/demo", + "require": { + "vendor1/keep": "^2.0" + } +} diff --git a/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/rule-change/composer.json.fixture b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/rule-change/composer.json.fixture new file mode 100644 index 00000000..a2cfcd50 --- /dev/null +++ b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/Fixtures/rule-change/composer.json.fixture @@ -0,0 +1,12 @@ +{ + "require": { + "vendor1/legacy": "^1.0", + "vendor1/keep": "^2.0" + } +} +----- +{ + "require": { + "vendor1/keep": "^2.0" + } +} diff --git a/packages/fractor-composer-json/tests/ComposerJsonProcessResult/config/fractor.php b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/config/fractor.php new file mode 100644 index 00000000..1af28660 --- /dev/null +++ b/packages/fractor-composer-json/tests/ComposerJsonProcessResult/config/fractor.php @@ -0,0 +1,10 @@ +import(__DIR__ . '/../../../config/application.php') + ->withConfiguredRule(RemovePackageComposerJsonFractor::class, ['vendor1/legacy']); diff --git a/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/already-formatted.typoscript.fixture b/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/already-formatted.typoscript.fixture new file mode 100644 index 00000000..17c6d84c --- /dev/null +++ b/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/already-formatted.typoscript.fixture @@ -0,0 +1,3 @@ +page = PAGE +page.10 = TEXT +page.10.value = Hello diff --git a/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/reformatting.typoscript.fixture b/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/reformatting.typoscript.fixture new file mode 100644 index 00000000..291afa76 --- /dev/null +++ b/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/reformatting.typoscript.fixture @@ -0,0 +1,7 @@ +page = PAGE + page.10 = TEXT + page.10.value = Hello +----- +page = PAGE +page.10 = TEXT +page.10.value = Hello diff --git a/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/rule-change.typoscript.fixture b/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/rule-change.typoscript.fixture new file mode 100644 index 00000000..fc5dadd7 --- /dev/null +++ b/packages/fractor-typoscript/tests/TypoScriptProcessResult/Fixtures/rule-change.typoscript.fixture @@ -0,0 +1,4 @@ +page = PAGE +config.spamProtectEmailAddresses = ascii +----- +page = PAGE diff --git a/packages/fractor-typoscript/tests/TypoScriptProcessResult/TypoScriptProcessResultTest.php b/packages/fractor-typoscript/tests/TypoScriptProcessResult/TypoScriptProcessResultTest.php new file mode 100644 index 00000000..4549d888 --- /dev/null +++ b/packages/fractor-typoscript/tests/TypoScriptProcessResult/TypoScriptProcessResultTest.php @@ -0,0 +1,50 @@ + $expectedAppliedRules + */ + #[DataProvider('provideData')] + public function test(string $fixture, array $expectedAppliedRules): void + { + $fractorTestResult = $this->doTestFile(__DIR__ . '/Fixtures/' . $fixture); + + self::assertSame($expectedAppliedRules, $fractorTestResult->getAppliedFractorRules()); + } + + /** + * @return \Iterator}> + */ + public static function provideData(): \Iterator + { + yield 'rule change is attributed to the real rule' => [ + 'rule-change.typoscript.fixture', + [DummyTypoScriptFractorRule::class], + ]; + yield 'pure reformat is attributed to the virtual code-format rule' => [ + 'reformatting.typoscript.fixture', + [AppliedRule::CODE_FORMAT_RULE], + ]; + yield 'already formatted file reports no change' => ['already-formatted.typoscript.fixture', []]; + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/fractor.php'; + } +} diff --git a/packages/fractor-typoscript/tests/TypoScriptProcessResult/config/fractor.php b/packages/fractor-typoscript/tests/TypoScriptProcessResult/config/fractor.php new file mode 100644 index 00000000..4f340831 --- /dev/null +++ b/packages/fractor-typoscript/tests/TypoScriptProcessResult/config/fractor.php @@ -0,0 +1,20 @@ +withOptions([ + TypoScriptProcessorOption::INDENT_SIZE => 4, + TypoScriptProcessorOption::INDENT_CHARACTER => PrettyPrinterConfiguration::INDENTATION_STYLE_SPACES, + TypoScriptProcessorOption::ADD_CLOSING_GLOBAL => false, + TypoScriptProcessorOption::INCLUDE_EMPTY_LINE_BREAKS => true, + TypoScriptProcessorOption::INDENT_CONDITIONS => true, + TypoScriptProcessorOption::CONDITION_TERMINATION => PrettyPrinterConditionTermination::Keep, + ]) + ->withRules([DummyTypoScriptFractorRule::class]); diff --git a/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/already-formatted.xlf.fixture b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/already-formatted.xlf.fixture new file mode 100644 index 00000000..c4ef41ad --- /dev/null +++ b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/already-formatted.xlf.fixture @@ -0,0 +1,11 @@ + + + + + + Goodbye + + + + diff --git a/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/reformatting.xlf.fixture b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/reformatting.xlf.fixture new file mode 100644 index 00000000..17e7b185 --- /dev/null +++ b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/reformatting.xlf.fixture @@ -0,0 +1,22 @@ + + + + + + Goodbye + + + + +----- + + + + + + Goodbye + + + + diff --git a/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/rule-and-formatting.xlf.fixture b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/rule-and-formatting.xlf.fixture new file mode 100644 index 00000000..b8fd7aec --- /dev/null +++ b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/rule-and-formatting.xlf.fixture @@ -0,0 +1,22 @@ + + + + + + Hello + + + + +----- + + + + + + Hello World + + + + diff --git a/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/rule-change.xlf.fixture b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/rule-change.xlf.fixture new file mode 100644 index 00000000..5f467836 --- /dev/null +++ b/packages/fractor-xliff/tests/XliffProcessResult/Fixtures/rule-change.xlf.fixture @@ -0,0 +1,23 @@ + + + + + + Hello + + + + +----- + + + + + + Hello World + + + + diff --git a/packages/fractor-xliff/tests/XliffProcessResult/XliffProcessResultTest.php b/packages/fractor-xliff/tests/XliffProcessResult/XliffProcessResultTest.php new file mode 100644 index 00000000..173bdf11 --- /dev/null +++ b/packages/fractor-xliff/tests/XliffProcessResult/XliffProcessResultTest.php @@ -0,0 +1,54 @@ + $expectedAppliedRules + */ + #[DataProvider('provideData')] + public function test(string $fixture, array $expectedAppliedRules): void + { + $fractorTestResult = $this->doTestFile(__DIR__ . '/Fixtures/' . $fixture); + + self::assertSame($expectedAppliedRules, $fractorTestResult->getAppliedFractorRules()); + } + + /** + * @return \Iterator}> + */ + public static function provideData(): \Iterator + { + yield 'rule change is attributed to the real rule' => [ + 'rule-change.xlf.fixture', + [DummyXliffFractorRule::class], + ]; + yield 'pure reformat is attributed to the virtual code-format rule' => [ + 'reformatting.xlf.fixture', + [AppliedRule::CODE_FORMAT_RULE], + ]; + yield 'rule change and reformat are both reported' => [ + 'rule-and-formatting.xlf.fixture', + [AppliedRule::CODE_FORMAT_RULE, DummyXliffFractorRule::class], + ]; + yield 'already formatted file reports no change' => ['already-formatted.xlf.fixture', []]; + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/fractor.php'; + } +} diff --git a/packages/fractor-xliff/tests/XliffProcessResult/config/fractor.php b/packages/fractor-xliff/tests/XliffProcessResult/config/fractor.php new file mode 100644 index 00000000..439dd186 --- /dev/null +++ b/packages/fractor-xliff/tests/XliffProcessResult/config/fractor.php @@ -0,0 +1,15 @@ +withOptions([ + XliffProcessorOption::INDENT_CHARACTER => Indent::STYLE_TAB, + XliffProcessorOption::INDENT_SIZE => 1, + ]) + ->withRules([DummyXliffFractorRule::class]); diff --git a/packages/fractor-xml/tests/XmlProcessResult/Fixtures/already-formatted.xml.fixture b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/already-formatted.xml.fixture new file mode 100644 index 00000000..1d411f40 --- /dev/null +++ b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/already-formatted.xml.fixture @@ -0,0 +1,5 @@ + + + value + value + diff --git a/packages/fractor-xml/tests/XmlProcessResult/Fixtures/reformatting.xml.fixture b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/reformatting.xml.fixture new file mode 100644 index 00000000..4869d3a7 --- /dev/null +++ b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/reformatting.xml.fixture @@ -0,0 +1,11 @@ + + + value + value + +----- + + + value + value + diff --git a/packages/fractor-xml/tests/XmlProcessResult/Fixtures/rule-and-formatting.xml.fixture b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/rule-and-formatting.xml.fixture new file mode 100644 index 00000000..83e67ab1 --- /dev/null +++ b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/rule-and-formatting.xml.fixture @@ -0,0 +1,34 @@ + + + + + + + + + input + 50 + + + + + + + +----- + + + + + + + + + input + + + + + + + diff --git a/packages/fractor-xml/tests/XmlProcessResult/Fixtures/rule-change.xml.fixture b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/rule-change.xml.fixture new file mode 100644 index 00000000..2a44d511 --- /dev/null +++ b/packages/fractor-xml/tests/XmlProcessResult/Fixtures/rule-change.xml.fixture @@ -0,0 +1,34 @@ + + + + + + + + + input + 50 + + + + + + + +----- + + + + + + + + + input + + + + + + + diff --git a/packages/fractor-xml/tests/XmlProcessResult/XmlProcessResultTest.php b/packages/fractor-xml/tests/XmlProcessResult/XmlProcessResultTest.php new file mode 100644 index 00000000..c309dd45 --- /dev/null +++ b/packages/fractor-xml/tests/XmlProcessResult/XmlProcessResultTest.php @@ -0,0 +1,51 @@ + $expectedAppliedRules + */ + #[DataProvider('provideData')] + public function test(string $fixture, array $expectedAppliedRules): void + { + $fractorTestResult = $this->doTestFile(__DIR__ . '/Fixtures/' . $fixture); + + self::assertSame($expectedAppliedRules, $fractorTestResult->getAppliedFractorRules()); + } + + /** + * @return \Iterator}> + */ + public static function provideData(): \Iterator + { + yield 'rule change is attributed to the real rule' => ['rule-change.xml.fixture', [DummyXmlFractorRule::class]]; + yield 'pure reformat is attributed to the virtual code-format rule' => [ + 'reformatting.xml.fixture', + [AppliedRule::CODE_FORMAT_RULE], + ]; + yield 'rule change and reformat are both reported' => [ + 'rule-and-formatting.xml.fixture', + [AppliedRule::CODE_FORMAT_RULE, DummyXmlFractorRule::class], + ]; + yield 'already formatted file reports no change' => ['already-formatted.xml.fixture', []]; + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/fractor.php'; + } +} diff --git a/packages/fractor-xml/tests/XmlProcessResult/config/fractor.php b/packages/fractor-xml/tests/XmlProcessResult/config/fractor.php new file mode 100644 index 00000000..dd110b47 --- /dev/null +++ b/packages/fractor-xml/tests/XmlProcessResult/config/fractor.php @@ -0,0 +1,15 @@ +withOptions([ + XmlProcessorOption::INDENT_CHARACTER => Indent::STYLE_TAB, + XmlProcessorOption::INDENT_SIZE => 1, + ]) + ->withRules([DummyXmlFractorRule::class]); diff --git a/packages/fractor-yaml/src/YamlFileProcessor.php b/packages/fractor-yaml/src/YamlFileProcessor.php index cf14c027..faeb6596 100644 --- a/packages/fractor-yaml/src/YamlFileProcessor.php +++ b/packages/fractor-yaml/src/YamlFileProcessor.php @@ -52,13 +52,17 @@ public function handle(File $file, iterable $appliedRules): void } } - // Nothing has changed. + // Never re-dump for formatting alone: the dumper drops comments, so + // untouched files stay byte-exact. if ($newYaml === $yaml) { $this->changedFilesDetector->addCachableFile($file->getFilePath()); return; } - $file->changeFileContent($this->yamlDumper->dump($newYaml, $indent)); + // Re-dumped original as baseline: the diff shows only the rule's change, + // any extra reformatting is attributed to CodeFormatRule by the runner. + $file->changeOriginalContent(rtrim($this->yamlDumper->dump($yaml, $indent)) . "\n"); + $file->changeFileContent(rtrim($this->yamlDumper->dump($newYaml, $indent)) . "\n"); } /** diff --git a/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/rule-and-reformat.yaml.fixture b/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/rule-and-reformat.yaml.fixture new file mode 100644 index 00000000..c63383fd --- /dev/null +++ b/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/rule-and-reformat.yaml.fixture @@ -0,0 +1,13 @@ +# this comment is lost when the file is re-dumped +Form: + renderingOptions: + translation: + translationFile: + 10: 'EXT:form/Resources/Private/Language/locallang.xlf' + 20: 'EXT:myextension/Resources/Private/Language/locallang.xlf' +----- +Form: + renderingOptions: + translation: + translationFiles: + 20: 'EXT:myextension/Resources/Private/Language/locallang.xlf' diff --git a/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/rule-only.yaml.fixture b/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/rule-only.yaml.fixture new file mode 100644 index 00000000..010077ab --- /dev/null +++ b/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/rule-only.yaml.fixture @@ -0,0 +1,12 @@ +Form: + renderingOptions: + translation: + translationFile: + 10: 'EXT:form/Resources/Private/Language/locallang.xlf' + 20: 'EXT:myextension/Resources/Private/Language/locallang.xlf' +----- +Form: + renderingOptions: + translation: + translationFiles: + 20: 'EXT:myextension/Resources/Private/Language/locallang.xlf' diff --git a/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/untouched-with-comment.yaml.fixture b/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/untouched-with-comment.yaml.fixture new file mode 100644 index 00000000..198c1967 --- /dev/null +++ b/packages/fractor-yaml/tests/YamlProcessResult/Fixtures/untouched-with-comment.yaml.fixture @@ -0,0 +1,4 @@ +# this comment survives because no rule rewrites the file +Form: + renderingOptions: + skinning: true diff --git a/packages/fractor-yaml/tests/YamlProcessResult/YamlProcessResultTest.php b/packages/fractor-yaml/tests/YamlProcessResult/YamlProcessResultTest.php new file mode 100644 index 00000000..4086c5cb --- /dev/null +++ b/packages/fractor-yaml/tests/YamlProcessResult/YamlProcessResultTest.php @@ -0,0 +1,52 @@ + $expectedAppliedRules + */ + #[DataProvider('provideData')] + public function test(string $fixture, array $expectedAppliedRules): void + { + $fractorTestResult = $this->doTestFile(__DIR__ . '/Fixtures/' . $fixture); + + self::assertSame($expectedAppliedRules, $fractorTestResult->getAppliedFractorRules()); + } + + /** + * @return \Iterator}> + */ + public static function provideData(): \Iterator + { + yield 'rule change on canonical input is attributed to the real rule' => [ + 'rule-only.yaml.fixture', + [DummyYamlFractorRule::class], + ]; + yield 'reformatting that rides along a rule is also attributed to the code-format rule' => [ + 'rule-and-reformat.yaml.fixture', + [AppliedRule::CODE_FORMAT_RULE, DummyYamlFractorRule::class], + ]; + yield 'a file no rule touches is left byte-exact' => ['untouched-with-comment.yaml.fixture', []]; + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/fractor.php'; + } +} diff --git a/packages/fractor-yaml/tests/YamlProcessResult/config/fractor.php b/packages/fractor-yaml/tests/YamlProcessResult/config/fractor.php new file mode 100644 index 00000000..61abaef3 --- /dev/null +++ b/packages/fractor-yaml/tests/YamlProcessResult/config/fractor.php @@ -0,0 +1,9 @@ +withRules([DummyYamlFractorRule::class]); diff --git a/packages/fractor/src/Application/FractorRunner.php b/packages/fractor/src/Application/FractorRunner.php index 5ae73ca0..1a0407d1 100644 --- a/packages/fractor/src/Application/FractorRunner.php +++ b/packages/fractor/src/Application/FractorRunner.php @@ -7,6 +7,7 @@ use a9f\Fractor\Application\Contract\FileProcessor; use a9f\Fractor\Application\Contract\FileWriter; use a9f\Fractor\Application\Contract\FractorRule; +use a9f\Fractor\Application\ValueObject\AppliedRule; use a9f\Fractor\Application\ValueObject\File; use a9f\Fractor\Caching\Detector\ChangedFilesDetector; use a9f\Fractor\Configuration\ConfigurationRuleFilter; @@ -65,7 +66,8 @@ public function run(Configuration $configuration): ProcessResult $totalChanged = 0; foreach ($filePaths as $filePath) { - $file = new File($filePath, FileSystem::read($filePath)); + $rawContent = FileSystem::read($filePath); + $file = new File($filePath, $rawContent); $this->fileCollector->addFile($file); if ($shouldShowProgressBar) { @@ -92,6 +94,13 @@ public function run(Configuration $configuration): ProcessResult ++$totalChanged; + // flag code formatting whenever the file was re-formatted: no real rule ran + // (pure reformat) or the normalized baseline differs from the raw input + // (reindent alongside a rule) + if ($file->getAppliedRules() === [] || $file->getOriginalContent() !== $rawContent) { + $file->addAppliedRule(AppliedRule::codeFormat()); + } + $file->setFileDiff($this->fileDiffFactory->createFileDiff($configuration->shouldShowDiffs(), $file)); $fileProcessResult = new FileProcessResult($file->getFileDiff()); diff --git a/packages/fractor/src/Application/ValueObject/AppliedRule.php b/packages/fractor/src/Application/ValueObject/AppliedRule.php index a387cfaa..85b92621 100644 --- a/packages/fractor/src/Application/ValueObject/AppliedRule.php +++ b/packages/fractor/src/Application/ValueObject/AppliedRule.php @@ -9,7 +9,12 @@ final readonly class AppliedRule { /** - * @param class-string $fractorClass + * Identifier for the virtual code-formatting rule, which has no backing class. + */ + public const CODE_FORMAT_RULE = 'CodeFormatRule'; + + /** + * @param class-string|self::CODE_FORMAT_RULE $fractorClass */ private function __construct( private string $fractorClass, @@ -29,11 +34,21 @@ public static function fromClassString(string $fractorRule): self return new self($fractorRule); } + public static function codeFormat(): self + { + return new self(self::CODE_FORMAT_RULE); + } + /** - * @return class-string + * @return class-string|self::CODE_FORMAT_RULE */ public function getFractorClass(): string { return $this->fractorClass; } + + public function isCodeFormat(): bool + { + return $this->fractorClass === self::CODE_FORMAT_RULE; + } } diff --git a/packages/fractor/src/ChangesReporting/Output/ConsoleOutputFormatter.php b/packages/fractor/src/ChangesReporting/Output/ConsoleOutputFormatter.php index 4f7c500f..0573d59d 100644 --- a/packages/fractor/src/ChangesReporting/Output/ConsoleOutputFormatter.php +++ b/packages/fractor/src/ChangesReporting/Output/ConsoleOutputFormatter.php @@ -71,7 +71,14 @@ private function reportFileDiffs(array $fileDiffs, bool $absoluteFilePath, bool $message = \sprintf('%d) %s', ++$i, $filePath); $this->symfonyStyle->writeln($message); $this->symfonyStyle->newLine(); - $this->symfonyStyle->writeln($fileDiff->getDiffConsoleFormatted()); + + // reformatting/normalization only: show a one-liner instead of dumping the whole file + if ($fileDiff->isCodeFormatOnly()) { + $this->symfonyStyle->writeln(' ~ reformatted / normalized'); + $this->symfonyStyle->newLine(); + } else { + $this->symfonyStyle->writeln($fileDiff->getDiffConsoleFormatted()); + } if ($fileDiff->getAppliedRules() !== []) { $this->symfonyStyle->writeln('Applied rules:'); @@ -111,7 +118,7 @@ private function reportRulesSummary(ProcessResult $processResult, Configuration $this->symfonyStyle->section('Rules Summary'); foreach ($ruleApplicationCounts as $ruleClass => $count) { - $ruleShortClass = (string) Strings::after($ruleClass, '\\', -1); + $ruleShortClass = Strings::after($ruleClass, '\\', -1) ?? $ruleClass; $this->symfonyStyle->writeln(sprintf( ' * %s %s %d time%s', $ruleShortClass, diff --git a/packages/fractor/src/Differ/ValueObject/FileDiff.php b/packages/fractor/src/Differ/ValueObject/FileDiff.php index 03f25db6..7212d975 100644 --- a/packages/fractor/src/Differ/ValueObject/FileDiff.php +++ b/packages/fractor/src/Differ/ValueObject/FileDiff.php @@ -79,13 +79,14 @@ public function getFractorShortClasses(): array { $fractorShortClasses = []; foreach ($this->getFractorClasses() as $fractorClass) { - $fractorShortClasses[] = (string) Strings::after($fractorClass, '\\', -1); + // fall back to the full identifier for labels without a namespace (e.g. the code-format rule) + $fractorShortClasses[] = Strings::after($fractorClass, '\\', -1) ?? $fractorClass; } return $fractorShortClasses; } /** - * @return array> + * @return array|AppliedRule::CODE_FORMAT_RULE> */ public function getFractorClasses(): array { @@ -98,6 +99,19 @@ public function getFractorClasses(): array return $fractorClasses; } + public function isCodeFormatOnly(): bool + { + if ($this->appliedRules === []) { + return false; + } + foreach ($this->appliedRules as $appliedRule) { + if (! $appliedRule->isCodeFormat()) { + return false; + } + } + return true; + } + public function getFirstLineNumber(): ?int { $match = Strings::match($this->diff, self::FIRST_LINE_REGEX); diff --git a/packages/fractor/src/Reporting/ChangelogExtractor.php b/packages/fractor/src/Reporting/ChangelogExtractor.php index 41be0f2d..db60f506 100644 --- a/packages/fractor/src/Reporting/ChangelogExtractor.php +++ b/packages/fractor/src/Reporting/ChangelogExtractor.php @@ -5,16 +5,22 @@ namespace a9f\Fractor\Reporting; use a9f\Fractor\Application\Contract\FractorRule; +use a9f\Fractor\Application\ValueObject\AppliedRule; use Nette\Utils\Strings; use Webmozart\Assert\Assert; final class ChangelogExtractor { /** - * @param class-string $ruleClassName + * @param class-string|AppliedRule::CODE_FORMAT_RULE $ruleClassName */ public function extractChangelogFromRule(string $ruleClassName): ?string { + // the virtual code-formatting rule has no backing class to reflect on + if ($ruleClassName === AppliedRule::CODE_FORMAT_RULE) { + return null; + } + Assert::isAOf($ruleClassName, FractorRule::class); $reflectionClass = new \ReflectionClass($ruleClassName); diff --git a/packages/fractor/src/Reporting/FractorsChangelogLinesResolver.php b/packages/fractor/src/Reporting/FractorsChangelogLinesResolver.php index 1988068d..6715b191 100644 --- a/packages/fractor/src/Reporting/FractorsChangelogLinesResolver.php +++ b/packages/fractor/src/Reporting/FractorsChangelogLinesResolver.php @@ -24,7 +24,7 @@ public function createFractorChangelogLines(array $appliedRules): array $fractorsChangelogsLines = []; foreach ($rectorsChangelogs as $fractorClass => $changelog) { - $fractorShortClass = (string) Strings::after($fractorClass, '\\', -1); + $fractorShortClass = Strings::after($fractorClass, '\\', -1) ?? $fractorClass; $fractorsChangelogsLines[] = $changelog === null ? $fractorShortClass : $fractorShortClass . ' (' . $changelog . ')'; } diff --git a/packages/fractor/src/Reporting/FractorsChangelogResolver.php b/packages/fractor/src/Reporting/FractorsChangelogResolver.php index 8d2d82eb..4383585a 100644 --- a/packages/fractor/src/Reporting/FractorsChangelogResolver.php +++ b/packages/fractor/src/Reporting/FractorsChangelogResolver.php @@ -4,6 +4,7 @@ namespace a9f\Fractor\Reporting; +use a9f\Fractor\Application\Contract\FractorRule; use a9f\Fractor\Application\ValueObject\AppliedRule; final readonly class FractorsChangelogResolver @@ -15,7 +16,7 @@ public function __construct( /** * @param AppliedRule[] $appliedRules - * @return array + * @return array|AppliedRule::CODE_FORMAT_RULE, string|null> */ public function resolveIncludingMissing(array $appliedRules): array { diff --git a/packages/fractor/src/Testing/PHPUnit/AbstractFractorTestCase.php b/packages/fractor/src/Testing/PHPUnit/AbstractFractorTestCase.php index b255f1c3..fbb956cd 100644 --- a/packages/fractor/src/Testing/PHPUnit/AbstractFractorTestCase.php +++ b/packages/fractor/src/Testing/PHPUnit/AbstractFractorTestCase.php @@ -89,7 +89,7 @@ protected static function yieldFilesFromDirectory(string $directory, string $suf return FixtureFileFinder::yieldDirectory($directory, $suffix); } - protected function doTestFile(string $fixtureFilePath): void + protected function doTestFile(string $fixtureFilePath): FractorTestResult { // prepare input file contents and expected file output contents $fixtureFileContents = FileSystem::read($fixtureFilePath); @@ -115,7 +115,7 @@ protected function doTestFile(string $fixtureFilePath): void // write temp file FileSystem::write($inputFilePath, $inputFileContents, null); - $this->doTestFileMatchesExpectedContent($inputFilePath, $expectedFileContents, $fixtureFilePath); + return $this->doTestFileMatchesExpectedContent($inputFilePath, $expectedFileContents, $fixtureFilePath); } /** @@ -144,7 +144,7 @@ private function doTestFileMatchesExpectedContent( string $originalFilePath, string $expectedFileContents, string $fixtureFilePath - ): void { + ): FractorTestResult { // the file is now changed (if any rule matches) $fractorTestResult = $this->processFilePath($originalFilePath); $changedContents = $fractorTestResult->getChangedContents(); @@ -161,6 +161,8 @@ private function doTestFileMatchesExpectedContent( } self::assertSame(trim($expectedFileContents), trim($changedContents), $failureMessage); + + return $fractorTestResult; } /** diff --git a/packages/fractor/src/Testing/PHPUnit/ValueObject/FractorTestResult.php b/packages/fractor/src/Testing/PHPUnit/ValueObject/FractorTestResult.php index 301cb360..fb05bfc0 100644 --- a/packages/fractor/src/Testing/PHPUnit/ValueObject/FractorTestResult.php +++ b/packages/fractor/src/Testing/PHPUnit/ValueObject/FractorTestResult.php @@ -5,6 +5,7 @@ namespace a9f\Fractor\Testing\PHPUnit\ValueObject; use a9f\Fractor\Application\Contract\FractorRule; +use a9f\Fractor\Application\ValueObject\AppliedRule; use a9f\Fractor\ValueObject\ProcessResult; final readonly class FractorTestResult @@ -21,7 +22,7 @@ public function getChangedContents(): string } /** - * @return array> + * @return array|AppliedRule::CODE_FORMAT_RULE> */ public function getAppliedFractorRules(): array { diff --git a/packages/fractor/src/ValueObject/ProcessResult.php b/packages/fractor/src/ValueObject/ProcessResult.php index 304f7e22..02798b70 100644 --- a/packages/fractor/src/ValueObject/ProcessResult.php +++ b/packages/fractor/src/ValueObject/ProcessResult.php @@ -5,6 +5,7 @@ namespace a9f\Fractor\ValueObject; use a9f\Fractor\Application\Contract\FractorRule; +use a9f\Fractor\Application\ValueObject\AppliedRule; use a9f\Fractor\Differ\ValueObject\FileDiff; use Webmozart\Assert\Assert; @@ -26,7 +27,10 @@ public function __construct( public function getFileDiffs(bool $onlyWithChanges = true): array { if ($onlyWithChanges) { - return array_filter($this->fileDiffs, static fn (FileDiff $fileDiff): bool => $fileDiff->getDiff() !== ''); + return array_filter( + $this->fileDiffs, + static fn (FileDiff $fileDiff): bool => $fileDiff->getDiff() !== '' || $fileDiff->getAppliedRules() !== [] + ); } return $this->fileDiffs; } @@ -37,7 +41,7 @@ public function getTotalChanged(): int } /** - * @return array, int> + * @return array|AppliedRule::CODE_FORMAT_RULE, int> */ public function getRuleApplicationCounts(): array { diff --git a/packages/fractor/tests/Application/CodeFormatRule/CodeFormatRuleTest.php b/packages/fractor/tests/Application/CodeFormatRule/CodeFormatRuleTest.php new file mode 100644 index 00000000..5430b610 --- /dev/null +++ b/packages/fractor/tests/Application/CodeFormatRule/CodeFormatRuleTest.php @@ -0,0 +1,54 @@ + $expectedAppliedRules + */ + #[DataProvider('provideData')] + public function test(string $fixture, array $expectedAppliedRules): void + { + $fractorTestResult = $this->doTestFile(__DIR__ . '/Fixtures/' . $fixture); + + self::assertSame($expectedAppliedRules, $fractorTestResult->getAppliedFractorRules()); + } + + /** + * @return \Iterator}> + */ + public static function provideData(): \Iterator + { + // Trailing whitespace gets trimmed by a rule that records nothing: the + // runner must recognise the change as code formatting on its own. + yield 'reformat without a real rule is attributed to the code-format rule' => [ + 'trailing-whitespace.txt.fixture', + [AppliedRule::CODE_FORMAT_RULE], + ]; + // Nothing to trim: no change is reported and no rule is applied. + yield 'already clean file reports no change' => ['clean.txt.fixture', []]; + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/fractor.php'; + } + + protected function additionalConfigurationFiles(): array + { + return [__DIR__ . '/../FractorRunner/config/config.php']; + } +} diff --git a/packages/fractor/tests/Application/CodeFormatRule/Fixtures/clean.txt.fixture b/packages/fractor/tests/Application/CodeFormatRule/Fixtures/clean.txt.fixture new file mode 100644 index 00000000..06fcdd77 --- /dev/null +++ b/packages/fractor/tests/Application/CodeFormatRule/Fixtures/clean.txt.fixture @@ -0,0 +1,2 @@ +first line +second line diff --git a/packages/fractor/tests/Application/CodeFormatRule/Fixtures/trailing-whitespace.txt.fixture b/packages/fractor/tests/Application/CodeFormatRule/Fixtures/trailing-whitespace.txt.fixture new file mode 100644 index 00000000..8138233e --- /dev/null +++ b/packages/fractor/tests/Application/CodeFormatRule/Fixtures/trailing-whitespace.txt.fixture @@ -0,0 +1,7 @@ +first line +second line +third line +----- +first line +second line +third line diff --git a/packages/fractor/tests/Application/CodeFormatRule/config/fractor.php b/packages/fractor/tests/Application/CodeFormatRule/config/fractor.php new file mode 100644 index 00000000..c543a256 --- /dev/null +++ b/packages/fractor/tests/Application/CodeFormatRule/config/fractor.php @@ -0,0 +1,9 @@ +withRules([NormalizeWhitespaceTextRule::class]); diff --git a/packages/fractor/tests/Application/ValueObject/AppliedRule/AppliedRuleTest.php b/packages/fractor/tests/Application/ValueObject/AppliedRule/AppliedRuleTest.php new file mode 100644 index 00000000..ad7b7901 --- /dev/null +++ b/packages/fractor/tests/Application/ValueObject/AppliedRule/AppliedRuleTest.php @@ -0,0 +1,27 @@ +getFractorClass()); + self::assertTrue($appliedRule->isCodeFormat()); + } + + public function testRealRuleIsNotCodeFormat(): void + { + $appliedRule = AppliedRule::fromClassString(RuleWithNoChangelog::class); + + self::assertFalse($appliedRule->isCodeFormat()); + } +} diff --git a/packages/fractor/tests/Fixture/DummyProcessor/Rules/NormalizeWhitespaceTextRule.php b/packages/fractor/tests/Fixture/DummyProcessor/Rules/NormalizeWhitespaceTextRule.php new file mode 100644 index 00000000..a88cbb05 --- /dev/null +++ b/packages/fractor/tests/Fixture/DummyProcessor/Rules/NormalizeWhitespaceTextRule.php @@ -0,0 +1,33 @@ +getContent()); + $trimmedLines = array_map(rtrim(...), $lines); + + $file->changeFileContent(implode("\n", $trimmedLines)); + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition('Trim trailing whitespace from every line', [new CodeSample("a \nb ", "a\nb")]); + } +} diff --git a/packages/fractor/tests/Reporting/ChangelogExtractor/ChangelogExtractorTest.php b/packages/fractor/tests/Reporting/ChangelogExtractor/ChangelogExtractorTest.php index 6a1a5dbd..aaf41788 100644 --- a/packages/fractor/tests/Reporting/ChangelogExtractor/ChangelogExtractorTest.php +++ b/packages/fractor/tests/Reporting/ChangelogExtractor/ChangelogExtractorTest.php @@ -4,6 +4,7 @@ namespace a9f\Fractor\Tests\Reporting\ChangelogExtractor; +use a9f\Fractor\Application\ValueObject\AppliedRule; use a9f\Fractor\Reporting\ChangelogExtractor; use a9f\Fractor\Tests\Reporting\ChangelogExtractor\Fixture\RuleWithChangelog; use a9f\Fractor\Tests\Reporting\ChangelogExtractor\Fixture\RuleWithNoChangelog; @@ -26,6 +27,13 @@ public function ruleWithNoChangelogDocBlock(): void self::assertNull($this->subject->extractChangelogFromRule(RuleWithNoChangelog::class)); } + #[Test] + public function virtualCodeFormatRuleIsToleratedWithoutReflection(): void + { + // the code-format label has no backing class; it must not be reflected on + self::assertNull($this->subject->extractChangelogFromRule(AppliedRule::CODE_FORMAT_RULE)); + } + #[Test] public function ruleWithChangelog(): void {