From d7cd59651f19c1e6709cb29f3885226ab6adfe33 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 26 Sep 2019 12:45:54 +0200 Subject: [PATCH 1/4] decouple SetProvider --- src/Console/Command/SetsCommand.php | 30 +++++++++-------------------- src/Set/SetProvider.php | 26 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 src/Set/SetProvider.php diff --git a/src/Console/Command/SetsCommand.php b/src/Console/Command/SetsCommand.php index b750591e88db..231ed35e52af 100644 --- a/src/Console/Command/SetsCommand.php +++ b/src/Console/Command/SetsCommand.php @@ -4,11 +4,11 @@ use Nette\Utils\Strings; use Rector\Console\Shell; +use Rector\Set\SetProvider; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; -use Symfony\Component\Finder\Finder; use Symplify\PackageBuilder\Console\Command\CommandNaming; final class SetsCommand extends AbstractCommand @@ -18,9 +18,15 @@ final class SetsCommand extends AbstractCommand */ private $symfonyStyle; - public function __construct(SymfonyStyle $symfonyStyle) + /** + * @var SetProvider + */ + private $setProvider; + + public function __construct(SymfonyStyle $symfonyStyle, SetProvider $setProvider) { $this->symfonyStyle = $symfonyStyle; + $this->setProvider = $setProvider; parent::__construct(); } @@ -34,7 +40,7 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { - $sets = $this->getAvailbleSets(); + $sets = $this->setProvider->provide(); if ($input->getArgument('name')) { $sets = $this->filterSetsByName($input, $sets); @@ -46,24 +52,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Shell::CODE_SUCCESS; } - /** - * @return string[] - */ - private function getAvailbleSets(): array - { - $finder = Finder::create()->files() - ->in(__DIR__ . '/../../../config/set'); - - $sets = []; - foreach ($finder->getIterator() as $fileInfo) { - $sets[] = $fileInfo->getBasename('.' . $fileInfo->getExtension()); - } - - sort($sets); - - return array_unique($sets); - } - /** * @param string[] $sets * @return string[] diff --git a/src/Set/SetProvider.php b/src/Set/SetProvider.php new file mode 100644 index 000000000000..7206e0884496 --- /dev/null +++ b/src/Set/SetProvider.php @@ -0,0 +1,26 @@ +files() + ->in(__DIR__ . '/../../config/set'); + + $sets = []; + foreach ($finder->getIterator() as $fileInfo) { + $sets[] = $fileInfo->getBasename('.' . $fileInfo->getExtension()); + } + + sort($sets); + + return array_unique($sets); + } +} From 2289c0d873919ea048f198e0b71d007e1b249b4c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 26 Sep 2019 12:48:28 +0200 Subject: [PATCH 2/4] [CI] run all sets check --- .travis.yml | 1 + bin/run_all_sets.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 bin/run_all_sets.php diff --git a/.travis.yml b/.travis.yml index 6835dc6fd9e3..cf06bfd22817 100644 --- a/.travis.yml +++ b/.travis.yml @@ -48,6 +48,7 @@ script: bin/rector process src --set symfony40 --dry-run composer docs php bin/check_services_in_yaml_configs.php + php bin/run_all_sets.php fi # Eat your own dog food diff --git a/bin/run_all_sets.php b/bin/run_all_sets.php new file mode 100644 index 000000000000..d11ae3907e2c --- /dev/null +++ b/bin/run_all_sets.php @@ -0,0 +1,31 @@ +provide() as $setName) { + $command = ['php', 'bin/rector', 'process', $file, '--set', $setName, '--dry-run']; + + $process = new Process($command, __DIR__ . '/..'); + echo sprintf('Set "%s" is OK' . PHP_EOL, $setName); + + try { + $process->mustRun(); + } catch (ProcessFailedException $processFailedException) { + if (! Strings::contains($processFailedException->getMessage(), '[ERROR]')) { + continue; + } + + echo $processFailedException->getMessage(); + exit(1); + } +} From e2a8659b2c64a8debfc80206b5db997c41e4459e Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 26 Sep 2019 13:10:48 +0200 Subject: [PATCH 3/4] fix easy-admin-bundle20 set --- bin/run_all_sets.php | 2 +- config/set/easy-corp/easy-admin-bundle20.yaml | 3 ++- .../NodeTypeResolver/src/NodeTypeResolver.php | 25 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/bin/run_all_sets.php b/bin/run_all_sets.php index d11ae3907e2c..5d41832ee37e 100644 --- a/bin/run_all_sets.php +++ b/bin/run_all_sets.php @@ -21,7 +21,7 @@ try { $process->mustRun(); } catch (ProcessFailedException $processFailedException) { - if (! Strings::contains($processFailedException->getMessage(), '[ERROR]')) { + if (! Strings::match($processFailedException->getMessage(), '#(Fatal error)|(\[ERROR\])#')) { continue; } diff --git a/config/set/easy-corp/easy-admin-bundle20.yaml b/config/set/easy-corp/easy-admin-bundle20.yaml index 344705ea7c3b..e8e619befe55 100644 --- a/config/set/easy-corp/easy-admin-bundle20.yaml +++ b/config/set/easy-corp/easy-admin-bundle20.yaml @@ -2,7 +2,8 @@ services: # first replace ->get("...") by constructor injection in every child of "EasyCorp\Bundle\EasyAdminBundle\AdminController" Rector\Symfony\Rector\FrameworkBundle\GetToConstructorInjectionRector: - EasyCorp\Bundle\EasyAdminBundle\AdminController: ~ + $getMethodAwareTypes: + - 'EasyCorp\Bundle\EasyAdminBundle\AdminController' # then rename the "EasyCorp\Bundle\EasyAdminBundle\AdminController" class Rector\Renaming\Rector\Class_\RenameClassRector: diff --git a/packages/NodeTypeResolver/src/NodeTypeResolver.php b/packages/NodeTypeResolver/src/NodeTypeResolver.php index c6b4b00643f0..022b9c23be6b 100644 --- a/packages/NodeTypeResolver/src/NodeTypeResolver.php +++ b/packages/NodeTypeResolver/src/NodeTypeResolver.php @@ -128,6 +128,8 @@ public function __construct( */ public function isObjectType(Node $node, $requiredType): bool { + $this->ensureRequiredTypeIsStringOrObjectType($requiredType, 'isObjectType'); + if (is_string($requiredType)) { if (Strings::contains($requiredType, '*')) { return $this->isFnMatch($node, $requiredType); @@ -699,4 +701,27 @@ private function unionWithParentClassesInterfacesAndUsedTraits(Type $type): Type return $type; } + + /** + * @param mixed $requiredType + */ + private function ensureRequiredTypeIsStringOrObjectType($requiredType, string $location): void + { + if (is_string($requiredType)) { + return; + } + + if ($requiredType instanceof ObjectType) { + return; + } + + $reportedType = is_object($requiredType) ? get_class($requiredType) : $requiredType; + + throw new ShouldNotHappenException(sprintf( + 'Value passed to "%s()" must be string or "%s". "%s" given', + $location, + ObjectType::class, + $reportedType + )); + } } From fcb865767a6ede3b302f9a1d0735b026d518a3f5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 26 Sep 2019 13:18:48 +0200 Subject: [PATCH 4/4] make RectorServiceParametersShifter report better error in case of invalid configuration --- bin/run_all_sets.php | 25 +++++++++++++++++-- config/set/celebrity/celebrity.yaml | 3 +-- .../framework-migration/nette-to-symfony.yaml | 5 ++-- ...ameTesterTestToPHPUnitToTestFileRector.php | 5 +++- .../Loader/RectorServiceParametersShifter.php | 15 ++++++++--- .../Loader/TolerantRectorYamlFileLoader.php | 2 +- 6 files changed, 44 insertions(+), 11 deletions(-) diff --git a/bin/run_all_sets.php b/bin/run_all_sets.php index 5d41832ee37e..3d8547b7ffe5 100644 --- a/bin/run_all_sets.php +++ b/bin/run_all_sets.php @@ -4,6 +4,7 @@ use Rector\Set\SetProvider; use Symfony\Component\Process\Exception\ProcessFailedException; use Symfony\Component\Process\Process; +use Symplify\PackageBuilder\Console\ShellCode; require __DIR__ . '/../vendor/autoload.php'; @@ -11,10 +12,20 @@ // any file to be tested $file = 'src/Rector/AbstractRector.php'; +$excludedSets = [ + // required Kernel class to be set in parameters + 'symfony-code-quality' +]; +$errors = []; foreach ($setProvider->provide() as $setName) { + if (in_array($setName, $excludedSets, true)) { + continue; + } + $command = ['php', 'bin/rector', 'process', $file, '--set', $setName, '--dry-run']; + $process = new Process($command, __DIR__ . '/..'); echo sprintf('Set "%s" is OK' . PHP_EOL, $setName); @@ -25,7 +36,17 @@ continue; } - echo $processFailedException->getMessage(); - exit(1); + $errors[] = $processFailedException->getMessage(); } } + +if ($errors === []) { + exit(ShellCode::SUCCESS); +} + +foreach ($errors as $error) { + echo $error; + echo PHP_EOL; +} + +exit(ShellCode::ERROR); diff --git a/config/set/celebrity/celebrity.yaml b/config/set/celebrity/celebrity.yaml index bf3057070245..62c82f6afa18 100644 --- a/config/set/celebrity/celebrity.yaml +++ b/config/set/celebrity/celebrity.yaml @@ -32,8 +32,7 @@ services: Rector\Celebrity\Rector\FuncCall\SetTypeToCastRector: ~ # class { var $value; } → class { public $value; } - Rector\Php52\Rector\Property\VarToPublicPropertyRector: ~pu - + Rector\Php52\Rector\Property\VarToPublicPropertyRector: ~ # false or true → false || true # false and true → false && true diff --git a/config/set/framework-migration/nette-to-symfony.yaml b/config/set/framework-migration/nette-to-symfony.yaml index 295b5a9aa91f..c64cc37047dc 100644 --- a/config/set/framework-migration/nette-to-symfony.yaml +++ b/config/set/framework-migration/nette-to-symfony.yaml @@ -38,8 +38,9 @@ services: addConfig: load Rector\Rector\Interface_\RemoveInterfacesRector: - # Removes "implements IPresenter" - - Nette\Application\IPresenter + $interfacesToRemove: + # Removes "implements IPresenter" + - Nette\Application\IPresenter Rector\Renaming\Rector\Constant\RenameClassConstantRector: Nette\Http\*Response: diff --git a/packages/NetteTesterToPHPUnit/src/Rector/RenameTesterTestToPHPUnitToTestFileRector.php b/packages/NetteTesterToPHPUnit/src/Rector/RenameTesterTestToPHPUnitToTestFileRector.php index 554a9f7889cf..25d5b4bb41af 100644 --- a/packages/NetteTesterToPHPUnit/src/Rector/RenameTesterTestToPHPUnitToTestFileRector.php +++ b/packages/NetteTesterToPHPUnit/src/Rector/RenameTesterTestToPHPUnitToTestFileRector.php @@ -18,8 +18,11 @@ public function getDefinition(): RectorDefinition public function refactor(SmartFileInfo $smartFileInfo): void { $oldRealPath = $smartFileInfo->getRealPath(); - $newRealPath = $this->createNewRealPath($oldRealPath); + if (! Strings::endsWith($oldRealPath, '*.phpt')) { + return; + } + $newRealPath = $this->createNewRealPath($oldRealPath); if ($newRealPath === $oldRealPath) { return; } diff --git a/src/DependencyInjection/Loader/RectorServiceParametersShifter.php b/src/DependencyInjection/Loader/RectorServiceParametersShifter.php index d81050f97c90..eccf50363da6 100644 --- a/src/DependencyInjection/Loader/RectorServiceParametersShifter.php +++ b/src/DependencyInjection/Loader/RectorServiceParametersShifter.php @@ -4,6 +4,7 @@ use Nette\Utils\Strings; use Rector\Exception\Configuration\InvalidConfigurationException; +use Rector\Exception\ShouldNotHappenException; use ReflectionClass; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; use Symfony\Component\Yaml\Yaml; @@ -43,13 +44,13 @@ public function __construct() * @param mixed[] $configuration * @return mixed[] */ - public function process(array $configuration): array + public function process(array $configuration, string $file): array { if (! isset($configuration[self::SERVICES_KEY]) || ! is_array($configuration[self::SERVICES_KEY])) { return $configuration; } - $configuration[self::SERVICES_KEY] = $this->processServices($configuration[self::SERVICES_KEY]); + $configuration[self::SERVICES_KEY] = $this->processServices($configuration[self::SERVICES_KEY], $file); return $configuration; } @@ -58,13 +59,21 @@ public function process(array $configuration): array * @param mixed[] $services * @return mixed[] */ - private function processServices(array $services): array + private function processServices(array $services, string $file): array { foreach ($services as $serviceName => $serviceDefinition) { if (! $this->isRectorClass($serviceName) || empty($serviceDefinition)) { continue; } + if (! is_array($serviceDefinition)) { + throw new ShouldNotHappenException(sprintf( + 'Rector rule "%s" has invalid configuration in "%s". Fix it to an array', + $serviceName, + $file + )); + } + $nonReservedNonVariables = $this->resolveRectorConfiguration($serviceDefinition); // nothing to change diff --git a/src/DependencyInjection/Loader/TolerantRectorYamlFileLoader.php b/src/DependencyInjection/Loader/TolerantRectorYamlFileLoader.php index 64222a9e157f..012a64e34159 100644 --- a/src/DependencyInjection/Loader/TolerantRectorYamlFileLoader.php +++ b/src/DependencyInjection/Loader/TolerantRectorYamlFileLoader.php @@ -47,7 +47,7 @@ protected function loadFile($file) $this->classExistenceValidator->ensureClassesAreValid($configuration, $file); - $configuration = $this->rectorServiceParametersShifter->process($configuration); + $configuration = $this->rectorServiceParametersShifter->process($configuration, $file); return $this->parameterInImportResolver->process($configuration); }