Skip to content
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.

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

Add Support of PHP 7 Syntax for MSI APIs #62

Closed
maghamed opened this issue Aug 15, 2017 · 4 comments
Closed

Add Support of PHP 7 Syntax for MSI APIs #62

maghamed opened this issue Aug 15, 2017 · 4 comments
Assignees

Comments

@maghamed
Copy link
Contributor

Add Support of

  1. Return types declaration
  2. Scalar type hinting

For Inventory API service contracts.

@maghamed maghamed changed the title Add PHP 7 Syntax for MSI APIs Add Support of PHP 7 Syntax for MSI APIs Aug 15, 2017
@larsroettig
Copy link
Member

Hi @maghamed,
I think we have two problems(issues) in the current code base.
First many Function has not one type or use string or void type
Examples:

The second issues is we use often return SourceInterface[]
but this is not allowed for PHP return types.
For every Interface[] we need some list object with a traversable implementation.

First of all, we must think how we can solve this issue and discuss the way.

Branch:
https://github.com/magento-engcom/magento2/tree/msi-php7

@maghamed
Copy link
Contributor Author

According to the guide, we provided https://github.com/magento-engcom/magento2/wiki/PHP-7-Syntax-usage-for-Magento-Contribution
we don't support in our code:

  1. Nullable types
  2. Void return type

But that does not mean that we can't have such contracts in our interfaces. The only issue that these contracts would not be strictly checked on the level of language.
But we can still declare return types in PHP Doc Blocks like this:

 * @return bool|null 

Regarding array type hinting, like this one return SourceInterface[].
It's not supported in any version of PHP neither 7.0 nor 7.1 not even going to be supported in 7.2
That's because PHP does not contain type information in the array internals. Thus, the only possibility to implement this currently is at runtime.

Some time ago there was even a proposal to fix that https://wiki.php.net/rfc/arrayof
BUT it was not accepted, because guys did not come to the agreement how to fix that correctly.
Also, the proposal was regarding PHP 5.6 Thus, there was no scalar type hinting at that moment. So, providing a type hinting just for object array was not a comprehensive decision. Here is the discussion regarding this - http://grokbase.com/t/php/php-internals/141f1kzdm8/introducing-array-of-rfc
I believe array type hinting will appear in PHP when type hinting would be supported in arrays internally.

Thus, what I propose to do is to keep the same strategy as we follow along with the Magento codebase.

  * @return  SourceInterface[]

@kadzielawa kadzielawa self-assigned this Sep 17, 2017
@naydav
Copy link

naydav commented Sep 27, 2017

Linked PR
#100

@naydav
Copy link

naydav commented Sep 28, 2017

Merged in #100

@naydav naydav closed this as completed Sep 28, 2017
mmansoor-magento pushed a commit that referenced this issue Feb 5, 2021
[Arrows] MC-40541: Admin Place Order not working as expected. MFTF test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants