Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[Proposal] Use PHP 7 return and type hinting #1034

Closed
carusogabriel opened this issue Feb 24, 2018 · 8 comments
Closed

[Proposal] Use PHP 7 return and type hinting #1034

carusogabriel opened this issue Feb 24, 2018 · 8 comments

Comments

@carusogabriel
Copy link

Hi,

Does anyone know if is there a discussion for add PHP 7 return and type hinting to Laravel's core?

If not, would it be a good idea?

@tomschlick
Copy link

tomschlick commented Feb 24, 2018 via email

@mfn
Copy link

mfn commented Feb 24, 2018

Taylor is not a fan of type hinting in general, so chances of it happening are low.

This is so weird because from all the code repos I use, the Laravel based ones are where type hinting works best (and it's part of our internal code review requirement 👍 ) and also static analyzers work well. 🤷‍♀️

@ntzm
Copy link

ntzm commented Feb 26, 2018

I think Taylor's view of type hinting is "if someone passes the invalid type, it will fail somewhere down the call stack anyway", which I think is ridiculous for several reasons:

  • It won't always fail due to PHP's loose type system, you can pass integers where strings are needed, and it will produce weird behaviour for those not familiar with PHP's type system
  • Failing early is always better, it will have a better error message
  • Better for static analysis and IDEs

I hope Taylor reconsiders his view on type hinting

@RemiCollin
Copy link

I agree with @ntzm ; Failing early is ALWAYS better. Also I have been embracing them since 1+ year and they do make my APIs more consistant, my code more readable and easier to test/maintain. I sincerely don't understand how anyone would feel the other way, or probably they just never really used them on a large project.

Yes, a call will probably fails 9 times out of 10 when not using type hinting, but the problem is when 1 time out of 10 hazardous behavior will happen, it can lead to weird bugs and, worst case scenario, security issues.

On the other hand, Laravel rely so much on dynamic typing for its magic (ie: lots of Collection methods accept both value or callbacks as first argument), that we could not possibly have type hinting for the whole framework, and upgrade path could possibly be a bit of a mess.

@mfn
Copy link

mfn commented Feb 26, 2018

Maybe Taylor is more afraid of declare(strict_types = 1);.

Unless you declare it, things usually might work out very well.

My comment in #1034 (comment) actually referred to that, too. Our whole app/ is strictly declared. Works like a charm.

@carusogabriel
Copy link
Author

Let's try mention core team before preparing the PR

@taylorotwell @GrahamCampbell @themsaid @tillkruss Can I prepare a PR, maybe multiple ones, add return types, strict_types, etc, to our code base in 5.7?

@michaeldyrynda
Copy link

I wouldn't hold my breath. So much of Laravel's power comes from leveraging the dynamic nature of PHP and accepting different inputs at runtime. Adding type hints, return types, and strict types will break a lot of that stuff.

It's a massive paradigm shift for the framework and I wouldn't expect to see it before 6.0, if at all. If you want the type safety, Laravel likely isn't for you.

@jakesylvestre
Copy link

jakesylvestre commented Oct 18, 2018

I'd love to revisit his comment/check out that blog post this laravel/framework/pull/17786 I agree with @mfn here, I think a slow rollout implementation without declare(strict_types = 1); is the way to go.

But I think @michaeldyrynda is right in that 6.0 is the earliest realistic timeline

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

No branches or pull requests

7 participants