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

More precise RecursiveIteratorIterator::__construct() types #2835

Merged
merged 5 commits into from
Jan 3, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Dec 18, 2023

I had initially implemented it with a separate StubFilesExtension to load this more precise signature only in bleedingEdge.
later on I realized most iterator implemetations already use pretty narrow $flags/$mode types, therefore I went just with this more precise types.

closes phpstan/phpstan#10324

@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.


/**
* @param T $iterator
* @param int $mode
* @param int $flags
* @param self::LEAVES_ONLY|self::SELF_FIRST|self::CHILD_FIRST $mode
Copy link
Member

Choose a reason for hiding this comment

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

I'd put this in resources/functionMap_bleedingEdge.php.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

initially I tried that, too. turns out I need a mix of both, as the constants were missing on the stub.

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand.

Copy link
Contributor Author

@staabm staabm Dec 21, 2023

Choose a reason for hiding this comment

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

in the first iteration of this PR I tried adding it to resources/functionMap_bleedingEdge.php and did not succeed (without the additional change in iterable stub). now with the PR as is it works

Copy link
Member

Choose a reason for hiding this comment

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

I'm interested in why you didn't succeeed. It should be fine.

@staabm
Copy link
Contributor Author

staabm commented Dec 21, 2023

phpstan-symfony errors are fixed with phpstan/phpstan-symfony#375

Comment on lines -163 to -164
* @param int $mode
* @param int $flags
Copy link
Contributor Author

@staabm staabm Dec 21, 2023

Choose a reason for hiding this comment

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

it seems as soon as I keep these phpdocs here, the signature from resources/functionMap_bleedingEdge.php does not work, as it starts failling

There was 1 failure:

1) PHPStan\Rules\Classes\InstantiationRuleTest::testBug10324
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'23: Parameter #3 $flags of class RecursiveIteratorIterator constructor expects 0|16, 2 given.
+'
 '

/Users/staabm/workspace/phpstan-src/src/Testing/RuleTestCase.php:159
/Users/staabm/workspace/phpstan-src/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php:466

@ondrejmirtes
Copy link
Member

Makes sense now, thank you.

@ondrejmirtes ondrejmirtes merged commit 5d67f0c into phpstan:1.10.x Jan 3, 2024
424 of 426 checks passed
@staabm staabm deleted the it branch January 3, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants