Skip to content

Commit

Permalink
Fix callable/lowercase strings coercion
Browse files Browse the repository at this point in the history
For callable/lowercase strings, neither is a proper subset of another,
but those sets intersect (are coercible to one another).

Fixes vimeo#11075
  • Loading branch information
weirdan committed Sep 7, 2024
1 parent dd24f5b commit 876ad5e
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 10 deletions.
20 changes: 14 additions & 6 deletions src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,21 @@ public static function isContainedBy(
return false;
}

if ($input_type_part instanceof TCallableString
&& (get_class($container_type_part) === TSingleLetter::class
|| get_class($container_type_part) === TNonEmptyString::class
if ($input_type_part instanceof TCallableString) {
if (get_class($container_type_part) === TNonEmptyString::class
|| get_class($container_type_part) === TNonFalsyString::class
|| get_class($container_type_part) === TLowercaseString::class)
) {
return true;
) {
return true;
}

if (get_class($container_type_part) === TLowercaseString::class
|| get_class($container_type_part) === TSingleLetter::class
) {
if ($atomic_comparison_result) {
$atomic_comparison_result->type_coerced = true;
}
return false;
}
}

if (($container_type_part instanceof TLowercaseString
Expand Down
8 changes: 8 additions & 0 deletions tests/FunctionCallTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,14 @@ function foo(string $s) : void {
'assertions' => [],
'ignored_issues' => ['MixedAssignment', 'MixedArgument'],
],
'noRedundantErrorForCallableStrToLower' => [
'code' => <<<'PHP'
<?php
/** @var callable-string */
$function = "strlen";
strtolower($function);
PHP,
],
'objectLikeArrayAssignmentInConditional' => [
'code' => '<?php
$a = [];
Expand Down
42 changes: 38 additions & 4 deletions tests/TypeComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use function array_diff_key;
use function array_keys;
use function array_map;
use Psalm\Internal\Type\Comparator\TypeComparisonResult;

class TypeComparatorTest extends TestCase
{
Expand Down Expand Up @@ -129,6 +130,43 @@ public function testTypeDoesNotAcceptType(string $parent_type_string, string $ch
);
}

/** @dataProvider getCoercibleComparisons */
public function testTypeIsCoercible(string $parent_type_string, string $child_type_string): void
{
$parent_type = Type::parseString($parent_type_string);
$child_type = Type::parseString($child_type_string);

$result = new TypeComparisonResult();

$contained = UnionTypeComparator::isContainedBy(
$this->project_analyzer->getCodebase(),
$child_type,
$parent_type,
false,
false,
$result,
);

$this->assertFalse($contained, 'Type ' . $parent_type_string . ' should not contain ' . $child_type_string);
$this->assertTrue(
$result->type_coerced,
'Type ' . $parent_type_string . ' should be coercible into ' . $child_type_string,
);
}

/** @return iterable<string, list{string, string}> */
public function getCoercibleComparisons(): iterable
{
yield 'callableStringIntoLowercaseString' => [
'lowercase-string',
'callable-string',
];
yield 'lowercaseStringIntoCallableString' => [
'callable-string',
'lowercase-string',
];
}

/**
* @return array<array{string, string}>
*/
Expand All @@ -155,10 +193,6 @@ public function getSuccessfulComparisons(): array
'array{foo?: string}&array<string, mixed>',
'array<never, never>',
],
'Lowercase-stringAndCallable-string' => [
'lowercase-string',
'callable-string',
],
'callableUnionAcceptsCallableUnion' => [
'(callable(int,string[]): void)|(callable(int): void)',
'(callable(int): void)|(callable(int,string[]): void)',
Expand Down

0 comments on commit 876ad5e

Please sign in to comment.