Rector: Bug: laravel57 ruleset ArgumentRemoverRector rule seems incorrectly configured

Created on 10 Jun 2019  路  2Comments  路  Source: rectorphp/rector

| Subject | Details |
| :------------- | :----------------------------------------------------------- |
| PHP version | PHP 7.2 |
| Full Command | vendor/bin/rector --config rector.yml process SomeClass.php -n |

# rector.yml
parameters:
  php_version_features: '7.2'

imports:
  - { resource: '../../vendor/rector/rector/config/level/laravel/laravel57.yaml' }

The internal rule causing incorrect behaviour:

    Rector\Rector\Argument\ArgumentRemoverRector:
        Illuminate\Foundation\Application:
            1:
                name: 'options'

See: https://github.com/rectorphp/rector/blob/master/config/set/laravel/laravel57.yaml#L23

Current Behaviour

Rector v0.5.5
Config file: /my-project/rector.yml

 3/3 [鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔鈻撯枔] 100%


 [ERROR] Could not process "Support/Localization/LocaleAccessor.php" file by "Rector\Rector\AbstractRector", due to:
         "Argument 2 passed to Rector\Rector\AbstractRector::isName() must be of the type string, integer given, called
         in /my-project/vendor/rector/rector/src/Rector/Argument/ArgumentRemoverRector.php on line 78".

Minimal PHP Code Causing Issue

<?php

use Illuminate\Foundation\Application;

class SomeClass
{
    /** @var Application */
    protected $application;

    public function __construct()
    {
        $this->application->asd();
    }
}

Expected Behaviour

I'm not sure what the configured rule is trying to accomplish:

    Rector\Rector\Argument\ArgumentRemoverRector:
        Illuminate\Foundation\Application:
            1:
                name: 'options'

But looking at the refactor method of ArgumentRemoverRector:

    public function refactor(Node $node): ?Node
    {
        foreach ($this->positionsByMethodNameByClassType as $type => $positionByMethodName) {
            if (! $this->isType($node, $type)) {
                continue;
            }

            foreach ($positionByMethodName as $methodName => $positions) {
                if (! $this->isName($node, $methodName)) {
                    continue;
                }

                foreach ($positions as $position => $match) {
                    $this->processPosition($node, $position, $match);
                }
            }
        }

        return $node;
    }

we can determine the following:

$this->positionsByMethodNameByClassType = [
  'Illuminate\Foundation\Application' => [1 => ['name' => 'options]]
]

// which means in foreach $this->positionsByMethodNameByClassType
$type = 'Illuminate\Foundation\Application'
$positionByMethodName = [1 => ['name' => 'options]]

// and within foreach $positionByMethodName
$methodName = 1
$positions = ['name' => 'options]

// which crashes
$this->isName($node, $methodName);
// since second parameter of isName should be string, (int)(1) given

That is - if I'm correct.
Can you check what is supposed to happen?

bug

All 2 comments

There is method name missing in a config :man_facepalming:

See: https://laravel.com/docs/5.7/upgrade

image

Thanks for reporting :+1:

Thanks for fixing this and the other reports this quickly!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

lstrojny picture lstrojny  路  4Comments

carusogabriel picture carusogabriel  路  4Comments

DaveLiddament picture DaveLiddament  路  5Comments

jakzal picture jakzal  路  4Comments

HDVinnie picture HDVinnie  路  3Comments