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

Gh 2538: Disable include walker #2580

Merged
merged 2 commits into from Mar 2, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -19,6 +19,8 @@ Improvements:
- Skip parent parameters on complete constructor #2471 @mamazu
- Support generics on `@mixin` #2463
- Remove "on develop warning" service #2533
- Disable the processing of includes/requires, it doesn't work very well but
it has massive performance impact on certain projects #2580

Bug fixes:

Expand Down
4 changes: 4 additions & 0 deletions lib/WorseReflection/Core/Inference/Walker/IncludeWalker.php
Expand Up @@ -17,6 +17,10 @@
use Psr\Log\LoggerInterface;
use Symfony\Component\Filesystem\Path;

/**
* This walker doesn't seem to work properly, and in addition it can cause massive performance
* problems on legacy projects that use lots of `includes`.
*/
class IncludeWalker implements Walker
{
private Parser $parser;
Expand Down
13 changes: 1 addition & 12 deletions lib/WorseReflection/Core/ServiceLocator.php
Expand Up @@ -13,7 +13,6 @@
use Phpactor\WorseReflection\Core\Inference\Walker\DiagnosticsWalker;
use Phpactor\WorseReflection\Core\Inference\Walker\PassThroughWalker;
use Phpactor\WorseReflection\Core\Inference\Walker\FunctionLikeWalker;
use Phpactor\WorseReflection\Core\Inference\Walker\IncludeWalker;
use Phpactor\WorseReflection\Core\Inference\Walker\VariableWalker;
use Phpactor\WorseReflection\Core\Inference\NodeToTypeConverter;
use Phpactor\WorseReflection\Core\Inference\NodeContextResolver;
Expand Down Expand Up @@ -61,7 +60,7 @@ public function __construct(
private array $memberContextResolvers,
Cache $cache,
private CacheForDocument $cacheForDocument,
bool $enableContextualLocation = false
bool $enableContextualLocation = false,
) {
$sourceReflector = $reflectorFactory->create($this);

Expand Down Expand Up @@ -148,16 +147,6 @@ public function frameBuilder(): FrameResolver
new FunctionLikeWalker(),
new PassThroughWalker(),
new VariableWalker($this->docblockFactory),

// enable subset of frame walkers to enable include variables
// to be merged into current frame
new IncludeWalker($this->logger, FrameResolver::create(
$this->nodeContextResolver(),
[
new VariableWalker($this->docblockFactory),
new PassThroughWalker(),
]
)),
], $this->frameWalkers),
);
}
Expand Down
9 changes: 9 additions & 0 deletions lib/WorseReflection/Tests/Inference/SelfTest.php
Expand Up @@ -8,6 +8,12 @@

class SelfTest extends IntegrationTestCase
{
public const DISABLED_TESTS = [
// disabling the includeWalker because it barely works
// and it causes severe performance issues.
'require_and_include',
];

/**
* @dataProvider provideSelf
*/
Expand All @@ -29,6 +35,9 @@ public function provideSelf(): Generator
{
foreach ((array)glob(__DIR__ . '/*/*.test') as $fname) {
$dirName = basename(dirname((string)$fname));
if (in_array($dirName, self::DISABLED_TESTS)) {
continue;
}
yield $dirName .' ' . basename((string)$fname) => [
$fname
];
Expand Down
Expand Up @@ -18,6 +18,9 @@ public function setUp(): void

public function provideWalk(): Generator
{
// disabled this walker for now due to perforamnce and behavioral
// issues.
return;
yield 'Require relative' => [
<<<'EOT'
<?php
Expand Down