numeric-string + '' not working as expected

[voku]

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 '' with NULL, then it's working: https://psalm.dev/r/a3c64dd3fe

[psalm-github-bot]

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);

Psalm output (using commit c6fb810):

ERROR: ArgumentTypeCoercion - 31:5 - Argument 1 of bar expects numeric-string, parent type non-empty-string provided

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);

Psalm output (using commit c6fb810):

No issues!

[orklah]

This is the work of TypeCombiner::combine

When Psalm tries to infer types, it'll often stumble on cases like these:
https://psalm.dev/r/84eeabb411

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 numeric-string|lowercase-string|'B'|'', some kind of collapsing is performed in the TypeCombiner. The rules are coded here: https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/Type/TypeCombiner.php#L1000
Namely, if we have a TNumericString along with literal strings that are not numeric, we collapse both under TString

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 :)

[psalm-github-bot]

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';
    }
}

Psalm output (using commit d540320):

INFO: MissingReturnType - 3:10 - Method scope does not have a return type, expecting string

INFO: MissingReturnType - 13:10 - Method scope2 does not have a return type, expecting int

INFO: MissingReturnType - 23:10 - Method scope3 does not have a return type, expecting string

INFO: MissingReturnType - 33:10 - Method scope4 does not have a return type, expecting string