Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DX] [Experimental] Add withPhpLevel() to raise PHP level one rule at a time #6261

Merged
merged 2 commits into from
Oct 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ parameters:
- '#Callable callable\(PHPStan\\Type\\Type\)\: PHPStan\\Type\\Type invoked with 2 parameters, 1 required#'

# known value
- '#Method Rector\\Php\\PhpVersionProvider\:\:provide\(\) should return 50200\|50300\|50400\|50500\|50600\|70000\|70100\|70200\|70300\|70400\|80000\|80100\|80200\|80300\|80400\|100000 but returns int#'
- '#Method (.*?) should return 50200\|50300\|50400\|50500\|50600\|70000\|70100\|70200\|70300\|70400\|80000\|80100\|80200\|80300\|80400\|100000 but returns int#'

-
message: '#Function "class_exists\(\)" cannot be used/left in the code#'
Expand Down Expand Up @@ -295,9 +295,6 @@ parameters:
- '#Register "Rector\\Php71\\Rector\\ClassConst\\PublicConstantVisibilityRector" service to "php71\.php" config set#'
- '#Public method "Rector\\ValueObject\\Error\\SystemError\:\:getFile\(\)" is never used#'

# known values
- '#Method Rector\\Util\\PhpVersionFactory\:\:createIntVersion\(\) should return 50200\|50300\|50400\|50500\|50600\|70000\|70100\|70200\|70300\|70400\|80000\|80100\|80200\|80300\|80400\|100000 but returns int#'

# soon to be used
- '#Property Rector\\Configuration\\RectorConfigBuilder\:\:\$isWithPhpSetsUsed is never read, only written#'

Expand Down
63 changes: 52 additions & 11 deletions src/Bridge/SetRectorsResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,68 @@

use Rector\Config\RectorConfig;
use Rector\Contract\Rector\RectorInterface;
use ReflectionProperty;
use Webmozart\Assert\Assert;

/**
* @api
* @experimental since 1.1.2
* Utils class to ease building bridges by 3rd-party tools
*
* @see \Rector\Tests\Bridge\SetRectorsResolverTest
*/
final class SetRectorsResolver
{
/**
* @return array<class-string<RectorInterface>>
* @param string[] $configFilePaths
* @return array<int, class-string<RectorInterface>|array<class-string<RectorInterface>, mixed[]>>
*/
public function resolveFromFilePath(string $configFilePath): array
public function resolveFromFilePathsIncludingConfiguration(array $configFilePaths): array
{
Assert::allString($configFilePaths);
Assert::allFileExists($configFilePaths);

$combinedRectorRulesWithConfiguration = [];

foreach ($configFilePaths as $configFilePath) {
$rectorRulesWithConfiguration = $this->resolveFromFilePathIncludingConfiguration($configFilePath);

$combinedRectorRulesWithConfiguration = array_merge(
$combinedRectorRulesWithConfiguration,
$rectorRulesWithConfiguration
);
}

return $combinedRectorRulesWithConfiguration;
}

/**
* @return array<int, class-string<RectorInterface>|array<class-string<RectorInterface>, mixed[]>>
*/
public function resolveFromFilePathIncludingConfiguration(string $configFilePath): array
{
$rectorConfig = $this->loadRectorConfigFromFilePath($configFilePath);

$rectorClassesWithOptionalConfiguration = $rectorConfig->getRectorClasses();

foreach ($rectorConfig->getRuleConfigurations() as $rectorClass => $configuration) {
// remove from non-configurable, if added again with better config
if (in_array($rectorClass, $rectorClassesWithOptionalConfiguration)) {
$rectorRulePosition = array_search($rectorClass, $rectorClassesWithOptionalConfiguration, true);
if (is_int($rectorRulePosition)) {
unset($rectorClassesWithOptionalConfiguration[$rectorRulePosition]);
}
}

$rectorClassesWithOptionalConfiguration[] = [
$rectorClass => $configuration,
];
}

// sort keys
return array_values($rectorClassesWithOptionalConfiguration);
}

private function loadRectorConfigFromFilePath(string $configFilePath): RectorConfig
{
Assert::fileExists($configFilePath);

Expand All @@ -29,13 +77,6 @@ public function resolveFromFilePath(string $configFilePath): array
$configCallable = require $configFilePath;
$configCallable($rectorConfig);

// get tagged class-names
$tagsReflectionProperty = new ReflectionProperty($rectorConfig, 'tags');
$tags = $tagsReflectionProperty->getValue($rectorConfig);

$rectorClasses = $tags[RectorInterface::class] ?? [];
sort($rectorClasses);

return array_unique($rectorClasses);
return $rectorConfig;
}
}
45 changes: 45 additions & 0 deletions src/Configuration/RectorConfigBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Nette\Utils\FileSystem;
use Rector\Bridge\SetProviderCollector;
use Rector\Bridge\SetRectorsResolver;
use Rector\Caching\Contract\ValueObject\Storage\CacheStorageInterface;
use Rector\Config\Level\CodeQualityLevel;
use Rector\Config\Level\DeadCodeLevel;
Expand Down Expand Up @@ -160,6 +161,8 @@ final class RectorConfigBuilder
*/
private ?bool $isWithPhpSetsUsed = null;

private ?bool $isWithPhpLevelUsed = null;

public function __invoke(RectorConfig $rectorConfig): void
{
// @experimental 2024-06
Expand All @@ -175,6 +178,13 @@ public function __invoke(RectorConfig $rectorConfig): void

$uniqueSets = array_unique($this->sets);

if ($this->isWithPhpLevelUsed && $this->isWithPhpSetsUsed) {
throw new InvalidConfigurationException(sprintf(
'Your config uses "withPhp*()" and "withPhpLevel()" methods at the same time.%sPick one of them to avoid rule conflicts.',
PHP_EOL
));
}

if (in_array(SetList::TYPE_DECLARATION, $uniqueSets, true) && $this->isTypeCoverageLevelUsed === true) {
throw new InvalidConfigurationException(sprintf(
'Your config already enables type declarations set.%sRemove "->withTypeCoverageLevel()" as it only duplicates it, or remove type declaration set.',
Expand Down Expand Up @@ -962,6 +972,41 @@ public function withTypeCoverageLevel(int $level): self
return $this;
}

/**
* @experimental Since 1.2.5 Raise your PHP level from, one level at a time
*/
public function withPhpLevel(int $level): self
{
Assert::natural($level);

$this->isWithPhpLevelUsed = true;

$phpVersion = ComposerJsonPhpVersionResolver::resolveFromCwdOrFail();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should allow read from withPhpVersion() defined from Rector config so it can be tested on getrector.com, which PhpVersionProvider seems can be used, or add fallback on ComposerJsonPhpVersionResolver to use predefined from withPhpversion() config when exists

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created experiment PR at #6384


$setRectorsResolver = new SetRectorsResolver();
$setFilePaths = PhpLevelSetResolver::resolveFromPhpVersion($phpVersion);

$rectorRulesWithConfiguration = $setRectorsResolver->resolveFromFilePathsIncludingConfiguration($setFilePaths);

foreach ($rectorRulesWithConfiguration as $position => $rectorRuleWithConfiguration) {
// add rules untill level is reached
if ($position > $level) {
continue;
}

if (is_string($rectorRuleWithConfiguration)) {
$this->rules[] = $rectorRuleWithConfiguration;
} elseif (is_array($rectorRuleWithConfiguration)) {
foreach ($rectorRuleWithConfiguration as $rectorRule => $rectorRuleConfiguration) {
/** @var class-string<ConfigurableRectorInterface> $rectorRule */
$this->withConfiguredRule($rectorRule, $rectorRuleConfiguration);
}
}
}

return $this;
}

/**
* @experimental Raise your code quality from the safest rules
* to more affecting ones, one level at a time
Expand Down
5 changes: 5 additions & 0 deletions tests/Bridge/Fixture/some-composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"require": {
"php": "^7.3"
}
}
74 changes: 74 additions & 0 deletions tests/Bridge/SetRectorsResolverTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\Bridge;

use PHPUnit\Framework\TestCase;
use Rector\Bridge\SetRectorsResolver;
use Rector\Configuration\PhpLevelSetResolver;
use Rector\Contract\Rector\RectorInterface;
use Rector\Php\PhpVersionResolver\ComposerJsonPhpVersionResolver;
use Rector\Set\ValueObject\SetList;
use Rector\ValueObject\PhpVersion;

final class SetRectorsResolverTest extends TestCase
{
private SetRectorsResolver $setRectorsResolver;

protected function setUp(): void
{
$this->setRectorsResolver = new SetRectorsResolver();
}

public function testResolveFromFilePathForPhpVersion(): void
{
$configFilePaths = PhpLevelSetResolver::resolveFromPhpVersion(PhpVersion::PHP_70);
$this->assertCount(6, $configFilePaths);
$this->assertContainsOnly('string', $configFilePaths);

foreach ($configFilePaths as $configFilePath) {
$this->assertFileExists($configFilePath);
}
}

public function testResolveFromFilePathForPhpLevel(): void
{
$projectPhpVersion = ComposerJsonPhpVersionResolver::resolve(__DIR__ . '/Fixture/some-composer.json');

$this->assertIsInt($projectPhpVersion);
$this->assertSame(PhpVersion::PHP_73, $projectPhpVersion);

$configFilePaths = PhpLevelSetResolver::resolveFromPhpVersion($projectPhpVersion);
$this->assertCount(9, $configFilePaths);

$rectorRulesWithConfiguration = $this->setRectorsResolver->resolveFromFilePathsIncludingConfiguration(
$configFilePaths
);
$this->assertCount(63, $rectorRulesWithConfiguration);
}

public function testResolveWithConfiguration(): void
{
$rectorRulesWithConfiguration = $this->setRectorsResolver->resolveFromFilePathIncludingConfiguration(
SetList::PHP_73
);
$this->assertCount(10, $rectorRulesWithConfiguration);

$this->assertArrayHasKey(0, $rectorRulesWithConfiguration);
$this->assertArrayHasKey(9, $rectorRulesWithConfiguration);

foreach ($rectorRulesWithConfiguration as $rectorRuleWithConfiguration) {
if (is_string($rectorRuleWithConfiguration)) {
$this->assertTrue(is_a($rectorRuleWithConfiguration, RectorInterface::class, true));
}

if (is_array($rectorRuleWithConfiguration)) {
foreach ($rectorRuleWithConfiguration as $rectorRule => $rectorRuleConfiguration) {
$this->assertTrue(is_a($rectorRule, RectorInterface::class, true));
$this->assertIsArray($rectorRuleConfiguration);
}
}
}
}
}