Concat should never remove "non-empty-"/"non-falsy-" from string

[kkmuffme]

https://psalm.dev/r/0baa3234b5

When concatenating a non-empty/non-falsy string with something else that is unknown (constants), the type should not "fall back", since it's wrong. And instead keep the type it already has (e.g. non-falsy-string in my example)

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/0baa3234b5

<?php

/**
 * @param non-falsy-string $arg
 * @return non-falsy-string
 */
function foo( $arg ) {
    $result = FOO . $arg;
    
    /** @psalm-trace $result */;
    
    return $result;
}

Psalm output (using commit a0ea8eb):

ERROR: UndefinedConstant - 8:15 - Const FOO is not defined

INFO: Trace - 10:32 - $result: string

INFO: LessSpecificReturnStatement - 12:12 - The type 'string' is more general than the declared return type 'non-falsy-string' for foo

INFO: MoreSpecificReturnType - 5:12 - The declared return type 'non-falsy-string' for foo is more specific than the inferred return type 'string'

[orklah]

It's handled here:

psalm/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php

Line 255 in 1c19260

if ($has_non_empty) {

It should be a pretty simple change

[EgorBakulin]

It's handled here:

psalm/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php

Line 255 in 1c19260

if ($has_non_empty) {

It should be a pretty simple change

The problem is more complex than it might seem.

psalm/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php

Line 65 in 1c19260

$left_type = $statements_analyzer->node_data->getType($left);

This variable is null for this snippets https://psalm.dev/r/f264a4732f https://psalm.dev/r/9ef452ff58

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/f264a4732f

<?php

/**
 * @param non-empty-string $arg
 * @return non-empty-string
 */
function foo( $arg ) {
    /** @psalm-suppress UndefinedConstant */
    return FOO . $arg;
}

Psalm output (using commit 1a2909e):

INFO: LessSpecificReturnStatement - 9:12 - The type 'string' is more general than the declared return type 'non-empty-string' for foo

INFO: MoreSpecificReturnType - 5:12 - The declared return type 'non-empty-string' for foo is more specific than the inferred return type 'string'

https://psalm.dev/r/9ef452ff58

<?php

/**
 * @param non-falsy-string $arg
 * @return non-falsy-string
 */
function foo( $arg ) {
    /** @psalm-suppress UndefinedConstant */
    return FOO . $arg;
}

Psalm output (using commit 1a2909e):

INFO: LessSpecificReturnStatement - 9:12 - The type 'string' is more general than the declared return type 'non-falsy-string' for foo

INFO: MoreSpecificReturnType - 5:12 - The declared return type 'non-falsy-string' for foo is more specific than the inferred return type 'string'

[orklah]

yeah, I guess it makes sense given FOO was not defined. Then

psalm/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php

Line 69 in 1c19260

if ($left_type && $right_type) {

will return false, so adding an else and setting $result_type accordingly with the known other operand should still be easy

[EgorBakulin]

#9422 I created pool request with fixes. But I newer worked with gihub pool requests. And I don't know how to link pool request to this issue.