From 46f1020e69f11427ca1042ef943f652f25e6281a Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Fri, 3 May 2024 22:29:07 +0200 Subject: [PATCH 1/4] Mixed should report error in encapsed like it does in concat Fix https://github.com/vimeo/psalm/issues/10942 --- .../Expression/EncapsulatedStringAnalyzer.php | 12 ++++++++++++ src/Psalm/Storage/ClassConstantStorage.php | 2 +- tests/BinaryOperationTest.php | 8 ++++++++ tests/SwitchTypeTest.php | 2 +- tests/TypeReconciliation/RedundantConditionTest.php | 2 +- tests/UnusedVariableTest.php | 2 +- 6 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php index 63c815ff521..fa538e22de9 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php @@ -9,6 +9,8 @@ use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\DataFlow\DataFlowNode; +use Psalm\Issue\MixedOperand; +use Psalm\IssueBuffer; use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent; use Psalm\Type; use Psalm\Type\Atomic\TLiteralFloat; @@ -52,6 +54,16 @@ public static function analyze( } $non_empty = $non_empty || $part->value !== ""; } elseif ($part_type = $statements_analyzer->node_data->getType($part)) { + if ($part_type->hasMixed()) { + IssueBuffer::maybeAdd( + new MixedOperand( + 'Operands cannot be mixed', + new CodeLocation($statements_analyzer, $part), + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + $casted_part_type = CastAnalyzer::castStringAttempt( $statements_analyzer, $context, diff --git a/src/Psalm/Storage/ClassConstantStorage.php b/src/Psalm/Storage/ClassConstantStorage.php index 072f80a0439..b36ea5a9f8f 100644 --- a/src/Psalm/Storage/ClassConstantStorage.php +++ b/src/Psalm/Storage/ClassConstantStorage.php @@ -116,7 +116,7 @@ public function getHoverMarkdown(string $const): string $types = $this->type->getAtomicTypes(); $type = array_values($types)[0]; if (property_exists($type, 'value')) { - /** @psalm-suppress UndefinedPropertyFetch */ + /** @psalm-suppress UndefinedPropertyFetch, MixedOperand */ $value = " = {$type->value};"; } } diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index e2dafc18724..9ffffcfc3e4 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -1218,6 +1218,14 @@ function foo(string $s1, string $s2): string { }', 'error_message' => 'LessSpecificReturnStatement', ], + 'encapsedMixedIssue10942' => [ + 'code' => ' 'MixedOperand', + ], ]; } } diff --git a/tests/SwitchTypeTest.php b/tests/SwitchTypeTest.php index 1560504d266..5bd7f0c52a0 100644 --- a/tests/SwitchTypeTest.php +++ b/tests/SwitchTypeTest.php @@ -245,7 +245,7 @@ function foo() : void { $x = rand() % 4; case 2: if (isset($x) && $x > 2) { - echo "$x is large"; + echo "large " . (string) $x; } break; } diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 289bc20738f..c7aa2cc1614 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -751,7 +751,7 @@ function propertyInUse(array $tokens, int $i): bool { /** @param mixed $value */ function test($value) : void { if (!is_numeric($value)) { - throw new Exception("Invalid $value"); + throw new Exception("Invalid"); } if (!is_string($value)) {} }', diff --git a/tests/UnusedVariableTest.php b/tests/UnusedVariableTest.php index 15bf8b182a3..b51cb0ae84b 100644 --- a/tests/UnusedVariableTest.php +++ b/tests/UnusedVariableTest.php @@ -486,7 +486,7 @@ function use_static() : void { if (!$token) { $token = rand(1, 10); } - echo "token is $token\n"; + echo "token is " . (string) $token; }', ], 'staticVarUsedLater' => [ From 69b7877aba5282f380056cc776d7bd7b5c4fd246 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Fri, 3 May 2024 23:24:02 +0200 Subject: [PATCH 2/4] Encapsed strings should return non-falsy-string if non-falsy like concat Fix https://github.com/vimeo/psalm/issues/10943 --- .../Expression/EncapsulatedStringAnalyzer.php | 33 ++++++++++++++++--- tests/BinaryOperationTest.php | 6 ++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php index fa538e22de9..c2b5d780f70 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php @@ -18,6 +18,7 @@ use Psalm\Type\Atomic\TLiteralString; use Psalm\Type\Atomic\TNonEmptyNonspecificLiteralString; use Psalm\Type\Atomic\TNonEmptyString; +use Psalm\Type\Atomic\TNonFalsyString; use Psalm\Type\Atomic\TNonspecificLiteralInt; use Psalm\Type\Atomic\TNonspecificLiteralString; use Psalm\Type\Atomic\TString; @@ -37,6 +38,7 @@ public static function analyze( ): bool { $parent_nodes = []; + $non_falsy = false; $non_empty = false; $all_literals = true; @@ -52,6 +54,7 @@ public static function analyze( if ($literal_string !== null) { $literal_string .= $part->value; } + $non_falsy = $non_falsy || $part->value; $non_empty = $non_empty || $part->value !== ""; } elseif ($part_type = $statements_analyzer->node_data->getType($part)) { if ($part_type->hasMixed()) { @@ -73,20 +76,37 @@ public static function analyze( if (!$casted_part_type->allLiterals()) { $all_literals = false; - } elseif (!$non_empty) { + } elseif (!$non_falsy) { // Check if all literals are nonempty - $non_empty = true; + $possibly_non_empty = true; + $non_falsy = true; foreach ($casted_part_type->getAtomicTypes() as $atomic_literal) { + if (($atomic_literal instanceof TLiteralInt && $atomic_literal->value === 0) + || ($atomic_literal instanceof TLiteralFloat && $atomic_literal->value === (float) 0) + || ($atomic_literal instanceof TLiteralString && $atomic_literal->value === "0") + ) { + $non_falsy = false; + + if ($non_empty) { + break; + } + } + if (!$atomic_literal instanceof TLiteralInt && !$atomic_literal instanceof TNonspecificLiteralInt && !$atomic_literal instanceof TLiteralFloat && !$atomic_literal instanceof TNonEmptyNonspecificLiteralString && !($atomic_literal instanceof TLiteralString && $atomic_literal->value !== "") ) { - $non_empty = false; + $possibly_non_empty = false; + $non_falsy = false; break; } } + + if ($possibly_non_empty) { + $non_empty = true; + } } if ($literal_string !== null) { @@ -131,7 +151,7 @@ public static function analyze( } } - if ($non_empty) { + if ($non_empty || $non_falsy) { if ($literal_string !== null) { $stmt_type = new Union( [Type::getAtomicStringFromLiteral($literal_string)], @@ -142,6 +162,11 @@ public static function analyze( [new TNonEmptyNonspecificLiteralString()], ['parent_nodes' => $parent_nodes], ); + } elseif ($non_falsy) { + $stmt_type = new Union( + [new TNonFalsyString()], + ['parent_nodes' => $parent_nodes], + ); } else { $stmt_type = new Union( [new TNonEmptyString()], diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index 9ffffcfc3e4..148b9532c66 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -828,6 +828,12 @@ function foo(string $s1): string { return "Hello $s1 $s2"; }', ], + 'encapsedStringIncludingLiterals3' => [ + 'code' => ' ['$interpolated===' => "non-falsy-string"], + ], 'encapsedStringIsInferredAsLiteral' => [ 'code' => ' Date: Fri, 3 May 2024 23:44:40 +0200 Subject: [PATCH 3/4] Encapsed non-literal non-empty/falsy strings always return generic string Fix https://github.com/vimeo/psalm/issues/10944 --- .../Expression/EncapsulatedStringAnalyzer.php | 17 ++++++--- tests/BinaryOperationTest.php | 36 +++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php index c2b5d780f70..6d082088072 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php @@ -76,7 +76,9 @@ public static function analyze( if (!$casted_part_type->allLiterals()) { $all_literals = false; - } elseif (!$non_falsy) { + } + + if (!$non_falsy) { // Check if all literals are nonempty $possibly_non_empty = true; $non_falsy = true; @@ -98,9 +100,16 @@ public static function analyze( && !$atomic_literal instanceof TNonEmptyNonspecificLiteralString && !($atomic_literal instanceof TLiteralString && $atomic_literal->value !== "") ) { - $possibly_non_empty = false; - $non_falsy = false; - break; + if (!$atomic_literal instanceof TNonFalsyString) { + $non_falsy = false; + } + + if (!$atomic_literal instanceof TNonEmptyString) { + $possibly_non_empty = false; + $non_falsy = false; + + break; + } } } diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index 148b9532c66..9f5b4171373 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -834,6 +834,42 @@ function foo(string $s1): string { $interpolated = "hello {$foo} bar";', 'assertions' => ['$interpolated===' => "non-falsy-string"], ], + 'encapsedStringWithoutLiterals1' => [ + 'code' => ' ['$interpolated===' => "non-falsy-string"], + ], + 'encapsedStringWithoutLiterals2' => [ + 'code' => ' ['$interpolated===' => "non-empty-string"], + ], + 'encapsedStringWithoutLiterals3' => [ + 'code' => ' ['$interpolated===' => "non-falsy-string"], + ], + 'encapsedStringWithoutLiterals4' => [ + 'code' => ' ['$interpolated===' => "non-falsy-string"], + ], 'encapsedStringIsInferredAsLiteral' => [ 'code' => ' Date: Sun, 5 May 2024 17:07:32 +0200 Subject: [PATCH 4/4] Encapsed string shouldn't lose unions of literals, like concat already does https://github.com/vimeo/psalm/issues/10945 https://github.com/vimeo/psalm/issues/10946 500 limit taken from TypeCombiner --- .../Expression/EncapsulatedStringAnalyzer.php | 61 ++++++++++++++++--- tests/BinaryOperationTest.php | 33 ++++++++++ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php index 6d082088072..c99c3fb0e5b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php @@ -24,6 +24,10 @@ use Psalm\Type\Atomic\TString; use Psalm\Type\Union; +use function array_map; +use function array_merge; +use function array_unique; +use function count; use function in_array; /** @@ -31,6 +35,8 @@ */ final class EncapsulatedStringAnalyzer { + private const MAX_LITERALS = 500; + public static function analyze( StatementsAnalyzer $statements_analyzer, PhpParser\Node\Scalar\Encapsed $stmt, @@ -43,7 +49,7 @@ public static function analyze( $all_literals = true; - $literal_string = ""; + $literal_strings = []; foreach ($stmt->parts as $part) { if (ExpressionAnalyzer::analyze($statements_analyzer, $part, $context) === false) { @@ -51,8 +57,8 @@ public static function analyze( } if ($part instanceof EncapsedStringPart) { - if ($literal_string !== null) { - $literal_string .= $part->value; + if ($literal_strings !== null) { + $literal_strings = self::combineLiteral($literal_strings, $part->value); } $non_falsy = $non_falsy || $part->value; $non_empty = $non_empty || $part->value !== ""; @@ -118,11 +124,22 @@ public static function analyze( } } - if ($literal_string !== null) { - if ($casted_part_type->isSingleLiteral()) { - $literal_string .= $casted_part_type->getSingleLiteral()->value; + if ($literal_strings !== null) { + if ($casted_part_type->allSpecificLiterals()) { + $new_literal_strings = []; + foreach ($casted_part_type->getLiteralStrings() as $literal_string_atomic) { + $new_literal_strings = array_merge( + $new_literal_strings, + self::combineLiteral($literal_strings, $literal_string_atomic->value), + ); + } + + $literal_strings = array_unique($new_literal_strings); + if (count($literal_strings) > self::MAX_LITERALS) { + $literal_strings = null; + } } else { - $literal_string = null; + $literal_strings = null; } } @@ -156,14 +173,14 @@ public static function analyze( } } else { $all_literals = false; - $literal_string = null; + $literal_strings = null; } } if ($non_empty || $non_falsy) { - if ($literal_string !== null) { + if ($literal_strings !== null && $literal_strings !== []) { $stmt_type = new Union( - [Type::getAtomicStringFromLiteral($literal_string)], + array_map([Type::class, 'getAtomicStringFromLiteral'], $literal_strings), ['parent_nodes' => $parent_nodes], ); } elseif ($all_literals) { @@ -198,4 +215,28 @@ public static function analyze( return true; } + + /** + * @param string[] $literal_strings + * @return non-empty-array + */ + private static function combineLiteral( + array $literal_strings, + string $append + ): array { + if ($literal_strings === []) { + return [$append]; + } + + if ($append === '') { + return $literal_strings; + } + + $new_literal_strings = array(); + foreach ($literal_strings as $literal_string) { + $new_literal_strings[] = "{$literal_string}{$append}"; + } + + return $new_literal_strings; + } } diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index 9f5b4171373..d9b97785c5a 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -977,6 +977,39 @@ function foo(int $s1): string { return "Hello $s1 $s2"; }', ], + 'encapsedWithUnionLiteralsKeepsLiteral' => [ + 'code' => ' ['$encapsed===' => "'0'|'2'"], + ], + 'encapsedWithUnionLiteralsKeepsLiteral2' => [ + 'code' => ' ['$encapsed===' => "'XaYbyeZ'|'XaYhelloZ'|'XaYworldZ'|'XbYbyeZ'|'XbYhelloZ'|'XbYworldZ'"], + ], + 'encapsedWithIntsKeepsLiteral' => [ + 'code' => ' ['$encapsed===' => "'a0'|'a1'|'a2'|'b0'|'b1'|'b2'"], + ], + 'encapsedWithIntRangeKeepsLiteral' => [ + 'code' => ' $b + */ + $encapsed = "{$a}{$b}";', + 'assertions' => ['$encapsed===' => "'a0'|'a1'|'a2'|'b0'|'b1'|'b2'"], + ], 'NumericStringIncrement' => [ 'code' => '