Skip to content

Reflect from Closures#151

Merged
asgrim merged 16 commits intomasterfrom
reflect-from-closure
Jan 6, 2016
Merged

Reflect from Closures#151
asgrim merged 16 commits intomasterfrom
reflect-from-closure

Conversation

@asgrim
Copy link
Copy Markdown
Member

@asgrim asgrim commented Nov 26, 2015

Handles issue #37 by adding ability to reflect on anonymous functions / closures.

@asgrim asgrim added this to the 1.0.0 milestone Nov 26, 2015
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.

Does the fact that a closure source locator is used limit reflection of type hints?

As in:
$c = function (Foo $foo) : Bar {}

Can I get reflection for $foo and for the return parameter even if a ClosureSourceLocator was used?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 78c3f78

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.

Should probably bubble up as an exception in our own namespace?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is there much benefit? The exception message is very clear, we'd only be wrapping it anyway - plus if this issue goes away, then this test will start failing anyway (alerting us that we can change this expectation...)

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.

That is true, but making an exception from a different namespace bubble up (for a known case) will cause the consumers to type-hint against the third-party lib (without importing it manually, which is a mistake), and we would have to document all @throws with third-party lib classes (again bad)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in fbbbe7c.

asgrim added a commit that referenced this pull request Jan 6, 2016
@asgrim asgrim merged commit f608597 into master Jan 6, 2016
@asgrim asgrim deleted the reflect-from-closure branch January 6, 2016 23:47
@Ocramius
Copy link
Copy Markdown
Member

Ocramius commented Jan 6, 2016

Dang, totally forgot to merge this, sorry :|

@Ocramius Ocramius assigned asgrim and unassigned Ocramius Jan 6, 2016
@asgrim
Copy link
Copy Markdown
Member Author

asgrim commented Jan 6, 2016

Don't worry, I did it :D

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