8000 Enforce return type hints for Symfony 4+ · Issue #24882 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Enforce return type hints for Symfony 4+ #24882

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.

Sign up for GitHub

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

Jump to bottom
Closed
tomasfejfar opened this issue Nov 9, 2017 · 4 comments
Closed

Enforce return type hints for Symfony 4+ #24882

tomasfejfar opened this issue Nov 9, 2017 · 4 comments

Comments

@tomasfejfar
Copy link
Contributor
Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 4+

In #24722 there is a massive change that turns docblocks into type hints. I think it would be a good idea to make an automated test in coding standard for it.

It would make it simpler to make sure that all new features and refactorings use return type hints.

Also as more and more code gets converted to type hints it will be harder and harder to find the places where type hints are missing.

Even if it were too hard to do it all at once, it would be possible to keep in branch, rebase and use it every now and then in newly created PR adding more type hints.

What do you think? Is it feasible? I wouldn't mind making a PR for the CS check, if it makes sense.
@tomasfejfar tomasfejfar mentioned this issue Nov 9, 2017
Merged
@linaori
Copy link
Contributor
linaori commented Nov 9, 2017

What if the return type is object or mixed?

@stof
Copy link
Member
stof commented Nov 9, 2017

It would make it simpler to make sure that all new features and refactorings use return type hints.

We cannot enforce it for all refactorings, as our BC policy is more important (and this is why the previous PR has not added them everywhere).
And as we cannot enforce it, we cannot make an automated CS check ensuring it (ignoring failures from the automated check when it breaks BC is not an option, as contributors would try to make things green and then cause more harm).

But for any new API we add, we should indeed add types whenever we can. And this is nothing new: our existing policy in 3.x is already to add types whenever we can. The only difference is that requiring PHP 7.1+ in Symfony 4 means that whenever we can covers much more cases than before.

@ro0NL
Copy link
Contributor
ro0NL commented Nov 11, 2017 •
edited
Loading

About void...

I'm introducing one in https://github.com/symfony/symfony/pull/24926/files#diff-e88ad8ede40599bea0592b1113c4452cR383 :) but im leaning to reverting now..

To me a missing return type implies mixed return type. We should embrace it as of 4.1 for new public API at least. In general.. im not sure it's worth for current/new private API nor current final public API. I guess it doesnt hurt.

edit:

But for any new API we add, we should indeed add types whenever we can.

Right.. agree 👍

@ro0NL ro0NL mentioned this issue Nov 12, 2017
Closed
@nicolas-grekas
Copy link
Member

Closing as code style issues are discussed on PR, and as @stof said we embrace new language feature at a slow pace because of the long term maintenance we (the core team) endorse for the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nicolas-grekas @stof @tomasfejfar @ro0NL @linaori
0