Skip to content

Commit

Permalink
Fix int-range concat to behave like union of int types
Browse files Browse the repository at this point in the history 
Fix vimeo#10947
Fix vimeo#10965 and similar issues including their incorrect tests
  • Loading branch information
@kkmuffme
kkmuffme committed May 7, 2024
1 parent 10956df commit e70d79b
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 10 deletions.
63 changes: 57 additions & 6 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use AssertionError;
use PhpParser;
use Psalm\CodeLocation;
use Psalm\Codebase;
use Psalm\Config;
use Psalm\Context;
use Psalm\Internal\Analyzer\FunctionLikeAnalyzer;
Expand All @@ -15,6 +16,7 @@
use Psalm\Internal\Type\Comparator\AtomicTypeComparator;
use Psalm\Internal\Type\Comparator\TypeComparisonResult;
use Psalm\Internal\Type\Comparator\UnionTypeComparator;
use Psalm\Internal\Type\TypeExpander;
use Psalm\Issue\FalseOperand;
use Psalm\Issue\ImplicitToStringCast;
use Psalm\Issue\ImpureMethodCall;
Expand All @@ -26,9 +28,13 @@
use Psalm\Issue\PossiblyNullOperand;
use Psalm\IssueBuffer;
use Psalm\Type;
use Psalm\Type\Atomic;
use Psalm\Type\Atomic\TFalse;
use Psalm\Type\Atomic\TFloat;
use Psalm\Type\Atomic\TInt;
use Psalm\Type\Atomic\TIntRange;
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;
Expand All @@ -40,6 +46,7 @@
use Psalm\Type\Atomic\TNumericString;
use Psalm\Type\Atomic\TString;
use Psalm\Type\Atomic\TTemplateParam;
use Psalm\Type\Atomic\TTrue;
use Psalm\Type\Union;
use UnexpectedValueException;

Expand All @@ -52,7 +59,7 @@
*/
final class ConcatAnalyzer
{
private const MAX_LITERALS = 64;
private const MAX_LITERALS = 500;

public static function analyze(
StatementsAnalyzer $statements_analyzer,
Expand Down Expand Up @@ -156,17 +163,26 @@ public static function analyze(

// If both types are specific literals, combine them into new literals
$literal_concat = false;

if ($left_type->allSpecificLiterals() && $right_type->allSpecificLiterals()) {
if ($left_type->allSpecificLiteralsOrRange() && $right_type->allSpecificLiteralsOrRange()) {
$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) {
if ($combinations > self::MAX_LITERALS) {
$left_type_parts = [];
$right_type_parts = [];
} else {
$left_type_parts = self::intRangeToInts($codebase, $left_type_parts);
$right_type_parts = self::intRangeToInts($codebase, $right_type_parts);
$combinations = count($left_type_parts) * count($right_type_parts);
}

if ($combinations !== 0 && $combinations <= self::MAX_LITERALS ) {
$literal_concat = true;
$result_type_parts = [];

foreach ($left_type->getAtomicTypes() as $left_type_part) {
foreach ($right_type->getAtomicTypes() as $right_type_part) {
foreach ($left_type_parts as $left_type_part) {
foreach ($right_type_parts 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
Expand Down Expand Up @@ -487,4 +503,39 @@ private static function analyzeOperand(
}
}
}

/**
* @param array<TLiteralString|TLiteralInt|TLiteralFloat|TFalse|TTrue|TIntRange> $type_parts
* @return array<TLiteralString|TLiteralInt|TLiteralFloat|TFalse|TTrue>
*/
private static function intRangeToInts(
Codebase $codebase,
array $type_parts
): array {
foreach ($type_parts as $key => $atomic) {
if ($atomic instanceof TIntRange) {
$atomic_from_range = TypeExpander::expandAtomic(
$codebase,
$atomic,
null,
null,
null,
);

if ($atomic_from_range[0] === $atomic || count($atomic_from_range) > self::MAX_LITERALS) {
$type_parts = [];
break;
}

foreach ($atomic_from_range as $atomic_int) {
assert($atomic instanceof TLiteralInt);
$type_parts[] = $atomic_int;
}

unset($type_parts[$key]);
}
}

return $type_parts;
}
}
14 changes: 14 additions & 0 deletions src/Psalm/Internal/Type/TypeExpander.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Psalm\Type\Atomic\TInt;
use Psalm\Type\Atomic\TIntMask;
use Psalm\Type\Atomic\TIntMaskOf;
use Psalm\Type\Atomic\TIntRange;
use Psalm\Type\Atomic\TIterable;
use Psalm\Type\Atomic\TKeyOf;
use Psalm\Type\Atomic\TKeyedArray;
Expand All @@ -45,6 +46,7 @@
use function count;
use function get_class;
use function is_string;
use function range;
use function reset;
use function strtolower;

Expand Down Expand Up @@ -437,6 +439,18 @@ public static function expandAtomic(
return TypeParser::getComputedIntsFromMask($potential_ints);
}

if ($return_type instanceof TIntRange
&& $return_type->min_bound !== null
&& $return_type->max_bound !== null
&& ($return_type->max_bound - $return_type->min_bound) < 500
) {
$literal_ints = [];
foreach (range($return_type->min_bound, $return_type->max_bound) as $literal_int) {
$literal_ints[] = new TLiteralInt($literal_int);
}
return $literal_ints;
}

if ($return_type instanceof TConditional) {
return self::expandConditional(
$codebase,
Expand Down
32 changes: 32 additions & 0 deletions src/Psalm/Type/UnionTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,7 @@ public function allFloatLiterals(): bool
}

/**
* @psalm-suppress PossiblyUnusedMethod
* @psalm-mutation-free
* @psalm-assert-if-true array<
* array-key,
Expand All @@ -1162,6 +1163,37 @@ public function allSpecificLiterals(): bool
return true;
}

/**
* @psalm-mutation-free
* @psalm-assert-if-true array<
* array-key,
* TLiteralString|TLiteralInt|TLiteralFloat|TFalse|TTrue|TIntRange
* > $this->getAtomicTypes()
*/
public function allSpecificLiteralsOrRange(): bool
{
foreach ($this->types as $atomic_key_type) {
if ($atomic_key_type instanceof TIntRange
&& $atomic_key_type->min_bound !== null
&& $atomic_key_type->max_bound !== null
&& ($atomic_key_type->max_bound - $atomic_key_type->min_bound) < 500
) {
continue;
}

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-mutation-free
* @psalm-assert-if-true array<
Expand Down
18 changes: 18 additions & 0 deletions tests/BinaryOperationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,24 @@ function foo($a, $b): void {
'$a' => 'float',
],
],
'concatWithIntsKeepsLiteral' => [
'code' => '<?php
/**
* @var "a"|"b" $a
* @var 0|1|2 $b
*/
$interpolated = $a . $b;',
'assertions' => ['$interpolated===' => "'a0'|'a1'|'a2'|'b0'|'b1'|'b2'"],
],
'concatWithIntRangeKeepsLiteral' => [
'code' => '<?php
/**
* @var "a"|"b" $a
* @var int<0, 2> $b
*/
$interpolated = $a . $b;',
'assertions' => ['$interpolated===' => "'a0'|'a1'|'a2'|'b0'|'b1'|'b2'"],
],
'literalConcatCreatesLiteral' => [
'code' => '<?php
/**
Expand Down
8 changes: 4 additions & 4 deletions tests/IntRangeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,8 @@ function getInt(): int{return 0;}
'$u===' => 'int<-2, 0>',
'$v===' => 'int<2, 0>',
'$w===' => 'mixed',
'$x===' => 'int<0, 2>',
'$y===' => 'int<-2, 0>',
'$x===' => '0',
'$y===' => '0',
'$z===' => 'mixed',
'$aa===' => 'int<-2, 2>',
'$ab===' => 'int<-2, 2>',
Expand Down Expand Up @@ -335,7 +335,7 @@ function getInt(): int{return 0;}
'$h===' => '0|1|float',
'$i===' => 'int',
'$j===' => 'float',
'$k===' => '-1',
'$k===' => '1',
'$l===' => 'float|int',
'$m===' => 'int<1, max>',
'$n===' => 'float',
Expand Down Expand Up @@ -651,7 +651,7 @@ function getInt(): int{ return rand(0, 10); }
}
',
'assertions' => [
'$c===' => 'int<0, 0>|null',
'$c===' => '0|null',
],
],
'minMax' => [
Expand Down

0 comments on commit e70d79b

Please sign in to comment.