Skip to content

Commit

Permalink
[Performance] Extract PathSkipVoter to dedicated service and call bef…
Browse files Browse the repository at this point in the history
…ore Filesystem::read() (#5451)

* [Performance] Extract PathSkipVoter to dedicated service and call before Filesystem::read()

* cache skipped paths

* [ci-review] Rector Rectify

* Fix phpstan

* Final touch: clean up readonly property as already in class

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Jan 10, 2024
1 parent e231caa commit f68d9f8
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 58 deletions.
6 changes: 6 additions & 0 deletions src/Application/ApplicationFileProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Rector\Configuration\Parameter\SimpleParameterProvider;
use Rector\Parallel\Application\ParallelFileProcessor;
use Rector\Provider\CurrentFileProvider;
use Rector\Skipper\Skipper\PathSkipper;
use Rector\Testing\PHPUnit\StaticPHPUnitEnvironment;
use Rector\Util\ArrayParametersMerger;
use Rector\ValueObject\Application\File;
Expand Down Expand Up @@ -49,6 +50,7 @@ public function __construct(
private readonly CurrentFileProvider $currentFileProvider,
private readonly FileProcessor $fileProcessor,
private readonly ArrayParametersMerger $arrayParametersMerger,
private readonly PathSkipper $pathSkipper
) {
}

Expand Down Expand Up @@ -122,6 +124,10 @@ public function processFiles(
$collectedData = [];

foreach ($filePaths as $filePath) {
if ($this->pathSkipper->shouldSkip($filePath)) {
continue;
}

if ($preFileCallback !== null) {
$preFileCallback($filePath);
}
Expand Down
3 changes: 1 addition & 2 deletions src/DependencyInjection/LazyContainerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@
use Rector\Skipper\Contract\SkipVoterInterface;
use Rector\Skipper\Skipper\Skipper;
use Rector\Skipper\SkipVoter\ClassSkipVoter;
use Rector\Skipper\SkipVoter\PathSkipVoter;
use Rector\StaticTypeMapper\Contract\PhpDocParser\PhpDocTypeMapperInterface;
use Rector\StaticTypeMapper\Contract\PhpParser\PhpParserNodeMapperInterface;
use Rector\StaticTypeMapper\Mapper\PhpParserNodeMapper;
Expand Down Expand Up @@ -366,7 +365,7 @@ final class LazyContainerFactory
/**
* @var array<class-string<SkipVoterInterface>>
*/
private const SKIP_VOTER_CLASSES = [ClassSkipVoter::class, PathSkipVoter::class];
private const SKIP_VOTER_CLASSES = [ClassSkipVoter::class];

/**
* @api used as next rectorConfig factory
Expand Down
13 changes: 7 additions & 6 deletions src/Skipper/SkipCriteriaResolver/SkippedPathsResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
final class SkippedPathsResolver
{
/**
* @var string[]
* @var null|string[]
*/
private array $skippedPaths = [];
private null|array $skippedPaths = null;

public function __construct(
private readonly FilePathHelper $filePathHelper
Expand All @@ -29,17 +29,18 @@ public function __construct(
*/
public function resolve(): array
{
// disable cache in tests
if (StaticPHPUnitEnvironment::isPHPUnitRun()) {
// disable cache in tests
$this->skippedPaths = [];
$this->skippedPaths = null;
}

// disable cache in tests
if ($this->skippedPaths !== []) {
// already filled, even empty array
if ($this->skippedPaths !== null) {
return $this->skippedPaths;
}

$skip = SimpleParameterProvider::provideArrayParameter(Option::SKIP);
$this->skippedPaths = [];

foreach ($skip as $key => $value) {
if (! is_int($key)) {
Expand Down
41 changes: 0 additions & 41 deletions src/Skipper/SkipVoter/PathSkipVoter.php

This file was deleted.

26 changes: 26 additions & 0 deletions src/Skipper/Skipper/PathSkipper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

declare(strict_types=1);

namespace Rector\Skipper\Skipper;

use Rector\Skipper\Matcher\FileInfoMatcher;
use Rector\Skipper\SkipCriteriaResolver\SkippedPathsResolver;

final readonly class PathSkipper
{
public function __construct(
private FileInfoMatcher $fileInfoMatcher,
private SkippedPathsResolver $skippedPathsResolver
) {
}

public function shouldSkip(string $filePath): bool
{
$skippedPaths = $this->skippedPathsResolver->resolve();
return $this->fileInfoMatcher->doesFileInfoMatchPatterns(
$filePath,
$skippedPaths
);
}
}
8 changes: 2 additions & 6 deletions src/Skipper/Skipper/Skipper.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,13 @@
*/
final readonly class Skipper
{
/**
* @var string
*/
private const FILE_ELEMENT = 'file_elements';

/**
* @param array<SkipVoterInterface> $skipVoters
*/
public function __construct(
private RectifiedAnalyzer $rectifiedAnalyzer,
private array $skipVoters,
private PathSkipper $pathSkipper
) {
Assert::allIsInstanceOf($this->skipVoters, SkipVoterInterface::class);
}
Expand All @@ -38,7 +34,7 @@ public function shouldSkipElement(string | object $element): bool

public function shouldSkipFilePath(string $filePath): bool
{
return $this->shouldSkipElementAndFilePath(self::FILE_ELEMENT, $filePath);
return $this->pathSkipper->shouldSkip($filePath);
}

public function shouldSkipElementAndFilePath(string | object $element, string $filePath): bool
Expand Down
5 changes: 2 additions & 3 deletions tests/Skipper/Skipper/SkipperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public static function provideDataShouldSkipFilePath(): Iterator
yield [__DIR__ . '/Fixture/SomeSkipped/any.txt', true];
yield ['tests/Skipper/Skipper/Fixture/SomeSkippedPath/any.txt', true];
yield ['tests/Skipper/Skipper/Fixture/SomeSkippedPathToFile/any.txt', true];
yield [__DIR__ . '/Fixture/AlwaysSkippedPath/some_file.txt', true];
yield [__DIR__ . '/Fixture/PathSkippedWithMask/another_file.txt', true];
}

/**
Expand Down Expand Up @@ -91,9 +93,6 @@ public static function provideCheckerAndFile(): Iterator

yield [NotSkippedClass::class, __DIR__ . '/Fixture/someFile', false];
yield [NotSkippedClass::class, __DIR__ . '/Fixture/someOtherFile', false];

yield ['anything', __DIR__ . '/Fixture/AlwaysSkippedPath/some_file.txt', true];
yield ['anything', __DIR__ . '/Fixture/PathSkippedWithMask/another_file.txt', true];
}

public static function provideDataShouldSkipElement(): Iterator
Expand Down

0 comments on commit f68d9f8

Please sign in to comment.