Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

/**
* A class/path skip is only "used" when the rule would actually have changed the skipped file.
* A rule-scoped path skip is marked used for every file whose path it matches, as the rule is
* skipped once per file.
*
* @see RenameClassRector
*/
Expand All @@ -32,7 +33,7 @@ public function testMarksSkipUsedOnlyWhenRuleWouldChangeFile(): void

$usedPaths = $usedSkips[RenameClassRector::class] ?? [];

// the skip that actually prevented a rename is marked used
// both skip masks match a processed file, so both are marked used
$this->assertContains('*skip_used_renames_old_class*', $usedPaths);

// the skip on a file the rule would not have touched is never marked used
Expand Down
4 changes: 1 addition & 3 deletions src/Application/RectorRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ public function refreshRectors(array $rectors): void
}

/**
* @return RectorInterface[]
*
* @api used in tests
* @return array<RectorInterface>
*/
public function forPath(string $filePath): array
{
Expand Down
8 changes: 0 additions & 8 deletions src/PhpParser/NodeTraverser/RectorNodeTraverser.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,7 @@
use Webmozart\Assert\Assert;

/**
* Based on native NodeTraverser class, but heavily customized for Rector needs.
*
* The main differences are:
* - no leaveNode(), the RectorRunner calls each rule's refactor() method on enter
* - cached visitors per node class for performance, e.g. when we find rules for Class_ node, they're cached for next time
* - immutability features, register Rector rules once, then use; no changes on the fly
*
* @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
* @internal No BC promise on this class, it might change any time.
*/
final class RectorNodeTraverser
{
Expand Down
6 changes: 6 additions & 0 deletions src/Testing/PHPUnit/AbstractRectorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Nette\Utils\Strings;
use PHPUnit\Framework\ExpectationFailedException;
use Rector\Application\ApplicationFileProcessor;
use Rector\Application\RectorRegistry;
use Rector\Autoloading\AdditionalAutoloader;
use Rector\Autoloading\BootstrapFilesIncluder;
use Rector\Composer\InstalledPackageResolver;
Expand Down Expand Up @@ -112,6 +113,11 @@ protected function setUp(): void
$rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class);
$rectorNodeTraverser->refreshPhpRectors($rectors);

// keep the shared registry in sync with the current test's rules
/** @var RectorRegistry $rectorRegistry */
$rectorRegistry = $rectorConfig->make(RectorRegistry::class);
$rectorRegistry->refreshRectors($rectors);

// store cache
self::$cacheByRuleAndConfig[$cacheKey] = true;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/Application/RectorRegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use Rector\Configuration\Option;
use Rector\Configuration\Parameter\SimpleParameterProvider;
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
use Rector\Tests\Application\Source\SkippableRector;
use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement;

final class RectorRegistryTest extends AbstractLazyTestCase
{
Expand All @@ -19,11 +19,11 @@ protected function setUp(): void
parent::setUp();

SimpleParameterProvider::setParameter(Option::SKIP, [
SkippableRector::class => ['*/skipped_directory/*'],
FifthElement::class => ['*/skipped_directory/*'],
]);

$this->rectorRegistry = $this->make(RectorRegistry::class);
$this->rectorRegistry->refreshRectors([new SkippableRector()]);
$this->rectorRegistry->refreshRectors([new FifthElement()]);
}

protected function tearDown(): void
Expand All @@ -43,6 +43,6 @@ public function testForPathKeepsRectorForOtherPath(): void
$rectorsForPath = $this->rectorRegistry->forPath(__DIR__ . '/some_file.php');

$this->assertCount(1, $rectorsForPath);
$this->assertInstanceOf(SkippableRector::class, $rectorsForPath[0]);
$this->assertInstanceOf(FifthElement::class, $rectorsForPath[0]);
}
}
27 changes: 0 additions & 27 deletions tests/Application/Source/SkippableRector.php

This file was deleted.

6 changes: 4 additions & 2 deletions tests/Reporting/MissConfigurationReporterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement;
use Rector\Tests\Skipper\Skipper\Fixture\Element\ThreeMan;
use Rector\Tests\Skipper\Skipper\Source\AnotherClassToSkip;
use Rector\Tests\Skipper\Skipper\Source\NotSkippedClass;
use Rector\Validation\RectorConfigValidator;
use Rector\ValueObject\ProcessResult;
use Symfony\Component\Console\Input\ArrayInput;
Expand Down Expand Up @@ -69,7 +70,7 @@ public function testReportsSkippedNonRectorClass(): void
{
RectorConfigValidator::ensureRectorRulesExist([
// not a Rector rule, can never be skipped
AnotherClassToSkip::class => ['some/path'],
NotSkippedClass::class => ['some/path'],
// Rector rule, must not be reported
OrdSingleByteRector::class => ['some/path'],
// post rector rule, must not be reported
Expand All @@ -80,7 +81,8 @@ public function testReportsSkippedNonRectorClass(): void

$output = $this->bufferedOutput->fetch();

$this->assertStringContainsString('AnotherClassToSkip', $output);
$this->assertStringContainsString('NotSkippedClass', $output);

$this->assertStringNotContainsString('OrdSingleByteRector', $output);
$this->assertStringNotContainsString('NameImportingPostRector', $output);
}
Expand Down
19 changes: 18 additions & 1 deletion tests/Skipper/Skipper/Fixture/Element/FifthElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@

namespace Rector\Tests\Skipper\Skipper\Fixture\Element;

final class FifthElement
use PhpParser\Node;
use Rector\RuleDoc\RuleDefinition;

final class FifthElement implements \Rector\Contract\Rector\RectorInterface
{
public function getRuleDefinition(): RuleDefinition
{
// TODO: Implement getRuleDefinition() method.
}

public function getNodeTypes(): array
{
// TODO: Implement getNodeTypes() method.
}

public function refactor(Node $node)
{
// TODO: Implement refactor() method.
}
}
12 changes: 6 additions & 6 deletions tests/Skipper/Skipper/SkipperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,23 @@ public static function provideDataShouldSkipFilePath(): Iterator
yield [__DIR__ . '/Fixture/PathSkippedWithMask/another_file.txt', true];
}

#[DataProvider('provideCheckerAndFile')]
public function testSkipRectorAndFile(object $rector, string $filePath, bool $expectedSkip): void
#[DataProvider('provideRectorAndFile')]
public function testSkipElementAndFilePath(object $rector, string $filePath, bool $expectedSkip): void
{
$resolvedSkip = $this->skipper->shouldSkipRectorAndFile($rector, $filePath);
$this->assertSame($expectedSkip, $resolvedSkip);
}

/**
* @return Iterator<array<array<int, mixed>, mixed>>
*/
public static function provideCheckerAndFile(): Iterator
public static function provideRectorAndFile(): Iterator
{
yield [new FifthElement(), __DIR__ . '/Fixture', true];

yield [new AnotherClassToSkip(), __DIR__ . '/Fixture/someFile', true];
yield [new AnotherClassToSkip(), __DIR__ . '/Fixture/someDirectory/anotherFile.php', true];

yield [new FifthElement(), __DIR__ . '/Fixture/someFile', true];
yield [new FifthElement(), __DIR__ . '/Fixture/someDirectory/anotherFile.php', true];

yield [new NotSkippedClass(), __DIR__ . '/Fixture/someFile', false];
yield [new NotSkippedClass(), __DIR__ . '/Fixture/someOtherFile', false];
}
Expand Down
20 changes: 19 additions & 1 deletion tests/Skipper/Skipper/Source/AnotherClassToSkip.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,24 @@

namespace Rector\Tests\Skipper\Skipper\Source;

final class AnotherClassToSkip
use PhpParser\Node;
use Rector\Contract\Rector\RectorInterface;
use Rector\RuleDoc\RuleDefinition;

final class AnotherClassToSkip implements RectorInterface
{
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Fixture rule used to assert path scoped skipping', []);
}

public function getNodeTypes(): array
{
return [];
}

public function refactor(Node $node)
{
return null;
}
}
2 changes: 1 addition & 1 deletion tests/Skipper/Skipper/UsedSkipCollectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected function tearDown(): void
public function testCollectsOnlyMatchedSkips(): void
{
$this->skipper->shouldSkipFilePath('tests/Skipper/Skipper/Fixture/SomeSkippedPath/any.txt');
$this->skipper->shouldSkipRectorAndFile(new FifthElement(), __FILE__);
$this->skipper->shouldSkipRectorAndFile(new FifthElement(), __DIR__ . '/Fixture/someFile');

$usedSkips = $this->usedSkipCollector->provide();

Expand Down
Loading