Skip to content

Conversation

@VincentLanglet
Copy link
Contributor

@VincentLanglet
Copy link
Contributor Author

Params names are coming from https://www.php.net/manual/en/function.extract.php
They are also here:
https://github.com/php/php-src/blob/7e45e57d8f85f8d1fa2521028f394da30268187d/ext/standard/basic_functions.stub.php#L1643-L1644

I added some tests to check & does not break the extract inference.

Is something more needed for this PR ? @ondrejmirtes

@thg2k
Copy link
Contributor

thg2k commented Oct 12, 2024

The param names should be the ones from php7. Reflection takes care of the correct names for 8.x.

@VincentLanglet
Copy link
Contributor Author

The param names should be the ones from php7. Reflection takes care of the correct names for 8.x.

Was the param names different in php 7 ?

@claudepache
Copy link
Contributor

Was the param names different in php 7 ?

See https://3v4l.org/kMrVd . The current parameter names aren’t exactly those of PHP 7 either…

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Nov 12, 2024

Since named parameter is only a PHP 8 feature, isn't it better to use them in stubs ?
I'm not sure what's the best for PHPStan

assertVariableCertainty(TrinaryLogic::createYes(), $foo);
assertType('42', $foo);

$foo = ['foo' => 42];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would make more sense to test this separately in a different function.

@ondrejmirtes
Copy link
Member

Also: please search for @prefer-ref in vendor/phpstan/php-8-stubs and update all the functions you find too.

@VincentLanglet
Copy link
Contributor Author

Would make more sense to test this separately in a different function.

Done in f9fa213

Also: please search for @prefer-ref in vendor/phpstan/php-8-stubs and update all the functions you find too.

Done in b5e5de5 if I understood correctly

'array_merge' => ['array', 'arr1'=>'array', '...args='=>'array'],
'array_merge_recursive' => ['array', 'arr1'=>'array', '...args='=>'array'],
'array_multisort' => ['bool', '&rw_array1'=>'array', 'array1_sort_order='=>'array|int', 'array1_sort_flags='=>'array|int', '...args='=>'array|int'],
'array_multisort' => ['bool', 'rw_array1'=>'array', 'array1_sort_order='=>'array|int', 'array1_sort_flags='=>'array|int', '...args='=>'array|int'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of the parameter is array1, not rw_array1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw many other param with a wrong name, like

  • array_pop => rw_stack
  • array_push => rw_stack
  • array_shift => rw_stack
  • array_splice => rw_input
  • array_unsplice => rw_stack
  • ...

There is at least 63 occurences of &rw_ param which might need to be checked...

Do you want me to open another PR with such param renaming ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't part of the name when there's & in front of them. There's an explanation at the top of functionMap.php

@ondrejmirtes ondrejmirtes merged commit ceac309 into phpstan:1.12.x Nov 13, 2024
452 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants