From 645904f26ff0e2ca5ac28e4e4c813cccd083510a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 11 Aug 2025 08:30:23 +0200 Subject: [PATCH 1/3] [ci] check extra after part in fixture, that show no change --- .github/workflows/code_analysis.yaml | 4 ++ bin/check-before-after-same-fixtures.php | 49 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 bin/check-before-after-same-fixtures.php diff --git a/.github/workflows/code_analysis.yaml b/.github/workflows/code_analysis.yaml index c9877225baf..cdf33ac9ebf 100644 --- a/.github/workflows/code_analysis.yaml +++ b/.github/workflows/code_analysis.yaml @@ -52,6 +52,10 @@ jobs: name: 'Finalize classes' run: vendor/bin/swiss-knife finalize-classes src tests --dry-run + - + name: 'Check before/after test fixture on no-changes' + run: php bin/check-before-after-same-fixtures.php + - name: 'Detect composer dependency issues' run: vendor/bin/composer-dependency-analyser diff --git a/bin/check-before-after-same-fixtures.php b/bin/check-before-after-same-fixtures.php new file mode 100644 index 00000000000..670d0908cf1 --- /dev/null +++ b/bin/check-before-after-same-fixtures.php @@ -0,0 +1,49 @@ +files() + ->in([__DIR__ . '/../tests', __DIR__ . '/../rules-tests']) + ->name('*.php.inc') + ->getIterator(); + +foreach ($testFixtureFiles as $testFixtureFile) { + if (! str_contains($testFixtureFile->getContents(), '-----')) { + continue; + } + + $parts = preg_split('/^\s*-{5,}\s*$/m', file_get_contents($testFixtureFile->getRealPath())); + if (count($parts) !== 2) { + continue; + } + + if (trim($parts[0]) !== trim($parts[1])) { + continue; + } + + $invalidFixturePaths[] = $testFixtureFile->getRealPath(); +} + +if ($invalidFixturePaths === []) { + $symfonyStyle->success('All fixtures are valid'); + exit(Command::SUCCESS); +} + +$symfonyStyle->error('The following fixtures have the same before and after content. Remove the part after "-----" to fix them'); + +$symfonyStyle->listing($invalidFixturePaths); + +exit(Command::FAILURE); From d4fa00b61830fff171fd42408c414d5f6023d087 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 11 Aug 2025 08:33:49 +0200 Subject: [PATCH 2/3] cleanup fixtures with no change --- bin/check-before-after-same-fixtures.php | 96 ++++++++++----- bin/no-php-file-in-fixtures.php | 112 ++++++++++++++++++ .../Fixture/static_property.php.inc | 19 --- .../Fixture/final_private_constructor.php.inc | 15 --- ...offset_variable_zero_equal_strpos.php.inc} | 0 .../Fixture/skip_different_cases.php.inc | 28 ----- .../RenameCastRector/RenameCastRectorTest.php | 4 +- .../FixtureUnion/array_reverse_index.php.inc | 28 ----- .../return_inside_inner_function.php.inc | 23 ---- .../RemoveFinfoBufferContextArgRector.php | 23 ++-- .../Renaming/Rector/Cast/RenameCastRector.php | 8 +- 11 files changed, 196 insertions(+), 160 deletions(-) create mode 100644 bin/no-php-file-in-fixtures.php rename rules-tests/Php80/Rector/NotIdentical/StrContainsRector/Fixture/{offset_variable_zero_equal_strpos.php => offset_variable_zero_equal_strpos.php.inc} (100%) diff --git a/bin/check-before-after-same-fixtures.php b/bin/check-before-after-same-fixtures.php index 670d0908cf1..55a6557041e 100644 --- a/bin/check-before-after-same-fixtures.php +++ b/bin/check-before-after-same-fixtures.php @@ -2,48 +2,88 @@ declare(strict_types=1); +use Nette\Utils\Strings; + require __DIR__ . '/../vendor/autoload.php'; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\ArgvInput; +use Symfony\Component\Console\Output\ConsoleOutput; +use Symfony\Component\Console\Style\SymfonyStyle; use Symfony\Component\Finder\Finder; +use Symfony\Component\Finder\SplFileInfo; +use Webmozart\Assert\Assert; -$invalidFixturePaths = []; +final readonly class SameBeforeAfterFixtureDetector +{ + private SymfonyStyle $symfonyStyle; -$symfonyStyle = new \Symfony\Component\Console\Style\SymfonyStyle( - new \Symfony\Component\Console\Input\ArgvInput(), - new \Symfony\Component\Console\Output\ConsoleOutput() -); + public function __construct() + { + $this->symfonyStyle = new SymfonyStyle(new ArgvInput(), new ConsoleOutput()); + } -$testFixtureFiles = (new Finder()) - ->files() - ->in([__DIR__ . '/../tests', __DIR__ . '/../rules-tests']) - ->name('*.php.inc') - ->getIterator(); + /** + * @param string[] $testDirectories + * @return Command::SUCCESS|Command::FAILURE + */ + public function run(array $testDirectories): int + { + $fixtureFiles = $this->findFixtureFiles($testDirectories); -foreach ($testFixtureFiles as $testFixtureFile) { - if (! str_contains($testFixtureFile->getContents(), '-----')) { - continue; - } + $invalidFixturePaths = []; + foreach ($fixtureFiles as $fixtureFile) { + if (! $this->hasFileSameBeforeAndAfterPart($fixtureFile)) { + continue; + } + + $invalidFixturePaths[] = substr($fixtureFile->getRealPath(), strlen(getcwd()) + 1); + } + + if ($invalidFixturePaths === []) { + $this->symfonyStyle->success('All fixtures are valid'); + return Command::SUCCESS; + } + + $this->symfonyStyle->error( + 'The following fixtures have the same before and after content. Remove the part after "-----" to fix them' + ); + $this->symfonyStyle->listing($invalidFixturePaths); - $parts = preg_split('/^\s*-{5,}\s*$/m', file_get_contents($testFixtureFile->getRealPath())); - if (count($parts) !== 2) { - continue; + return Command::FAILURE; } - if (trim($parts[0]) !== trim($parts[1])) { - continue; + /** + * @param string[] $directories + * @return SplFileInfo[] + */ + private function findFixtureFiles(array $directories): array + { + Assert::allDirectory($directories); + + $finder = (new Finder()) + ->files() + ->in($directories) + ->name('*.php.inc') + ->sortByName(); + + return iterator_to_array($finder->getIterator()); } - $invalidFixturePaths[] = $testFixtureFile->getRealPath(); -} + private function hasFileSameBeforeAndAfterPart(SplFileInfo $fixtureFile): bool + { + $parts = Strings::split($fixtureFile->getContents(), '#^\s*-----\s*$#m'); + if (count($parts) !== 2) { + return false; + } -if ($invalidFixturePaths === []) { - $symfonyStyle->success('All fixtures are valid'); - exit(Command::SUCCESS); + return trim((string) $parts[0]) === trim((string) $parts[1]); + } } -$symfonyStyle->error('The following fixtures have the same before and after content. Remove the part after "-----" to fix them'); - -$symfonyStyle->listing($invalidFixturePaths); -exit(Command::FAILURE); +$sameBeforeAfterFixtureDetector = new SameBeforeAfterFixtureDetector(); +exit($sameBeforeAfterFixtureDetector->run([ + __DIR__ . '/../tests', + __DIR__ . '/../rules-tests', +])); diff --git a/bin/no-php-file-in-fixtures.php b/bin/no-php-file-in-fixtures.php new file mode 100644 index 00000000000..9dddea51023 --- /dev/null +++ b/bin/no-php-file-in-fixtures.php @@ -0,0 +1,112 @@ +symfonyStyle = new SymfonyStyle(new ArgvInput(), new ConsoleOutput()); + } + + /** + * @param string[] $testDirectories + * @return Command::SUCCESS|Command::FAILURE + */ + public function run(array $testDirectories): int + { + $phpFiles = $this->findPhpFiles($testDirectories); + + $allFixtureFiles = $this->findFixtureFiles($testDirectories); + + $relativePhpFiles = []; + foreach ($phpFiles as $phpFile) { + $relativeFilePath = substr($phpFile->getRealPath(), strlen(getcwd()) + 1); + + // should skip? + if (in_array($relativeFilePath, self::EXCLUDED_FILES, true)) { + continue; + } + + $relativePhpFiles[] = $relativeFilePath; + } + + if ($relativePhpFiles === []) { + $this->symfonyStyle->success(sprintf('All %d fixtures are valid', count($allFixtureFiles))); + return Command::SUCCESS; + } + + $this->symfonyStyle->error( + 'The following "*.php* files were found in /Fixtures directory, but only "*.php.inc" files are picked up and allowed. Rename their suffix or remove them' + ); + $this->symfonyStyle->listing($relativePhpFiles); + + return Command::FAILURE; + } + + /** + * @param string[] $directories + * @return SplFileInfo[] + */ + private function findPhpFiles(array $directories): array + { + Assert::allDirectory($directories); + + $finder = (new Finder()) + ->files() + ->in($directories) + ->path('/Fixture') + ->path('/Fixture*') + ->notPath('Source') + ->name('*.php') + ->sortByName(); + + return iterator_to_array($finder->getIterator()); + } + + /** + * @param string[] $directories + * @return SplFileInfo[] + */ + private function findFixtureFiles(array $directories): array + { + Assert::allDirectory($directories); + + $finder = (new Finder()) + ->files() + ->in($directories) + ->path('Fixture') + ->path('Fixture*') + ->notPath('Source') + ->sortByName(); + + return iterator_to_array($finder->getIterator()); + } +} + + +$noPhpFileInFixturesDetector = new NoPhpFileInFixturesDetector(); + +exit($noPhpFileInFixturesDetector->run([ + __DIR__ . '/../rules-tests', +])); diff --git a/rules-tests/DeadCode/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/static_property.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/static_property.php.inc index 498d2a54226..9b49c802aa7 100644 --- a/rules-tests/DeadCode/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/static_property.php.inc +++ b/rules-tests/DeadCode/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/static_property.php.inc @@ -12,22 +12,3 @@ class StaticProperty return $this::$privateProperty; } } - -?> ------ - diff --git a/rules-tests/Php80/Rector/ClassMethod/FinalPrivateToPrivateVisibilityRector/Fixture/final_private_constructor.php.inc b/rules-tests/Php80/Rector/ClassMethod/FinalPrivateToPrivateVisibilityRector/Fixture/final_private_constructor.php.inc index a9a2a78a86c..069bf70a364 100644 --- a/rules-tests/Php80/Rector/ClassMethod/FinalPrivateToPrivateVisibilityRector/Fixture/final_private_constructor.php.inc +++ b/rules-tests/Php80/Rector/ClassMethod/FinalPrivateToPrivateVisibilityRector/Fixture/final_private_constructor.php.inc @@ -8,18 +8,3 @@ abstract class FinalPrivate { } } - -?> ------ - diff --git a/rules-tests/Php80/Rector/NotIdentical/StrContainsRector/Fixture/offset_variable_zero_equal_strpos.php b/rules-tests/Php80/Rector/NotIdentical/StrContainsRector/Fixture/offset_variable_zero_equal_strpos.php.inc similarity index 100% rename from rules-tests/Php80/Rector/NotIdentical/StrContainsRector/Fixture/offset_variable_zero_equal_strpos.php rename to rules-tests/Php80/Rector/NotIdentical/StrContainsRector/Fixture/offset_variable_zero_equal_strpos.php.inc diff --git a/rules-tests/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector/Fixture/skip_different_cases.php.inc b/rules-tests/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector/Fixture/skip_different_cases.php.inc index 010068cba81..cbc6fc21a85 100644 --- a/rules-tests/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector/Fixture/skip_different_cases.php.inc +++ b/rules-tests/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector/Fixture/skip_different_cases.php.inc @@ -21,31 +21,3 @@ function bar($otherObject): void $finfo->buffer($fileContents, FILEINFO_NONE, context: $context); $finfo->buffer($fileContents, flags: FILEINFO_NONE, context: $context); } - -?> ------ -file($fileContents, FILEINFO_NONE, $context); - $finfo->file($fileContents, FILEINFO_NONE, context: $context); - $finfo->file($fileContents, flags: FILEINFO_NONE, context: $context); -} - -function bar($otherObject): void -{ - $finfo->buffer($fileContents, FILEINFO_NONE, $context); - $finfo->buffer($fileContents, FILEINFO_NONE, context: $context); - $finfo->buffer($fileContents, flags: FILEINFO_NONE, context: $context); -} - -?> diff --git a/rules-tests/Renaming/Rector/Cast/RenameCastRector/RenameCastRectorTest.php b/rules-tests/Renaming/Rector/Cast/RenameCastRector/RenameCastRectorTest.php index 4bd7bbf3d58..e6d60a6166e 100644 --- a/rules-tests/Renaming/Rector/Cast/RenameCastRector/RenameCastRectorTest.php +++ b/rules-tests/Renaming/Rector/Cast/RenameCastRector/RenameCastRectorTest.php @@ -1,12 +1,14 @@ ------ - $uri) { - if ($index === 0) { - continue; - } - - $this->someOtherMethod($index, $uri); - } - } - - private function someOtherMethod(int $index, string $uri) - { - return sprintf('%d-%s', $index, $uri); - } -} - -?> diff --git a/rules-tests/TypeDeclaration/Rector/Closure/AddClosureVoidReturnTypeWhereNoReturnRector/Fixture/return_inside_inner_function.php.inc b/rules-tests/TypeDeclaration/Rector/Closure/AddClosureVoidReturnTypeWhereNoReturnRector/Fixture/return_inside_inner_function.php.inc index 4ae3e7826c9..6b98d44c850 100644 --- a/rules-tests/TypeDeclaration/Rector/Closure/AddClosureVoidReturnTypeWhereNoReturnRector/Fixture/return_inside_inner_function.php.inc +++ b/rules-tests/TypeDeclaration/Rector/Closure/AddClosureVoidReturnTypeWhereNoReturnRector/Fixture/return_inside_inner_function.php.inc @@ -16,26 +16,3 @@ final class ReturnInsideInnerFunction }; } } - -?> ------ - diff --git a/rules/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector.php b/rules/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector.php index ac625bdc26c..70604712e08 100644 --- a/rules/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector.php +++ b/rules/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector.php @@ -6,7 +6,6 @@ use PhpParser\Node; use PhpParser\Node\Arg; -use PhpParser\Node\Expr; use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; @@ -55,14 +54,10 @@ public function getNodeTypes(): array } /** - * @param FuncCall $node + * @param MethodCall|FuncCall $node */ public function refactor(Node $node): ?Node { - if ($node->name instanceof Expr) { - return null; - } - if ($node instanceof FuncCall && ! $this->isName($node->name, 'finfo_buffer')) { return null; } @@ -88,29 +83,29 @@ public function provideMinPhpVersion(): int } /** - * @param FuncCall|MethodCall $funcCall + * @param FuncCall|MethodCall $callLike */ - private function removeContextArg(CallLike $funcCall): bool + private function removeContextArg(CallLike $callLike): bool { // In `finfo::buffer` method calls, the first parameter, compared to `finfo_buffer`, does not exist. $methodArgCorrection = 0; - if ($funcCall instanceof MethodCall) { + if ($callLike instanceof MethodCall) { $methodArgCorrection = -1; } - if (count($funcCall->args) <= 2 + $methodArgCorrection) { + if (count($callLike->args) <= 2 + $methodArgCorrection) { return false; } // Cannot handle variadic args - foreach ($funcCall->args as $position => $arg) { + foreach ($callLike->args as $position => $arg) { if (! $arg instanceof Arg) { return false; } } /** @var array $args */ - $args = $funcCall->args; + $args = $callLike->args; // Argument 3 ($flags) and argument 4 ($context) are optional, thus named parameters must be considered if (! $this->argsAnalyzer->hasNamedArg($args)) { @@ -118,14 +113,14 @@ private function removeContextArg(CallLike $funcCall): bool return false; } - unset($funcCall->args[3 + $methodArgCorrection]); + unset($callLike->args[3 + $methodArgCorrection]); return true; } foreach ($args as $position => $arg) { if ($arg->name instanceof Identifier && $arg->name->name === 'context') { - unset($funcCall->args[$position]); + unset($callLike->args[$position]); return true; } diff --git a/rules/Renaming/Rector/Cast/RenameCastRector.php b/rules/Renaming/Rector/Cast/RenameCastRector.php index eb977fa53cf..e7b17d2f7b9 100644 --- a/rules/Renaming/Rector/Cast/RenameCastRector.php +++ b/rules/Renaming/Rector/Cast/RenameCastRector.php @@ -51,10 +51,10 @@ public function refactor(Node $node): ?Node { foreach ($this->renameCasts as $renameCast) { $expectedClassName = $renameCast->getFromCastExprClass(); - if ( - ! $node instanceof $expectedClassName - || $node->getAttribute(AttributeKey::KIND) !== $renameCast->getFromCastKind() - ) { + if (! $node instanceof $expectedClassName) { + continue; + } + if ($node->getAttribute(AttributeKey::KIND) !== $renameCast->getFromCastKind()) { continue; } From cceab77838b62324a3b77f7d84027c85fe146d64 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 11 Aug 2025 09:19:38 +0200 Subject: [PATCH 3/3] [ci] add script that checks no PHP files in /Fixture directory --- .github/workflows/code_analysis.yaml | 4 ++++ bin/check-before-after-same-fixtures.php | 6 +----- bin/no-php-file-in-fixtures.php | 5 +---- rules/Renaming/Rector/Cast/RenameCastRector.php | 1 + 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.github/workflows/code_analysis.yaml b/.github/workflows/code_analysis.yaml index cdf33ac9ebf..3c9dec53363 100644 --- a/.github/workflows/code_analysis.yaml +++ b/.github/workflows/code_analysis.yaml @@ -56,6 +56,10 @@ jobs: name: 'Check before/after test fixture on no-changes' run: php bin/check-before-after-same-fixtures.php + - + name: 'Check no "*.php" files in rules Fixture directory' + run: php bin/no-php-file-in-fixtures.php + - name: 'Detect composer dependency issues' run: vendor/bin/composer-dependency-analyser diff --git a/bin/check-before-after-same-fixtures.php b/bin/check-before-after-same-fixtures.php index 55a6557041e..5b4822a6bce 100644 --- a/bin/check-before-after-same-fixtures.php +++ b/bin/check-before-after-same-fixtures.php @@ -81,9 +81,5 @@ private function hasFileSameBeforeAndAfterPart(SplFileInfo $fixtureFile): bool } } - $sameBeforeAfterFixtureDetector = new SameBeforeAfterFixtureDetector(); -exit($sameBeforeAfterFixtureDetector->run([ - __DIR__ . '/../tests', - __DIR__ . '/../rules-tests', -])); +exit($sameBeforeAfterFixtureDetector->run([__DIR__ . '/../tests', __DIR__ . '/../rules-tests'])); diff --git a/bin/no-php-file-in-fixtures.php b/bin/no-php-file-in-fixtures.php index 9dddea51023..d356a310d4e 100644 --- a/bin/no-php-file-in-fixtures.php +++ b/bin/no-php-file-in-fixtures.php @@ -104,9 +104,6 @@ private function findFixtureFiles(array $directories): array } } - $noPhpFileInFixturesDetector = new NoPhpFileInFixturesDetector(); -exit($noPhpFileInFixturesDetector->run([ - __DIR__ . '/../rules-tests', -])); +exit($noPhpFileInFixturesDetector->run([__DIR__ . '/../rules-tests'])); diff --git a/rules/Renaming/Rector/Cast/RenameCastRector.php b/rules/Renaming/Rector/Cast/RenameCastRector.php index e7b17d2f7b9..6bf68083b75 100644 --- a/rules/Renaming/Rector/Cast/RenameCastRector.php +++ b/rules/Renaming/Rector/Cast/RenameCastRector.php @@ -54,6 +54,7 @@ public function refactor(Node $node): ?Node if (! $node instanceof $expectedClassName) { continue; } + if ($node->getAttribute(AttributeKey::KIND) !== $renameCast->getFromCastKind()) { continue; }