Introduce PHP7.2 `object` type hint #112

shulard · 2017-07-03T10:24:23Z

Hello,

This PR is about introducing the PHP7.2 object type hint recently merged in the PHP core as requested in #110.

Ocramius · 2017-07-03T10:55:45Z

src/Generator/TypeGenerator.php

 */
- private static $internalPhpTypes = ['void', 'int', 'float', 'string', 'bool', 'array', 'callable', 'iterable'];
+ private static $internalPhpTypes = ['void', 'int', 'float', 'string', 'bool', 'array', 'callable', 'iterable',
+ 'object'];


Alignment is broken here

What's the right alignment here ? I try to find a "too" long array property but can't find one in the project...

Is this ok?

private static $internalPhpTypes = [ 'void', 'int', 'float', 'string', 'bool', 'array', 'callable', 'iterable', 'object' ];

Ocramius · 2017-07-03T10:56:31Z

test/Generator/MethodGeneratorTest.php

 || (
 false === strpos($parameter[2], '?')
- && ! in_array(strtolower($parameter[2]), ['void', 'iterable'])
+ && ! in_array(strtolower($parameter[2]), ['void', 'iterable', 'object'])


Careful about the php version comparison: should be 70200

Ocramius · 2017-07-03T10:57:06Z

test/Generator/ParameterGeneratorTest.php

 $compatibleParameters = array_filter(
 $parameters,
 function (array $parameter) {
 return PHP_VERSION_ID >= 70100


Same as above: different conditionals for 70200

Ocramius · 2017-07-03T10:58:02Z

test/Generator/ParameterGeneratorTest.php

 [IterableHintsClass::class, 'iterableParameter', 'foo', 'iterable'],
 [IterableHintsClass::class, 'nullableIterableParameter', 'foo', '?iterable'],
 [IterableHintsClass::class, 'nullDefaultIterableParameter', 'foo', '?iterable'],
+ [ObjectHintsClass::class, 'objectParameter', 'foo', 'object'],


These should only be added to the data providrr if the php version is 70200 or greater

Ocramius · 2017-07-03T10:58:12Z

test/Generator/MethodGeneratorTest.php

 [NullableReturnTypeHintedClass::class, 'otherClassReturn', '?\\' . InternalHintsClass::class],
 [IterableHintsClass::class, 'iterableReturnValue', 'iterable'],
 [IterableHintsClass::class, 'nullableIterableReturnValue', '?iterable'],
+ [ObjectHintsClass::class, 'objectReturnValue', 'object'],


These should only be added to the data providrr if the php version is 70200 or greater

I've updated the tests regarding PHP version.

Ocramius · 2017-07-03T11:07:30Z

Yup

…

On 3 Jul 2017 13:04, "Stéphane HULARD" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/Generator/TypeGenerator.php <#112 (comment)> : > @@ -33,7 +33,8 @@ * * @link http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration */ - private static $internalPhpTypes = ['void', 'int', 'float', 'string', 'bool', 'array', 'callable', 'iterable']; + private static $internalPhpTypes = ['void', 'int', 'float', 'string', 'bool', 'array', 'callable', 'iterable', + 'object']; What's the right alignment here ? I try to find a "too" long array property but can't find one in the project... Is this ok? private static $internalPhpTypes = [ 'void', 'int', 'float', 'string', 'bool', 'array', 'callable', 'iterable', 'object' ]; — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#112 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakOMzTKQhdY0auFDbefLE21Kdylqlks5sKMqsgaJpZM4OMGre> .

Ocramius · 2017-07-03T17:06:23Z

Couple things here:

Patch target should be changed to develop
Bumped PHP to 7.1 and updated PHPUnit to ^6.2.2 #114 was just merged, and that may cause rebase conflicts

Allow the TypeGenerator to recognize this new PHP7.2 internal type. # Conflicts: # src/Generator/TypeGenerator.php

shulard · 2017-07-03T18:00:34Z

Hello,

I've updated the target branch and rebased on the develop branch 😄.

Ocramius

LGTM! Great work, thanks!

michalbundyra · 2017-07-03T18:04:38Z

test/Generator/MethodGeneratorTest.php

+ || (
+ false === strpos($parameter[2], 'object')
+ ))
+ && (PHP_VERSION_ID >= 70100


We don't need this part of condition, as now develop supports only PHP 7.1+

michalbundyra · 2017-07-03T18:04:47Z

test/Generator/ParameterGeneratorTest.php

+ || (
+ false === strpos($parameter[3], 'object')
+ ))
+ && (PHP_VERSION_ID >= 70100


michalbundyra · 2017-07-03T18:05:37Z

@Ocramius tests still are failing !

Ocramius · 2017-07-03T18:05:39Z

Hmm, can you check up on the test failures?

shulard · 2017-07-03T18:06:46Z

I've just removed the PHP 7.1 conditions in the tests...

michalbundyra

@shulard Please see my comments.

michalbundyra · 2017-07-03T18:18:28Z

test/Generator/ParameterGeneratorTest.php

+ [ObjectHintsClass::class, 'nullDefaultObjectParameter', 'foo', '?object'],
 ];

+ $compatibleParameters = array_filter(


You have to use this variable in line 487 and 489.

michalbundyra · 2017-07-03T18:18:38Z

test/Generator/ParameterGeneratorTest.php

+ $compatibleParameters = array_filter(
+ $parameters,
+ function (array $parameter) {
+


Please remove empty line above.

michalbundyra · 2017-07-03T18:19:32Z

test/Generator/TypeGeneratorTest.php

 ['\\ITERABLE'],
+ ['\\object'],
+ ['\\Object'],
+ ['\\OBJECT'],


Here you added these to invalidTypeProvider, don't we need remove 'object' from validTypeProvider?

Introduce a new TestAsset\ObjectHintsClass as a sample to be used in test. Also update generator tests to check for object type.

shulard · 2017-07-03T18:58:38Z

Hello @webimpress,

I've updated the code with your comments and fixed the test suite. I've made a mistake in the "validTypeProvider" method...

michalbundyra

LGTM 👍 And all green now. Thanks @shulard !

Ocramius · 2017-07-03T23:36:37Z

🚢

Ocramius suggested changes Jul 3, 2017

View reviewed changes

shulard force-pushed the feature/object-type-hint branch 2 times, most recently from fb4277e to b95a4e4 Compare July 3, 2017 11:19

Ocramius added the enhancement label Jul 3, 2017

Ocramius added this to the 3.2.0 milestone Jul 3, 2017

shulard changed the base branch from master to develop July 3, 2017 17:56

Add the new object internal type.

ae7c4b6

Allow the TypeGenerator to recognize this new PHP7.2 internal type. # Conflicts: # src/Generator/TypeGenerator.php

shulard force-pushed the feature/object-type-hint branch from b95a4e4 to 754faf1 Compare July 3, 2017 17:59

Ocramius approved these changes Jul 3, 2017

View reviewed changes

michalbundyra reviewed Jul 3, 2017

View reviewed changes

shulard force-pushed the feature/object-type-hint branch from 754faf1 to c7b1922 Compare July 3, 2017 18:06

michalbundyra suggested changes Jul 3, 2017

View reviewed changes

shulard force-pushed the feature/object-type-hint branch from c7b1922 to a8acf4e Compare July 3, 2017 18:56

Update test suite to check object internal type.

b959211

Introduce a new TestAsset\ObjectHintsClass as a sample to be used in test. Also update generator tests to check for object type.

shulard force-pushed the feature/object-type-hint branch from a8acf4e to b959211 Compare July 3, 2017 18:57

michalbundyra approved these changes Jul 3, 2017

View reviewed changes

Ocramius merged commit 9f6eb29 into zendframework:develop Jul 3, 2017

Ocramius self-assigned this Jul 3, 2017

Ocramius mentioned this pull request Jul 23, 2017

Add support for PHP 7.2's object type hint #110

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Introduce PHP7.2 `object` type hint #112

Introduce PHP7.2 `object` type hint #112

Uh oh!

shulard commented Jul 3, 2017

Ocramius Jul 3, 2017

shulard Jul 3, 2017

Ocramius Jul 3, 2017

Ocramius Jul 3, 2017

Ocramius Jul 3, 2017

Ocramius Jul 3, 2017

shulard Jul 3, 2017

Ocramius commented Jul 3, 2017 via email

Ocramius commented Jul 3, 2017

shulard commented Jul 3, 2017

Ocramius left a comment

michalbundyra Jul 3, 2017

michalbundyra Jul 3, 2017

michalbundyra commented Jul 3, 2017

Ocramius commented Jul 3, 2017

shulard commented Jul 3, 2017

michalbundyra left a comment

michalbundyra Jul 3, 2017

michalbundyra Jul 3, 2017

michalbundyra Jul 3, 2017

shulard commented Jul 3, 2017

michalbundyra left a comment

Ocramius commented Jul 3, 2017

Introduce PHP7.2 object type hint #112

Introduce PHP7.2 object type hint #112

Uh oh!

Conversation

shulard commented Jul 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ocramius commented Jul 3, 2017 via email

Ocramius commented Jul 3, 2017

shulard commented Jul 3, 2017

Ocramius left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalbundyra commented Jul 3, 2017

Ocramius commented Jul 3, 2017

shulard commented Jul 3, 2017

michalbundyra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shulard commented Jul 3, 2017

michalbundyra left a comment

Choose a reason for hiding this comment

Ocramius commented Jul 3, 2017

Introduce PHP7.2 `object` type hint #112

Introduce PHP7.2 `object` type hint #112