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

Revert non-empty-string changes and add regression test #89

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Jan 17, 2022

Closes #85
Includes #88
Reverts 19b869e and 9009135

Adapts the ImpossibleCheckTypeMethodCallRuleTest to use phpstan-strict-rules so that checkAlwaysTrueCheckTypeFunctionCall is enabled and we can actually regression test #85

Sorry all for all the troubles caused :/

@herndlm herndlm changed the title Revert non empty string changes and add regression test Revert non-empty-string changes and add regression test Jan 17, 2022
@herndlm herndlm force-pushed the revert-non-empty-string-changes-and-add-regression-test branch from 806d2f8 to 3775aed Compare January 17, 2022 11:48
@ondrejmirtes
Copy link
Member

Perfect, gonna release it right away. We can then talk about the solution :) Thank you.

@ondrejmirtes ondrejmirtes merged commit 67f970c into phpstan:master Jan 17, 2022
@herndlm herndlm deleted the revert-non-empty-string-changes-and-add-regression-test branch January 17, 2022 11:50
@herndlm
Copy link
Contributor Author

herndlm commented Jan 17, 2022

Thx Ondrej for the quick review, merge and release!

Let me know if you have any ideas already how to move forward with the type problems and ImpossibleCheckTypeMethodCallRule. In the context of this extension this seems to be making problems sometimes, e.g. #68 too for example (with the modified rule test here this can be reproduced now too).

In general I guess this starts to get problematic if the assertions do more than simple type checks (e.g. assertCount, assertLength or assertUuid). But I have no idea how to solve that right now unfortunately. It all comes down to phpstan's core ImpossibleCheckTypeHelper that finds the specified types and the rule that then finds out that no more information type-wise (non-empty-string is still going to be a non-empty-string after checking that it is a valid UUID for example) is added.

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.

False positive on Assert::upper() always evaluates as true
2 participants