Page MenuHomePhabricator

Add support for nullable return types
ClosedPublic

Authored by joshuaspence on Sep 18 2017, 11:07 AM.

Details

Summary

Ref T4334. Adds support for nullable return type, which are a thing as of PHP 7.1.

Test Plan

Updated test cases.

Diff Detail

Repository
rPHU libphutil
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

joshuaspence created this revision.Sep 18 2017, 11:07 AM
epriestley accepted this revision.Sep 18 2017, 11:08 AM
This revision is now accepted and ready to land.Sep 18 2017, 11:08 AM

Oh cool. I'm somewhat surprised this is being accepted as I expected it to be blocked on T4334#222499.

Do you want to just delete all the tests and I'll accept those and abandon D17819?

joshuaspence added inline comments.Sep 18 2017, 11:12 AM
support/xhpast/parser.y
1089

I was trying to avoid adding a new node type (which is technically a breaking API change), but it seems necessary to accommodate nullability.

1089

This technically allows ?void, which is a fatal error... but I'm not sure it's worth special casing that here.

support/xhpast/xhpast.cpp
15

Actually, given that nullable types were added in PHP 7.1, maybe I'll bump this to 7.1.0.

Do you want to just delete all the tests and I'll accept those and abandon D17819?

I can if you want, but I think the tests are somewhat useful... if not only because they show me when a parser change has a bigger scope than I had anticipated. I think it would be good (or, at least, an improvement) to land D17819 or some variation thereupon, but I'm guessing it's not a priority.

wjiang added a subscriber: wjiang.Sep 18 2017, 11:00 PM
wjiang added inline comments.
support/xhpast/parser.y
1089

Maybe we should add a new T_VOID token and exclude void from type.

joshuaspence added inline comments.Sep 18 2017, 11:01 PM
support/xhpast/parser.y
1089

Yeah, that sounds reasonable. I'll do that in a separate diff though.

When will this be landed?

This revision was automatically updated to reflect the committed changes.