Skip to content

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Nov 21, 2021

Closes #46 by showing that this has been fixed already. I think. If wrong - maybe somebody can point me in the direction to reproduce it to create a failing test?

@ondrejmirtes
Copy link
Member

Can you bisect when this was fixed and why it didn't work on April 23rd? (The date on the original issue.) Basically I want you to find the commit where the test still fails, thanks.

@herndlm
Copy link
Contributor Author

herndlm commented Nov 22, 2021

Strange, same result on all 0.12.8 (Dec 20), 0.12.9, 0.12.10, 0.12.11, 0.12.12 (Jan), 0.12.13, 0.12.14, 0.12.15 (July), 0.12.16 (Aug). I get

 ------ --------------------------------------------------------------- 
  Line   foo.php                                                        
 ------ --------------------------------------------------------------- 
  13     Dumped type: array<array('id' => int)>                         
  17     Dumped type: array<int, array<string, mixed>&hasOffset('id')>  
  21     Dumped type: array<array&hasOffset('id')>                      
 ------ --------------------------------------------------------------- 

Same thing by requiring the same version of phpstan that was reported (0.12.84)

Can this be something else? Not sure what else could be checked if I can't even find the "bad" state basically to do a real bisect 🤔

@ondrejmirtes
Copy link
Member

No idea, but issue needs to be reproduced before we can consider it fixed.

@herndlm
Copy link
Contributor Author

herndlm commented Nov 22, 2021

@waltertamboer it's been a while since you opened the issue, but do you maybe still know more about it?

@ondrejmirtes
Copy link
Member

There's now a conflict in this PR, please rebase, thanks.

@herndlm herndlm force-pushed the feature/add-all-key-exists-tests branch from 160a6b4 to 399933c Compare November 29, 2021 12:55
@ondrejmirtes ondrejmirtes merged commit 6371a58 into phpstan:master Nov 29, 2021
@ondrejmirtes
Copy link
Member

Thank you.

@herndlm herndlm deleted the feature/add-all-key-exists-tests branch November 29, 2021 12:57
@waltertamboer
Copy link

Sorry @herndlm - Missed the notifications, good job 👍

@herndlm
Copy link
Contributor Author

herndlm commented Nov 29, 2021

@waltertamboer no worries, I did not fix anything here though. I just added tests that show that it is working fine right now. And I was not able to find out why or when it was broken.
Please let us know if the original issue is still relevant!

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.

Internal type conversion from array to iterable.
3 participants