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 #10947
  • Loading branch information
kkmuffme committed May 7, 2024
1 parent 10956df commit 0376aeb
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 6 deletions.
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 ($left_type_parts !== [] && $right_type_parts !== [] && $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,38 @@ private static function analyzeOperand(
}
}
}

/**
* @param array<TLiteralString|TLiteralInt|TLiteralFloat|TFalse|TTrue|TIntRange> $type_parts
* @return array<TLiteralString|TLiteralInt|TLiteralFloat|TFalse|TTrue>

Check failure on line 509 in src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php

View workflow job for this annotation

GitHub Actions / build

MoreSpecificReturnType

src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php:509:16: MoreSpecificReturnType: The declared return type 'array<array-key, Psalm\Type\Atomic\TFalse|Psalm\Type\Atomic\TLiteralFloat|Psalm\Type\Atomic\TLiteralInt|Psalm\Type\Atomic\TLiteralString|Psalm\Type\Atomic\TTrue>' for Psalm\Internal\Analyzer\Statements\Expression\BinaryOp\ConcatAnalyzer::intRangeToInts is more specific than the inferred return type 'array<array-key, Psalm\Type\Atomic>' (see https://psalm.dev/070)
*/
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) {
$type_parts[] = $atomic_int;
}

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

return $type_parts;

Check failure on line 538 in src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php

View workflow job for this annotation

GitHub Actions / build

LessSpecificReturnStatement

src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php:538:16: LessSpecificReturnStatement: The type 'array<array-key, Psalm\Type\Atomic|Psalm\Type\Atomic\TFalse|Psalm\Type\Atomic\TIntRange|Psalm\Type\Atomic\TLiteralFloat|Psalm\Type\Atomic\TLiteralInt|Psalm\Type\Atomic\TLiteralString|Psalm\Type\Atomic\TTrue>' is more general than the declared return type 'array<array-key, Psalm\Type\Atomic\TFalse|Psalm\Type\Atomic\TLiteralFloat|Psalm\Type\Atomic\TLiteralInt|Psalm\Type\Atomic\TLiteralString|Psalm\Type\Atomic\TTrue>' for Psalm\Internal\Analyzer\Statements\Expression\BinaryOp\ConcatAnalyzer::intRangeToInts (see https://psalm.dev/129)
}
}
14 changes: 14 additions & 0 deletions src/Psalm/Internal/Type/TypeExpander.php
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
33 changes: 33 additions & 0 deletions src/Psalm/Type/UnionTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,8 @@ public function allFloatLiterals(): bool
* array-key,
* TLiteralString|TLiteralInt|TLiteralFloat|TFalse|TTrue
* > $this->getAtomicTypes()
*
* @psalm-suppress PossiblyUnusedMethod
*/
public function allSpecificLiterals(): bool
{
Expand All @@ -1162,6 +1164,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
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

0 comments on commit 0376aeb

Please sign in to comment.