Rector: CakePHP - failing on certain levels

[micdobro]

I tried rector today for the first time. I am running on dev-master@40ad334 and nikic/php-parser from 4.1.x-dev.
Some levels are passing on --dry-run, but for example --cakephp34 or --cakephp37 are going down with variation of the following:

[ERROR] Could not process "/src/Controller/UsersController.php" file by                 
         "Rector\Rector\MethodCall\MethodNameReplacerRector", due to:                                                   
         "Argument 1 passed to Rector\Rector\MethodCall\MethodNameReplacerRector::isInstanceOf() must implement         
         interface PhpParser\Node, null given, called in                                                                
         src/Rector/MethodCall/MethodNameReplacerRector.php on line
         71".         

Installed as described by @dereuromark. I get like 10 boxes of this type. Is there anything I can do so that we can get this under control?

My stack:

• php 7.1
• cakephp 3.7 (upgrading from 3.4)

[TomasVotruba]

Hi, could you provide failing code from UsersController.php?

I'll look on it

[micdobro]

Thanks a lot for your response ;)

I found another, much shorter file that also generates the red box at the end. Here is the code.
The funny part is that rector does show some changes for files that are also named in error boxes. I am also getting plenty of errors on CLI a'la 88%PHP Notice: Undefined index: SOMEFILENAME.

There is nothing special here apart from the fact that this controller has Admin prefix. Something must be weird somehwere - and I am afraid it could be something related to my setup?

<?php
namespace App\Controller\Admin;

use App\Controller\AppController;
use Cake\Utility\Hash;
use Cake\Core\Configure;

/**
 * Logs Controller
 *
 * @property \App\Model\Table\LogsTable $Logs
 */
class LogsController extends AppController
{

    public function index()
    {
        $this->loadModel('Users');        
        $users = $this->Users->getAdminUsers();        

        $search_params = [
            'created',
            'user_id',
            'logclass'
        ];

        $s_params = $this->Search->handleSearch($search_params);        

        $this->paginate = [
            'sortWhitelist' => ['Logs.created', 'logclass', 'message', 'user_id'],
            'finder' => [
                'logViewer' => [
                    's' => $s_params
                ],
            ],
            'order' => [
                'Logs.created' => 'desc'
            ]
        ];

        $logs = $this->paginate($this->Logs);

        $this->set(compact('users', 'logs'));         
    }

    public function view($id = null)
    {
        $this->viewBuilder()->layout('ajax');
        $settings = $this->populateSettings(); 

        if($this->request->is(['post'])) {
            $log = $this->Logs->get($id, ['contain' => ['Users']]);
            $log = $this->Logs->decorate($log); 
            $this->set(compact('log', 'settings'));             
        }

    }

}

[TomasVotruba]

Thanks for the code. I've tested it locally and it passed the:

vendor/bin/rector process MyClass.php --dry-run --level cakephp34 --debug

And think I'll need some more code. Could you narrow the code down the the line it causes the error?

Try removing parent class, the @property etc. and run it only on the file with --debug option.

The error is trivial (incorrect node resolving), just need to find the isolated code causing it so I can fix it :)

[TomasVotruba]

Installed as described by @dereuromark.

Where is that?

The funny part is that rector does show some changes for files that are also named in error boxes. I am also getting plenty of errors on CLI a'la 88%PHP Notice: Undefined index: SOMEFILENAME.

It actually process everything until it stops error. That's why your code is partially changed.

[micdobro]

As for "Where is that?":
I updated of course ver for nikic/php-parser - the article is here: https://www.dereuromark.de/2018/03/14/cakephp-3-6-is-coming/

As for narrowing down - I am starting on this soon - will post back as soon as I know/have something. Thanks :)

[TomasVotruba]

I see, Rector became more stable since then, so you should install it without dev-version: https://github.com/rectorphp/rector#install

...will post back as soon as I know/have something

Thank you :)

[dereuromark]

I updated the command in the post accordingly.

[micdobro]

Thanks @dereuromark and @TomasVotruba. I did reinstall everything and rerun everything on that particular file. It failed here:

public function view($id = null)
    {
        $this->viewBuilder()->layout('ajax'); // IF I REMOVE THIS LINE -> WORKS
        $settings = $this->populateSettings(); 

        if($this->request->is(['post', 'put', 'patch'])) {
            $log = $this->Logs->get($id, ['contain' => ['Users']]);
            $log = $this->Logs->decorate($log); 
            $this->set(compact('log', 'settings'));             
        }

    }

Removing $this->viewBuilder()->layout('ajax'); results in green OK ;)

Here is the output:

Rector\Rector\Class_\ClassReplacerRector
Rector\Rector\Class_\ClassReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Class_\ClassReplacerRector
Rector\Rector\Visibility\ChangeMethodVisibilityRector
Rector\Rector\MethodBody\NormalToFluentRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\CakePHP\Rector\MethodCall\ModalToGetSetRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Property\PropertyToMethodRector
Rector\CakePHP\Rector\MethodCall\ModalToGetSetRector
Rector\Rector\Property\PropertyNameReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Property\PropertyToMethodRector
Rector\Rector\Property\PropertyToMethodRector
Rector\CakePHP\Rector\MethodCall\ModalToGetSetRector
Rector\Rector\Property\PropertyNameReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Property\PropertyToMethodRector
Rector\Rector\Property\PropertyNameReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Property\PropertyToMethodRector
Rector\CakePHP\Rector\MethodCall\ModalToGetSetRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Property\PropertyNameReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\CakePHP\Rector\MethodCall\ModalToGetSetRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Class_\ClassReplacerRector
Rector\Rector\Visibility\ChangeMethodVisibilityRector
Rector\Rector\MethodBody\NormalToFluentRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\Class_\ClassReplacerRector
Rector\CakePHP\Rector\MethodCall\ModalToGetSetRector
Rector\CakePHP\Rector\MethodCall\ModalToGetSetRector
Rector\Rector\MethodCall\MethodNameReplacerRector
Rector\Rector\MethodCall\MethodNameReplacerRector

In MethodNameReplacerRector.php line 96:

  Argument 1 passed to Rector\Rector\MethodCall\MethodNameReplacerRector::isInstanceOf() must implement interface PhpParser\Node, null given, called in /home/mic/work/dev/bazinga/vendor-bin/rector/vendor/rector/rector/src/Rector/  
  MethodCall/MethodNameReplacerRector.php on line 71              

And now comes the best of it. When I change the line to:

$this->viewBuilder()->setLayout('ajax');

Then it passes.

[TomasVotruba]

Perfect, that's what I needed :+1: I'll check that in the afternoon

[TomasVotruba]

I've added isolated test and it passes: https://github.com/rectorphp/rector/pull/839/files#diff-019b716ceb16b25c8b86df68eeee7ce8

-$this->viewBuilder()->layout('ajax');
+$this->viewBuilder()->setLayout('ajax');

I've released a new version. Since now there is no isInstanceOf() on line 71: https://github.com/rectorphp/rector/blob/f7bf21fec3a6140ebb1808c2a07f0c71766c2c6e/src/Rector/MethodCall/MethodNameReplacerRector.php#L71

Try https://github.com/rectorphp/rector/releases/tag/v0.3.25

[micdobro]

Sorry for silence on my side - will be testing tonight.

[micdobro]

All works. Thank you very much for your prompt help :) :+1:

[TomasVotruba]

Thanks for confirmation :)