Skip to content

Conversation

clxmstaab
Copy link
Contributor

@clxmstaab clxmstaab commented Feb 17, 2021

adds failing test for #39

it seems there is already some kind of keyExists handling, but it does not work yet for the added testcase

@ondrejmirtes
Copy link
Member

This issue is probably totally different, it's about ConstantArrayType::accepts() behaviour in core PHPStan.

@clxmstaab
Copy link
Contributor Author

clxmstaab commented Feb 17, 2021

This issue is probably totally different, it's about ConstantArrayType::accepts() behaviour in core PHPStan.

hmm I guess this means I am lost now. Could you give me some more pointers? or is it something very deep in the core, which a beginner will not be able to fix?

@clxmstaab
Copy link
Contributor Author

clxmstaab commented Feb 17, 2021

I already tried to narrow it down on phpstan.org

using array_key_exists directly seem to work like expected.
https://phpstan.org/r/1d951760-7d0d-47bc-bed6-429eeaa5de58

I got the impression that phpstan rewrites the ast to something like

array_key_exists('password', $data);
array_key_exists('email', $data);
a($data);

while it should be more something like

if (array_key_exists('password', $data) && array_key_exists('email', $data)) {
	a($data);
}

@ondrejmirtes
Copy link
Member

If this works https://phpstan.org/r/47c28281-644a-4193-bb02-5fa61cbcb93f then this extension has to work too...

@staabm
Copy link
Contributor

staabm commented Feb 17, 2021

If this works https://phpstan.org/r/47c28281-644a-4193-bb02-5fa61cbcb93f then this extension has to work too...

Hmm this looks like expected to me, right?

any more pointers where the error might come from?

@clxmstaab
Copy link
Contributor Author

I dont know where the difference is, but I am now able to reproduce the issue on phpstan

https://phpstan.org/r/d1f68381-095d-45c3-b89a-f9eacfa4b18b

@clxmstaab
Copy link
Contributor Author

error reported upstream: phpstan/phpstan#4560

@clxmstaab clxmstaab closed this Feb 22, 2021
@clxmstaab clxmstaab deleted the key-exists branch February 22, 2021 13:34
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