Allow EntityGenerator to create PHP7 compliant methods #5569
Conversation
|
How is strict mode related to return/parameter types? Those are two totally separate things. |
|
This PR covers methods return strict mode and scalar type hinting. |
| @@ -1380,15 +1432,26 @@ protected function generateEntityStubMethod(ClassMetadataInfo $metadata, $type, | |||
| $methodTypeHint = '\\' . $typeHint . ' '; | |||
| } | |||
|
|
|||
| $methodReturnType = null; | |||
| if ($this->strictTypes) { | |||
There was a problem hiding this comment.
Sorry for not being specific - meant to add a comment to this line. This seems you are mixing two things.
By strict types, we refer to declare(strict_types=<value>), but absence of this declaration does not imply absence of return types. It's perfectly ok to have return types but no strict declaration (thus having weak types).
There was a problem hiding this comment.
I understand.
Is it ok if I add a specific option for 'strict_types' and another one for return types and scalar type hinting ?
Or only an option for return types and forget strict_types mode declaration.
There was a problem hiding this comment.
+1 for two options, one for strict types and another one for return types/scalar type hint
jdecool
changed the title
|
I just pushed 2 commits to create 2 options |
|
This way seems better, I'm not ok of assuming the return types but it's not a problem 👍 |
|
👍 |
1 similar comment
|
👍 |
| */ | ||
| public function setStrictTypes($bool) | ||
| { | ||
| if ($bool && version_compare(PHP_VERSION, '7.0.0', '<')) { |
There was a problem hiding this comment.
Are there any use-cases where people might need to generate the source from a different machine (possibly with a different PHP version) than where they plan to use it?
There was a problem hiding this comment.
Yes, of course, developers often use diferent versions... :)
There was a problem hiding this comment.
Agree with @DHager: this block isn't needed.
|
I just push code improvement with your feedbacks. |
|
What's the state of this PR? Is anything missing? Do you need help, @jdecool? |
|
I need to add some PHP 7.1 features and rebase the branch (to fix conflicts). I unfortunately remove the fork in my account. I'll make it this week. |
|
@jdecool you can fetch this branch by adding |
|
or also just |
|
Thanks @Ocramius I will update this PR quickly |
|
I just retrieve this PR in a new fork and rebase the branch. @Ocramius Do you know if it's possible ? Or do I need to create a new PR ? |
|
I would like to finish my work on this PR. But does somebody know how can I push my "new fork" which is based on this PR without recreate a new PR ? |
|
@jdecool I'm afraid you can't, when you deleted your old fork, GitHub lost reference to it. :/ You'll have to create new PR unfortunately. |
|
Ok @Majkl578 Thanks for your answer. I will recreate a new PR tomorrow. Thanks. |
|
Because I can't work on this PR anymore due to the deletion of the initial fork, I've recreate a new PR #6832. Sorry for the duplicate. |
PHP files are in weak type-checking mode. Since PHP 7, strict type-checking mode is used for function calls and return statements in the the file.
This PR add support for generate class with are strict_types compliant.