Skip to content

Implemented missing functions in ReflectionFunction#357

Merged
Ocramius merged 4 commits intoRoave:masterfrom
kukulich:reflectionfunction
Sep 5, 2017
Merged

Implemented missing functions in ReflectionFunction#357
Ocramius merged 4 commits intoRoave:masterfrom
kukulich:reflectionfunction

Conversation

@kukulich
Copy link
Copy Markdown
Collaborator

@kukulich kukulich commented Sep 5, 2017

No description provided.

@Ocramius Ocramius self-assigned this Sep 5, 2017
@Ocramius Ocramius added this to the 2.0.0 milestone Sep 5, 2017
*/
public function invokeArgs(array $args = [])
{
$this->assertIsNoClosure();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't you call it on a function?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the only possible solution for invokeArgs working for closures is to build PHP code from AST and use eval. And it will fail anyway for code like this:

<?php

class Foo
{
    private $foo = 'foo';
    
    public function boo()
    {
        return (function () {
            return $this->foo;
        })();
    }
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it. Didn't consider that the closure wasn't loaded yet.

@Ocramius Ocramius assigned kukulich and Ocramius and unassigned Ocramius and kukulich Sep 5, 2017
Copy link
Copy Markdown
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚢

@Ocramius Ocramius merged commit 4215e66 into Roave:master Sep 5, 2017
@Ocramius
Copy link
Copy Markdown
Member

Ocramius commented Sep 5, 2017

Thanks @kukulich!

@kukulich kukulich deleted the reflectionfunction branch September 5, 2017 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants