From 2559222f67140378ec32067b0b2f8a957ae4f447 Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Fri, 24 Jun 2022 19:22:59 -0500 Subject: [PATCH] More interpolation and concatenation improvements. --- .../Statements/Expression/AssertionFinder.php | 61 ++++++++----------- .../Expression/BinaryOp/ConcatAnalyzer.php | 50 +++++++-------- .../Statements/Expression/CastAnalyzer.php | 3 +- .../Expression/EncapsulatedStringAnalyzer.php | 24 +++++--- .../FilterVarReturnTypeProvider.php | 2 +- src/Psalm/Type/Atomic/TFalse.php | 2 + src/Psalm/Type/Atomic/TTrue.php | 2 + src/Psalm/Type/Union.php | 29 +++++++++ tests/BinaryOperationTest.php | 34 +++++++++++ tests/TypeReconciliation/EmptyTest.php | 29 +++++++++ 10 files changed, 165 insertions(+), 71 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php index e2fdfa395..7d4c2808c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php @@ -3,6 +3,7 @@ namespace Psalm\Internal\Analyzer\Statements\Expression; use PhpParser; +use PhpParser\Node\Expr\BinaryOp; use PhpParser\Node\Expr\BinaryOp\Equal; use PhpParser\Node\Expr\BinaryOp\Greater; use PhpParser\Node\Expr\BinaryOp\GreaterOrEqual; @@ -103,6 +104,7 @@ class AssertionFinder 'is_iterable' => ['iterable'], 'is_countable' => ['countable'], ]; + /** * Gets all the type assertions in a conditional * @@ -1499,50 +1501,35 @@ class AssertionFinder PhpParser\Node\Expr\BinaryOp $conditional, ?int &$min_count ) { - $left_count = $conditional->left instanceof PhpParser\Node\Expr\FuncCall + if ($conditional->left instanceof PhpParser\Node\Expr\FuncCall && $conditional->left->name instanceof PhpParser\Node\Name && strtolower($conditional->left->name->parts[0]) === 'count' - && $conditional->left->getArgs(); - - $operator_greater_than_or_equal = - $conditional instanceof PhpParser\Node\Expr\BinaryOp\Greater - || $conditional instanceof PhpParser\Node\Expr\BinaryOp\GreaterOrEqual; - - if ($left_count - && $conditional->right instanceof PhpParser\Node\Scalar\LNumber - && $operator_greater_than_or_equal - && $conditional->right->value >= ( - $conditional instanceof PhpParser\Node\Expr\BinaryOp\Greater - ? 0 - : 1 - ) + && $conditional->left->getArgs() + && ($conditional instanceof BinaryOp\Greater || $conditional instanceof BinaryOp\GreaterOrEqual) ) { - $min_count = $conditional->right->value + - ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Greater ? 1 : 0); - - return self::ASSIGNMENT_TO_RIGHT; - } - - $right_count = $conditional->right instanceof PhpParser\Node\Expr\FuncCall + $assignment_to = self::ASSIGNMENT_TO_RIGHT; + $compare_to = $conditional->right; + $comparison_adjustment = $conditional instanceof BinaryOp\Greater ? 1 : 0; + } elseif ($conditional->right instanceof PhpParser\Node\Expr\FuncCall && $conditional->right->name instanceof PhpParser\Node\Name && strtolower($conditional->right->name->parts[0]) === 'count' - && $conditional->right->getArgs(); - - $operator_less_than_or_equal = - $conditional instanceof PhpParser\Node\Expr\BinaryOp\Smaller - || $conditional instanceof PhpParser\Node\Expr\BinaryOp\SmallerOrEqual; - - if ($right_count - && $conditional->left instanceof PhpParser\Node\Scalar\LNumber - && $operator_less_than_or_equal - && $conditional->left->value >= ( - $conditional instanceof PhpParser\Node\Expr\BinaryOp\Smaller ? 0 : 1 - ) + && $conditional->right->getArgs() + && ($conditional instanceof BinaryOp\Smaller || $conditional instanceof BinaryOp\SmallerOrEqual) ) { - $min_count = $conditional->left->value + - ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Smaller ? 1 : 0); + $assignment_to = self::ASSIGNMENT_TO_LEFT; + $compare_to = $conditional->left; + $comparison_adjustment = $conditional instanceof BinaryOp\Smaller ? 1 : 0; + } else { + return false; + } - return self::ASSIGNMENT_TO_LEFT; + // TODO get node type provider here somehow and check literal ints and int ranges + if ($compare_to instanceof PhpParser\Node\Scalar\LNumber + && $compare_to->value > (-1 * $comparison_adjustment) + ) { + $min_count = $compare_to->value + $comparison_adjustment; + + return $assignment_to; } return false; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php index fe2ec8b21..c88b6c305 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php @@ -28,12 +28,14 @@ use Psalm\Type; use Psalm\Type\Atomic\TFalse; use Psalm\Type\Atomic\TFloat; use Psalm\Type\Atomic\TInt; +use Psalm\Type\Atomic\TLiteralFloat; use Psalm\Type\Atomic\TLiteralInt; use Psalm\Type\Atomic\TLiteralString; use Psalm\Type\Atomic\TLowercaseString; use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Atomic\TNonEmptyNonspecificLiteralString; use Psalm\Type\Atomic\TNonEmptyString; +use Psalm\Type\Atomic\TNonspecificLiteralInt; use Psalm\Type\Atomic\TNonspecificLiteralString; use Psalm\Type\Atomic\TNull; use Psalm\Type\Atomic\TString; @@ -52,6 +54,8 @@ use function strlen; */ class ConcatAnalyzer { + private const MAX_LITERALS = 64; + /** * @param Union|null $result_type */ @@ -155,39 +159,35 @@ class ConcatAnalyzer self::analyzeOperand($statements_analyzer, $left, $left_type, 'Left', $context); self::analyzeOperand($statements_analyzer, $right, $right_type, 'Right', $context); - // If one of the types is a single int or string literal, and the other - // type is all string or int literals, combine them into new literal(s). + // If both types are specific literals, combine them into new literals $literal_concat = false; - if (($left_type->allStringLiterals() || $left_type->allIntLiterals()) - && ($right_type->allStringLiterals() || $right_type->allIntLiterals()) - ) { - $literal_concat = true; - $result_type_parts = []; + if ($left_type->allSpecificLiterals() && $right_type->allSpecificLiterals()) { + $left_type_parts = $left_type->getAtomicTypes(); + $right_type_parts = $right_type->getAtomicTypes(); + $combinations = count($left_type_parts) * count($right_type_parts); + if ($combinations < self::MAX_LITERALS) { + $literal_concat = true; + $result_type_parts = []; - foreach ($left_type->getAtomicTypes() as $left_type_part) { - assert($left_type_part instanceof TLiteralString || $left_type_part instanceof TLiteralInt); - foreach ($right_type->getAtomicTypes() as $right_type_part) { - assert($right_type_part instanceof TLiteralString || $right_type_part instanceof TLiteralInt); - $literal = $left_type_part->value . $right_type_part->value; - if (strlen($literal) >= $config->max_string_length) { - // Literal too long, use non-literal type instead - $literal_concat = false; - break 2; + foreach ($left_type->getAtomicTypes() as $left_type_part) { + foreach ($right_type->getAtomicTypes() as $right_type_part) { + $literal = $left_type_part->value . $right_type_part->value; + if (strlen($literal) >= $config->max_string_length) { + // Literal too long, use non-literal type instead + $literal_concat = false; + break 2; + } + + $result_type_parts[] = new TLiteralString($literal); } - - $result_type_parts[] = new TLiteralString($literal); } - } - if (!empty($result_type_parts)) { - if ($literal_concat && count($result_type_parts) < 64) { + if ($literal_concat) { + assert(count($result_type_parts) === $combinations); + assert(count($result_type_parts) !== 0); // #8163 $result_type = new Union($result_type_parts); - } else { - $result_type = new Union([new TNonEmptyNonspecificLiteralString]); } - - return; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php index 021b466ec..439ee3bf1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php @@ -28,6 +28,7 @@ use Psalm\Type\Atomic\TFloat; use Psalm\Type\Atomic\TInt; use Psalm\Type\Atomic\TKeyedArray; use Psalm\Type\Atomic\TList; +use Psalm\Type\Atomic\TLiteralFloat; use Psalm\Type\Atomic\TLiteralInt; use Psalm\Type\Atomic\TLiteralString; use Psalm\Type\Atomic\TMixed; @@ -327,7 +328,7 @@ class CastAnalyzer || $atomic_type instanceof TInt || $atomic_type instanceof TNumeric ) { - if ($atomic_type instanceof TLiteralInt) { + if ($atomic_type instanceof TLiteralInt || $atomic_type instanceof TLiteralFloat) { $castable_types[] = new TLiteralString((string) $atomic_type->value); } elseif ($atomic_type instanceof TNonspecificLiteralInt) { $castable_types[] = new TNonspecificLiteralString(); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php index dc6d8215a..5454f3b02 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php @@ -11,8 +11,12 @@ use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\DataFlow\DataFlowNode; use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent; use Psalm\Type; +use Psalm\Type\Atomic\TLiteralFloat; +use Psalm\Type\Atomic\TLiteralInt; +use Psalm\Type\Atomic\TLiteralString; use Psalm\Type\Atomic\TNonEmptyNonspecificLiteralString; use Psalm\Type\Atomic\TNonEmptyString; +use Psalm\Type\Atomic\TNonspecificLiteralInt; use Psalm\Type\Union; use function in_array; @@ -33,12 +37,6 @@ class EncapsulatedStringAnalyzer $literal_string = ""; foreach ($stmt->parts as $part) { - if ($part instanceof PhpParser\Node\Scalar\EncapsedStringPart - && $part->value - ) { - $non_empty = true; - } - if (ExpressionAnalyzer::analyze($statements_analyzer, $part, $context) === false) { return false; } @@ -55,11 +53,22 @@ class EncapsulatedStringAnalyzer if (!$casted_part_type->allLiterals()) { $all_literals = false; + } elseif (!$non_empty) { + // Check if all literals are nonempty + foreach ($casted_part_type->getAtomicTypes() as $atomic_literal) { + $non_empty = $atomic_literal instanceof TLiteralInt + || $atomic_literal instanceof TNonspecificLiteralInt + || $atomic_literal instanceof TLiteralFloat + || $atomic_literal instanceof TNonEmptyNonspecificLiteralString + || ($atomic_literal instanceof TLiteralString && $atomic_literal->value !== "") + ; + } } if ($literal_string !== null) { if ($casted_part_type->isSingleLiteral()) { - $literal_string .= $casted_part_type->getSingleLiteral(); + $literal_string .= $casted_part_type->getSingleLiteral()->value; + if (!$non_empty && $literal_string !== "") {} } else { $literal_string = null; } @@ -97,6 +106,7 @@ class EncapsulatedStringAnalyzer if ($literal_string !== null) { $literal_string .= $part->value; } + $non_empty = $non_empty || $part->value !== ""; } else { $all_literals = false; $literal_string = null; diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php index 0e267608a..3ebc5e1d2 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php @@ -92,7 +92,7 @@ class FilterVarReturnTypeProvider implements FunctionReturnTypeProviderInterface if (isset($atomic_type->properties['options']) && $atomic_type->properties['options']->hasArray() - && ($options_array = $atomic_type->properties['options']->getAtomicTypes()['array']) + && ($options_array = $atomic_type->properties['options']->getAtomicTypes()['array'] ?? null) && $options_array instanceof TKeyedArray && isset($options_array->properties['default']) ) { diff --git a/src/Psalm/Type/Atomic/TFalse.php b/src/Psalm/Type/Atomic/TFalse.php index 5ffd3112f..50e4a0bfd 100644 --- a/src/Psalm/Type/Atomic/TFalse.php +++ b/src/Psalm/Type/Atomic/TFalse.php @@ -7,6 +7,8 @@ namespace Psalm\Type\Atomic; */ class TFalse extends TBool { + public bool $value = false; + public function __toString(): string { return 'false'; diff --git a/src/Psalm/Type/Atomic/TTrue.php b/src/Psalm/Type/Atomic/TTrue.php index 29313a0d4..d5f2db439 100644 --- a/src/Psalm/Type/Atomic/TTrue.php +++ b/src/Psalm/Type/Atomic/TTrue.php @@ -7,6 +7,8 @@ namespace Psalm\Type\Atomic; */ class TTrue extends TBool { + public bool $value = true; + public function __toString(): string { return 'true'; diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index 8e8f7b00c..6ff34be31 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -270,6 +270,7 @@ class Union implements TypeNode } /** + * @psalm-mutation-free * @return non-empty-array */ public function getAtomicTypes(): array @@ -1302,6 +1303,34 @@ class Union implements TypeNode return true; } + /** + * @psalm-assert-if-true array< + * array-key, + * TLiteralString|TLiteralInt|TLiteralFloat|TFalse|TTrue + * > $this->getAtomicTypes() + */ + public function allSpecificLiterals(): bool + { + foreach ($this->types as $atomic_key_type) { + if (!$atomic_key_type instanceof TLiteralString + && !$atomic_key_type instanceof TLiteralInt + && !$atomic_key_type instanceof TLiteralFloat + && !$atomic_key_type instanceof TFalse + && !$atomic_key_type instanceof TTrue + ) { + return false; + } + } + + return true; + } + + /** + * @psalm-assert-if-true array< + * array-key, + * TLiteralString|TLiteralInt|TLiteralFloat|TNonspecificLiteralString|TNonSpecificLiteralInt|TFalse|TTrue + * > $this->getAtomicTypes() + */ public function allLiterals(): bool { foreach ($this->types as $atomic_key_type) { diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index 087a7f0f4..a1d761e1a 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -686,6 +686,40 @@ class BinaryOperationTest extends TestCase return "Hello $s1 $s2"; }', ], + 'encapsedStringIsInferredAsLiteral' => [ + ' ['$interpolated===' => '"12.3foobar"'], + ], + 'concatenatedStringIsInferredAsLiteral' => [ + ' ['$concatenated===' => '"12.3foobar"'], + ], + 'encapsedNonEmptyNonSpecificLiteralString' => [ + ' ['$interpolated===' => 'non-empty-literal-string'], + ], + 'concatenatedNonEmptyNonSpecificLiteralString' => [ + ' ['$concatenated===' => 'non-empty-literal-string'], + ], 'literalIntConcatCreatesLiteral' => [ ' [ // #8163 + ' */ + $arr = [1]; + assert(count($arr) === $c); + ', + 'assertions' => ['$arr===' => 'non-empty-list'], + ], + 'SKIPPED-countWithIntRange' => [ // #8163 + ' */ + $c = 1; + /** @var list */ + $arr = [1]; + assert(count($arr) === $c); + ', + 'assertions' => ['$arr===' => 'non-empty-list'], + ], + 'SKIPPED-countEmptyWithIntRange' => [ // #8163 + ' */ + $c = 1; + /** @var list */ + $arr = [1]; + assert(count($arr) === $c); + ', + 'assertions' => ['$arr===' => 'list'], + ], ]; }