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

False positive: Call to static method Webmozart\Assert\Assert::count() with arguments non-empty-array<int, string>, 2 and 'error' will always evaluate to true #68

Closed
ruudk opened this issue Dec 1, 2021 · 6 comments · Fixed by #108

Comments

@ruudk
Copy link
Contributor

ruudk commented Dec 1, 2021

Related to #62 /cc @villfa

$encryptedValue = "some value";
$valueParts = explode(':', $encryptedValue);

Assert::count(
    $valueParts,
    2,
    'Encrypted secret parameter was expected to consist of 2 parts separated by a colon'
);
// Call to static method Webmozart\Assert\Assert::count() with arguments non-empty-array<int, string>, 2 and 'error' will always evaluate to true
@ondrejmirtes
Copy link
Member

Really weird, it's not happening on equivalent code: https://phpstan.org/r/66b68119-ae4b-449d-ba3b-1ac7c8863662

@gjuric
Copy link

gjuric commented Feb 10, 2022

I am having the same issue, the sample you provided is not using Webmozart\Assert\Assert library.

@herndlm
Copy link
Contributor

herndlm commented Feb 11, 2022

@gjuric the reason for that is that this project translates the assert methods to expressions like these. And that's why most of the problems are reproducable with the expressions it would create

I was able to confirm this bug in a test, but don't have a good enough fix yet unfortunately. I'll try to take another look soon

@herndlm
Copy link
Contributor

herndlm commented Feb 16, 2022

I found out a bit more.

As we already saw in the phpstan.org snippet the type before the assertion is already non-empty-array. And Assert::count does not add any additional information here. It only finds out that the given variable has a NonEmptyArrayType. In contrary to e.g. minCount and maxCount where it also returns a sureType for count($valueParts).

The interesting thing is why the snippet code does not show the same error. I think this happens because assert has a special handling in https://github.com/phpstan/phpstan-src/blob/1.4.6/src/Rules/Comparison/ImpossibleCheckTypeHelper.php#L62.
UPDATE: this does not change anything. The main difference is more that for the assert we have an expression like count($valueParts) === 2 as argument, which resolves to a BooleanType. In contrast, for the Assert::count we have a Variable as expression which resolves to an IntersectionType here and is handled different.

Regarding the fix - I'm still not sure yet. I could easily check if the expression would not add any more typing-value here and return null then instead of the expression which avoids this error. Another "fix" would be to enable overwrite for the SpecifiedTypes which would disable all of those errors.
I'm not quite happy with either of those, maybe there is a better alternative, that makes these assertion SpecifiedTypes basically behave as the assert with its special handling. I'll try that out later.

@ondrejmirtes
Copy link
Member

Please check this once phpstan/phpstan-src#1021 is built in phpstan/phpstan, thanks.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants