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

BoundMethod::call is broken. #26886

Closed
dkulyk opened this issue Dec 17, 2018 · 6 comments
Closed

BoundMethod::call is broken. #26886

dkulyk opened this issue Dec 17, 2018 · 6 comments

Comments

@dkulyk
Copy link
Contributor

dkulyk commented Dec 17, 2018

  • Laravel Version: 5.7.18
  • PHP Version: 5.2
  • Database Driver & Version: MariaDB 10.2

Steps To Reproduce:

\Illuminate\Hashing\HashManager - is an example.

app()->call(function(\Illuminate\Hashing\HashManager $hash){
}, ['log'=>'not a HashManager type']);

//Also
app()->call(function(\Illuminate\Hashing\HashManager $hash){
}, ['log' => 'not a HashManager type', 'hash' => new \Illuminate\Hashing\HashManager]);

Throws exception.

TypeError: Argument 1 passed to {closure}() must be an instance of Illuminate/Hashing/HashManager, string given

Works if:

  • $log exists in function parameters;
  • additional arguments is empty;

Related #26852

@driesvints
Copy link
Member

Where/why are you calling the HashManager like that? I don't think it's suppose to work like this?

@imanghafoori1
Copy link
Contributor

imanghafoori1 commented Dec 18, 2018

Is this kind of usage documented anywhere or is there any usage in laravel source code like this ?!

Anyway I get it, you expect the 'log' to be ignored automatically yes ?!

@gustavofabiane
Copy link

Like @dkulyk I was using the container 'call()' to call methods with parameter binding like in 'make/makeWith' (Here), where the given parameters array keys are checked with the reflected parameters names from class constructor to be injected automatically.

Actually this behavior still working in BoundMethod as previously expected but the parameter inversion in this commit breaks it.

This line in BoundMethod class (153) ensures it:

        if (array_key_exists($parameter->name, $parameters)) {
            $dependencies[] = $parameters[$parameter->name];

@Quintile
Copy link

After updating my Lumen app to include the BoundMethod change, controller injections have stopped working. If I go into BoundMethod.php and revert the changes made here: 881be81#diff-62fd60bbf5999df1027491ff843c6812L119 everything works again.

For example I've got a controller method:
public function destroy(Buckets $buckets, int $id): JsonResponse

With this commit, it throws an exception that destroy is expecting an instance of Buckets, but got string.

Once reverted, everything works as expected.

@sebastianmulders
Copy link

@taylorotwell
Copy link
Member

Reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants