Skip to content

Update AppendIterator stub.#1517

Merged
ondrejmirtes merged 2 commits intophpstan:1.8.xfrom
drupol:fix-append-iterator
Jul 18, 2022
Merged

Update AppendIterator stub.#1517
ondrejmirtes merged 2 commits intophpstan:1.8.xfrom
drupol:fix-append-iterator

Conversation

@drupol
Copy link
Copy Markdown
Contributor

@drupol drupol commented Jul 17, 2022

This PR:

  • Update the typing information of the AppendIterator stub.

Example on 3v4l: https://3v4l.org/gSPtO#v8.1.8
False positive on PHPstan: https://phpstan.org/r/36c91b68-01f7-404f-9fba-7a9e03d28953

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Jul 17, 2022

could you add the phpstan.org/try sample as NodeScopeResolver test?

@drupol
Copy link
Copy Markdown
Contributor Author

drupol commented Jul 17, 2022

NodeScopeResolver

I just added it, but I'm not convinced. The test pass in 1.8.x and this branch. I guess my modifications doesn't have any impact here. Weird.

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Jul 17, 2022

Sorry my mistake: we don't need a NodeResolverTest but one for the rule which emits the errors which can be seen in your snippet

Parameter #1 $iterator of method AppendIterator<(int|string),mixed,Iterator<TKey of (int|string), TValue>>::append() expects Iterator<TKey of (int|string), bool>, Generator<false, false, mixed, void> given.

@drupol
Copy link
Copy Markdown
Contributor Author

drupol commented Jul 17, 2022

Ok, going to remove the test.

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Jul 18, 2022

the error we see in the phpstan sandbox is emitted here:

'Parameter %s of method ' . $messagesMethodName . ' expects %s, %s given.',

therefore the repro case should be in CallMethodsRuleTest

@ondrejmirtes ondrejmirtes merged commit 9a6ebbb into phpstan:1.8.x Jul 18, 2022
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you.

@drupol drupol deleted the fix-append-iterator branch July 18, 2022 16:01
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

Successfully merging this pull request may close these issues.

3 participants