-
Notifications
You must be signed in to change notification settings - Fork 657
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
numeric-string + '' not working as expected #6646
Comments
I found these snippets: https://psalm.dev/r/2e6b02d53a<?php
/**
* @param numeric $price
*
* @return string
*
* @phpstan-return ''|numeric-string
*/
function foo($price): string {
// ...
/** @var ''|numeric-string $price | hack for this example */
$price = $price;
return $price;
}
/**
* @param numeric-string $price
*/
function bar($price): void {
echo $price;
}
// ----------------------------
$price = foo('123');
if ($price === '') {
$price = '0';
}
bar($price);
https://psalm.dev/r/a3c64dd3fe<?php
/**
* @param numeric $price
*
* @return string
*
* @phpstan-return null|numeric-string
*/
function foo($price): ?string {
// ...
/** @var null|numeric-string $price | hack for this example */
$price = $price;
return $price;
}
/**
* @param numeric-string $price
*/
function bar($price): void {
echo $price;
}
// ----------------------------
$price = foo('123');
if ($price === null) {
$price = '0';
}
bar($price);
|
This is the work of TypeCombiner::combine When Psalm tries to infer types, it'll often stumble on cases like these: A specific type in one branch and a generic one in another, or two subtypes of the same surtype. To prevent Psalm to infer horrors like I'd personally not be opposed to treating the empty string as a special case and keep it along with NumericString, but if we do, I'd expect the same thing for other Strings: TLowercaseString, TNonFalsyString... I won't work on that but you're welcome to give it a go :) |
I found these snippets: https://psalm.dev/r/84eeabb411<?php
function scope(){
if(rand(0,1)){
/** @var numeric-string */
$a = '1';
return $a;
} else {
return '';
}
}
function scope2(){
if(rand(0,1)){
/** @var positive-int */
$a = 1;
return $a;
} else {
return -1;
}
}
function scope3(){
if(rand(0,1)){
/** @var lowercase-string */
$a = 'p';
return $a;
} else {
return 'P';
}
}
function scope4(){
if(rand(0,1)){
/** @var string */
$a = 'a';
return $a;
} else {
return 'P';
}
}
|
I currently see some false-positive errors from psalm:
https://psalm.dev/r/2e6b02d53a
EDIT: It looks like
''|numeric-string
is not working at all, or? If I replace''
withNULL
, then it's working: https://psalm.dev/r/a3c64dd3feThe text was updated successfully, but these errors were encountered: