implemented str_contains FunctionTypeSpecifyingExtension#1068
implemented str_contains FunctionTypeSpecifyingExtension#1068ondrejmirtes merged 2 commits intophpstan:1.6.xfrom
str_contains FunctionTypeSpecifyingExtension#1068Conversation
|
It's not ideal but this feature has to wait because it can lead to these kinds of errors:
|
|
What do you think about enabling it via bleedingEdge?
didn't work... not sure why yet |
|
did some debugging and found out, that the just added a test and a fix for that |
There was a problem hiding this comment.
optimal would be a non-empty-string here.. but the extension as-is cannot deliver it atm.
I have no idea yet, how a type-specifying extension could evaluate a comparision like if (strstr($s, ':', true) === 'hallo') {.
atm I am only verifying a plain string type, to make sure the new extension does not scramble the existing type in this scenario
There was a problem hiding this comment.
Comparison based inference can be implemented like df27a12 but === should be preferred over ==
|
the PR now contains various str-containing functions, no longer a |
|
The problem here is still this: #1068 (comment) Unfortunately |
ondrejmirtes
force-pushed
the
str-contains
branch
from
6fd372d to
c0dcd57
Compare
|
Looks like my idea is going to work :) /cc @herndlm |
|
Thank you! |
| $args[$needleArg]->value, | ||
| new String_(''), | ||
| ), | ||
| new FuncCall(new Name('FAUX_FUNCTION'), [ |
There was a problem hiding this comment.
thx for getting this over the finishing line <3.
could you elaborate a bit, what this FAUX_FUNCTION thing is about?
is this a kind-of workaround, which should be encapsulated in a helper somehow, so the code gets readable for non-phpstan-hard-core users :-)?
There was a problem hiding this comment.
it's kind of the soft force flag xD but I agree, it is hard to understand and workaround-ish, maybe it should end up in some kind of helper or smth like that hmm
There was a problem hiding this comment.
on the other hand this is veeery special and the expression is different for each case maybe
There was a problem hiding this comment.
The thing that's problematic for str_contains() type-specifying extensions and similar is that PHPStan thinks the function is is_string_non_empty() because it narrows down the type to non-empty-string. But there are other properties of the narrowed string than that - in this case that it contains another string which can't be expressed by the PHPStan typesystem.
So when you call str_contains($nonEmptyString) PHPStan thinks it's is_string_non_empty($nonEmptyString) and that it has to be always true. But in fact it should be is_string_non_empty($nonEmptyString) && string_contains_string(...). So that's what I'm simulating here.
I don't think we need a helper here.
I have not quite decided yet if I really like that modified expression thing. Potentially we then have x different ways of working around that. I also found phpstan/phpstan-webmozart-assert#134 (double executing code would not report, but that's maybe ok to live with) |
4 participants
first str-family-function to infer
non-empty-stringtype.Initially had the idea of putting everything into a single StrFamily* extension, but then figured that the semantics between substr, substring, strpos, stripos, str_contains etc are too different, and it would be more readable to put each of them into a separate extension.
therefore this is the first PR. if merged, I will work on similar semantics for the remaining functions
refs phpstan/phpstan#6792