Rector: Getting Rector\Core\Exception\ShouldNotHappenException on php5.6 code

[AntonioCS]

Bug Report

| Subject | Details |
| :------------- | :---------------------------------------------------------------|
| Rector version | v0.7.6 |
| Installed as | prefixed Rector PHAR |

I am on PHP 7.4.3 (cli) (built: Feb 26 2020 12:17:01)
I am running this command: vendor/bin/rector process . --set php74 --dry-run --verbose
After rector finishes parsing all the files I get the following:

[refactoring] admin/CategorySelection/CategorySelection.php

In NodeNameResolver.php line 81:

  [Rector\Core\Exception\ShouldNotHappenException]               
  Pick more specific node than "PhpParser\Node\Expr\MethodCall"  

Exception trace:
  at phar:///projectName/common/vendor/rector/rector-prefixed/rector/packages/node-name-resolver/src/NodeNameResolver.php:81
 Rector\NodeNameResolver\NodeNameResolver->getName() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/PhpParser/Node/Manipulator/ClassMethodPropertyFetchManipulator.php:47
 Rector\Core\PhpParser\Node\Manipulator\ClassMethodPropertyFetchManipulator->Rector\Core\PhpParser\Node\Manipulator\{closure}() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/PhpParser/NodeTraverser/CallableNodeTraverser.php:42
 class@anonymous\phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/PhpParser/NodeTraverser/CallableNodeTraverser.php:26$23b->enterNode() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:113
 PhpParser\NodeTraverser->traverseNode() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:196
 PhpParser\NodeTraverser->traverseArray() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:85
 PhpParser\NodeTraverser->traverse() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/PhpParser/NodeTraverser/CallableNodeTraverser.php:22
 Rector\Core\PhpParser\NodeTraverser\CallableNodeTraverser->traverseNodesWithCallable() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/PhpParser/Node/Manipulator/ClassMethodPropertyFetchManipulator.php:49
 Rector\Core\PhpParser\Node\Manipulator\ClassMethodPropertyFetchManipulator->resolveParamForPropertyFetch() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/rules/type-declaration/src/TypeInferer/PropertyTypeInferer/ConstructorPropertyTypeInferer.php:54
 Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer\ConstructorPropertyTypeInferer->inferProperty() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/rules/type-declaration/src/TypeInferer/PropertyTypeInferer.php:40
 Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer->inferProperty() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/rules/php-74/src/Rector/Property/TypedPropertyRector.php:78
 Rector\Php74\Rector\Property\TypedPropertyRector->refactor() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Rector/AbstractRector.php:123
 Rector\Core\Rector\AbstractRector->enterNode() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:176
 PhpParser\NodeTraverser->traverseArray() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:105
 PhpParser\NodeTraverser->traverseNode() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:196
 PhpParser\NodeTraverser->traverseArray() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:105
 PhpParser\NodeTraverser->traverseNode() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:196
 PhpParser\NodeTraverser->traverseArray() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:85
 PhpParser\NodeTraverser->traverse() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/PhpParser/NodeTraverser/RectorNodeTraverser.php:53
 Rector\Core\PhpParser\NodeTraverser\RectorNodeTraverser->traverse() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Application/FileProcessor.php:100
 Rector\Core\Application\FileProcessor->refactor() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Application/RectorApplication.php:123
 Rector\Core\Application\RectorApplication->Rector\Core\Application\{closure}() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Application/RectorApplication.php:176
 Rector\Core\Application\RectorApplication->tryCatchWrapper() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Application/RectorApplication.php:124
 Rector\Core\Application\RectorApplication->runOnFileInfos() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Console/Command/ProcessCommand.php:128
 Rector\Core\Console\Command\ProcessCommand->execute() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/symfony/console/Command/Command.php:228
 _HumbugBoxe941696e063c\Symfony\Component\Console\Command\Command->run() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Console/Command/AbstractCommand.php:30
 Rector\Core\Console\Command\AbstractCommand->run() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/symfony/console/Application.php:849
 _HumbugBoxe941696e063c\Symfony\Component\Console\Application->doRunCommand() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/symfony/console/Application.php:235
 _HumbugBoxe941696e063c\Symfony\Component\Console\Application->doRun() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/src/Console/Application.php:76
 Rector\Core\Console\Application->doRun() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/vendor/symfony/console/Application.php:136
 _HumbugBoxe941696e063c\Symfony\Component\Console\Application->run() at phar:///projectName/common/vendor/rector/rector-prefixed/rector/bin/rector:40
 require() at /projectName/common/vendor/rector/rector-prefixed/rector:6

Minimal PHP Code Causing Issue

This is the contents of the file admin/CategorySelection/CategorySelection.php

<?php
namespace ProjectName\Admin\CategorySelection;

abstract class CategorySelection
{
    protected $categories = [];

    public function __construct($category_mod)
    {
        $this->categories = Categories::getInstance($category_mod)->getCategories();
    }

    public function getSelectDropDown($selected_cat_id = 0)
    {
        return $this->getDisplayString($this->categories, $selected_cat_id, 1);
    }

    abstract protected function getDisplayString($categories, $selected_cat_id, $depth);

    protected function getPrefix($depth)
    {
        if ($depth == 1) {
            return '';
        } else {
            return str_repeat("&nbsp;", $depth) . "&raquo;";
        }
    }
}

Expected Behaviour

I have no idea because I haven't been able to get rector to ever work on any of the projects I've tried it on, so I hope that this time is different and I do get it to work on my current project.

[TomasVotruba]

I tried it running on the file content locally, but with no error.

What happens when you analyze only the file?

-vendor/bin/rector process . --set php74 --dry-run --verbose
+vendor/bin/rector process absolute/file/path.php --set php74 --dry-run --verbose

Also, you should never use . in any command. Always specify the directories and files. It can analyse vendor and break on so many cases.

[AntonioCS]

Ok so I followed your advice and got the same error:

root@4cf0cb2b2ccd:/projectName/common# vendor/bin/rector process admin/CategorySelection/CategorySelection.php --set php74 --dry-run --verbose
Rector v0.7.6

[parsing] admin/CategorySelection/CategorySelection.php
[refactoring] admin/CategorySelection/CategorySelection.php

In NodeNameResolver.php line 81:

  [Rector\Core\Exception\ShouldNotHappenException]               
  Pick more specific node than "PhpParser\Node\Expr\MethodCall"  

[TomasVotruba]

Does it persist in non-phar version?

[AntonioCS]

I am using the phar version because of conflict issues, but I can try tomorrow to fix all the issues and try the non-phar version.
Thanks

[TomasVotruba]

I tried the prefixed rector with no change. It seems like you've some difference locally.

We need failing test case demo here: https://github.com/rectorphp/rector-prefixed
Just copy workflow and add your test case.

The code should be isolated from the rest and minimalistic, e.g. (what I tried):

<?php

namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

final class ConstructorOnCall
{
    protected $categories = [];

    public function __construct($category_mod)
    {
        $this->categories = Categories::getInstance($category_mod)->getCategories();
    }
}

class Categories
{
    public static function getInstance(): self
    {
        return new Categories();
    }

    public function getCategories(): array
    {
        return [1, 2, 3];
    }
}

[AntonioCS]

Did you try this with the same version of php? (using the php docker image from docker hub)

[TomasVotruba]

I don't use Docker. Failing CI is need to verify though

[AntonioCS]

So I have removed my composer.lockand changed my composer.json to basically this:

    "require": {
        "php": ">=7.4",

      "ext-json": "*"
    },
    "require-dev": {
        "rector/rector": "^0.7.6"
    },

after running composer install I got this:

tracy/tracy suggests installing https://nette.org/donate (Please support Tracy via a donation)
nette/utils suggests installing ext-intl (to use Strings::webalize(), toAscii(), normalize() and compare())
symfony/console suggests installing symfony/lock
symfony/polyfill-intl-idn suggests installing ext-intl (For best performance)
symfony/var-dumper suggests installing ext-intl (To show region name in time zone dump)
symfony/http-kernel suggests installing symfony/browser-kit
symfony/dependency-injection suggests installing symfony/expression-language (For using expressions in service container configuration)
symfony/dependency-injection suggests installing symfony/proxy-manager-bridge (Generate service proxies to lazy load them)
nette/http suggests installing nette/security (allows use Nette\Http\UserStorage)
nette/application suggests installing nette/forms (Allows to use Nette\Application\UI\Form)
nette/application suggests installing latte/latte (Allows using Latte in templates)

not saying there is anything wrong.. but installing the latest version of rector I would assume I wouldn't be getting any of these (I am just documenting this here in case this is needed for something)

[AntonioCS]

This seems to be an issue with the latest version of php. I am trying this on php 7.3 and I am not getting the exception

[TomasVotruba]

Closing as unable to reproduce. Failing test/CI is needed

[lptn]

@TomasVotruba
I have the same problem on PHP 7.4.4

How about running CI tests on PHP 7.4 also?

[TomasVotruba]

Sure, send PR

[HenkPoley]

Also seeing [Rector\Core\Exception\ShouldNotHappenException] Pick more specific node than "PhpParser\Node\Expr\MethodCall" under PHP 7.3.17 & Rector v0.7.21 though.

Is it possible to get an even more verbose Exception trace? E.g. with some extra info, like variables used in the functions, so I know which thing in my 1400 line file it's tripping over?

[TomasVotruba]

I'd try --debug

Faster way is keep deleting half of the 1400 lines lines,
until you find the one responsible.
Then create a demo with that file content: https://getrector.org/demo, so we can fix it

[HenkPoley]

--debug shows the same stack strace (but less)

The following file content gives an exception for me on this commandline: php7.3 vendor/bin/rector process app/Some/Names/SomeClass.php --set php52 --dry-run --verbose

<?php

namespace Some\Names;

abstract class SomeClass
{
    protected function someFunction()
    {
        $this->{$this->foo()};
    }
}

Rector v0.7.21
Config file: rector.yaml

[parsing] app/Some/Names/SomeClass.php

In NodeNameResolver.php line 96:

  [Rector\Core\Exception\ShouldNotHappenException]               
  Pick more specific node than "PhpParser\Node\Expr\MethodCall"  


Exception trace:
  at /some/path/vendor/rector/rector/packages/node-name-resolver/src/NodeNameResolver.php:96
 Rector\NodeNameResolver\NodeNameResolver->getName() at /some/path/vendor/rector/rector/packages/node-collector/src/NodeCollector/ParsedPropertyFetchNodeCollector.php:56
 Rector\NodeCollector\NodeCollector\ParsedPropertyFetchNodeCollector->collect() at /some/path/vendor/rector/rector/packages/node-collector/src/NodeVisitor/NodeCollectorNodeVisitor.php:50
 Rector\NodeCollector\NodeVisitor\NodeCollectorNodeVisitor->enterNode() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:123
 PhpParser\NodeTraverser->traverseNode() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
 PhpParser\NodeTraverser->traverseArray() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
 PhpParser\NodeTraverser->traverseNode() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
 PhpParser\NodeTraverser->traverseArray() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
 PhpParser\NodeTraverser->traverseNode() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
 PhpParser\NodeTraverser->traverseArray() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
 PhpParser\NodeTraverser->traverseNode() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
 PhpParser\NodeTraverser->traverseArray() at /some/path/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:91
 PhpParser\NodeTraverser->traverse() at /some/path/vendor/rector/rector/packages/node-type-resolver/src/NodeScopeAndMetadataDecorator.php:152
 Rector\NodeTypeResolver\NodeScopeAndMetadataDecorator->decorateNodesFromFile() at /some/path/vendor/rector/rector/src/Application/FileProcessor.php:166
 Rector\Core\Application\FileProcessor->parseAndTraverseFileInfoToNodes() at /some/path/vendor/rector/rector/src/Application/FileProcessor.php:103
 Rector\Core\Application\FileProcessor->parseFileInfoToLocalCache() at /some/path/vendor/rector/rector/src/Application/RectorApplication.php:135
 Rector\Core\Application\RectorApplication->Rector\Core\Application\{closure}() at /some/path/vendor/rector/rector/src/Application/RectorApplication.php:210
 Rector\Core\Application\RectorApplication->tryCatchWrapper() at /some/path/vendor/rector/rector/src/Application/RectorApplication.php:136
 Rector\Core\Application\RectorApplication->runOnFileInfos() at /some/path/vendor/rector/rector/src/Console/Command/ProcessCommand.php:242
 Rector\Core\Console\Command\ProcessCommand->execute() at /some/path/vendor/symfony/console/Command/Command.php:255
 Symfony\Component\Console\Command\Command->run() at /some/path/vendor/rector/rector/src/Console/Command/AbstractCommand.php:34
 Rector\Core\Console\Command\AbstractCommand->run() at /some/path/vendor/symfony/console/Application.php:1001
 Symfony\Component\Console\Application->doRunCommand() at /some/path/vendor/symfony/console/Application.php:271
 Symfony\Component\Console\Application->doRun() at /some/path/vendor/rector/rector/src/Console/Application.php:101
 Rector\Core\Console\Application->doRun() at /some/path/vendor/symfony/console/Application.php:147
 Symfony\Component\Console\Application->run() at /some/path/vendor/rector/rector/bin/rector:54

process [-n|--dry-run] [-a|--autoload-file AUTOLOAD-FILE] [--match-git-diff] [-r|--only ONLY] [-o|--output-format [OUTPUT-FORMAT]] [--no-progress-bar] [--output-file OUTPUT-FILE] [--cache-debug] [--clear-cache] [--] [<source>...]

But it doesn't on the demo rector.

I highly suspect the ->{..} construct is the culprit. As since if I remove that, the problem goes away.

[TomasVotruba]

Great, that's more helpful. Could you open a new issue from your last comment?

If you'd find single responsible rule (not full php52 set), that would be awesome

[HenkPoley]

php52 is like 2 rules: https://github.com/rectorphp/rector/blob/master/config/set/php/php52.yaml

services:
    Rector\Php52\Rector\Property\VarToPublicPropertyRector: null
    Rector\Php52\Rector\Switch_\ContinueToBreakInSwitchRector: null

How do I execute 1 rule? Create a set somewhere and execute it? Like --set{full path}?

--only=\Rector\Php52\Rector\Property\VarToPublicPropertyRector --dry-run --verbose doesn't work

[HenkPoley]

I edited the php52.yaml, they crash on either. You may pick :P

It's probably in the parser and not the rules.

[TomasVotruba]

It's only 2 rules? :D Then it's fine. Other sets have 10-40 rules.

Make an issue then.

[TomasVotruba]

The --only needs correct syntax, I suspect slashes are the problem.