Skip to content

Commit

Permalink
Merge pull request #9981 from kkmuffme/fix-replace-functions-return-t…
Browse files Browse the repository at this point in the history 
…ype-provider-less-specific

fix mixed replace return types for arrays
  • Loading branch information
@orklah
orklah committed Jul 2, 2023
2 parents 53ce62b + 0377dd9 commit 8d1876a
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 74 deletions.
5 changes: 5 additions & 0 deletions src/Psalm/Config/Creator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use function implode;
use function is_array;
use function is_dir;
use function is_string;
use function json_decode;
use function ksort;
use function max;
Expand Down Expand Up @@ -250,6 +251,10 @@ private static function getPsr4Or0Paths(string $current_dir, array $composer_jso
}

foreach ($paths as $path) {
if (!is_string($path)) {
continue;
}

if ($path === '') {
$nodes = [...$nodes, ...self::guessPhpFileDirs($current_dir)];

Expand Down
39 changes: 32 additions & 7 deletions src/Psalm/Config/FileFilter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use function in_array;
use function is_dir;
use function is_iterable;
use function is_string;
use function preg_match;
use function preg_replace;
use function preg_split;
Expand Down Expand Up @@ -57,12 +58,12 @@ class FileFilter
protected $fq_classlike_names = [];

/**
* @var array<string>
* @var array<non-empty-string>
*/
protected $fq_classlike_patterns = [];

/**
* @var array<string>
* @var array<non-empty-string>
*/
protected $method_ids = [];

Expand Down Expand Up @@ -314,22 +315,39 @@ public static function loadFromArray(
if (isset($config['referencedMethod']) && is_iterable($config['referencedMethod'])) {
/** @var array $referenced_method */
foreach ($config['referencedMethod'] as $referenced_method) {
$method_id = (string) ($referenced_method['name'] ?? '');

if (!preg_match('/^[^:]+::[^:]+$/', $method_id) && !static::isRegularExpression($method_id)) {
$method_id = $referenced_method['name'] ?? '';
if (!is_string($method_id)
|| (!preg_match('/^[^:]+::[^:]+$/', $method_id) && !static::isRegularExpression($method_id))) {
throw new ConfigException(
'Invalid referencedMethod ' . $method_id,
'Invalid referencedMethod ' . ((string) $method_id),
);
}

if ($method_id === '') {
continue;
}

$filter->method_ids[] = strtolower($method_id);
}
}

if (isset($config['referencedFunction']) && is_iterable($config['referencedFunction'])) {
/** @var array $referenced_function */
foreach ($config['referencedFunction'] as $referenced_function) {
$filter->method_ids[] = strtolower((string) ($referenced_function['name'] ?? ''));
$function_id = $referenced_function['name'] ?? '';
if (!is_string($function_id)
|| (!preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/', $function_id)
&& !static::isRegularExpression($function_id))) {
throw new ConfigException(
'Invalid referencedFunction ' . ((string) $function_id),
);
}

if ($function_id === '') {
continue;
}

$filter->method_ids[] = strtolower($function_id);
}
}

Expand Down Expand Up @@ -441,8 +459,15 @@ public static function loadFromXMLElement(
return self::loadFromArray($config, $base_dir, $inclusive);
}

/**
* @psalm-assert-if-true non-empty-string $string
*/
private static function isRegularExpression(string $string): bool
{
if ($string === '') {
return false;
}

set_error_handler(
static fn(): bool => true,
E_WARNING,
Expand Down
61 changes: 21 additions & 40 deletions src/Psalm/Internal/Provider/ReturnTypeProvider/StrReplaceReturnTypeProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@
use Psalm\Plugin\EventHandler\Event\FunctionReturnTypeProviderEvent;
use Psalm\Plugin\EventHandler\FunctionReturnTypeProviderInterface;
use Psalm\Type;
use Psalm\Type\Atomic\TNull;
use Psalm\Type\Atomic\TString;
use Psalm\Type\Union;

use function call_user_func;
use function count;
use function in_array;

/**
* @internal
Expand All @@ -27,61 +24,45 @@ public static function getFunctionIds(): array
return [
'str_replace',
'str_ireplace',
'substr_replace',
'preg_replace',
'preg_replace_callback',
];
}

public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $event): Union
public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $event): ?Union
{
$statements_source = $event->getStatementsSource();
$call_args = $event->getCallArgs();
$function_id = $event->getFunctionId();
if (!$statements_source instanceof StatementsAnalyzer
|| count($call_args) < 3
) {
return Type::getMixed();
// use the defaults, it will already report an error for the invalid params
return null;
}

if ($subject_type = $statements_source->node_data->getType($call_args[2]->value)) {
if (!$subject_type->hasString() && $subject_type->hasArray()) {
return Type::getArray();
if (!$subject_type->isSingleStringLiteral()) {
return null;
}

$return_type = Type::getString();

if (in_array($function_id, ['str_replace', 'str_ireplace'], true)
&& $subject_type->isSingleStringLiteral()
$first_arg = $statements_source->node_data->getType($call_args[0]->value);
$second_arg = $statements_source->node_data->getType($call_args[1]->value);
if ($first_arg
&& $second_arg && $first_arg->isSingleStringLiteral()
&& $second_arg->isSingleStringLiteral()
) {
$first_arg = $statements_source->node_data->getType($call_args[0]->value);
$second_arg = $statements_source->node_data->getType($call_args[1]->value);
if ($first_arg
&& $second_arg && $first_arg->isSingleStringLiteral()
&& $second_arg->isSingleStringLiteral()
) {
/**
* @var string $replaced_string
*/
$replaced_string = call_user_func(
$function_id,
$first_arg->getSingleStringLiteral()->value,
$second_arg->getSingleStringLiteral()->value,
$subject_type->getSingleStringLiteral()->value,
);
$return_type = Type::getString($replaced_string);
}
} elseif (in_array($function_id, ['preg_replace', 'preg_replace_callback'], true)) {
$codebase = $statements_source->getCodebase();

$return_type = new Union([new TString, new TNull()], [
'ignore_nullable_issues' => $codebase->config->ignore_internal_nullable_issues,
]);
/**
* @var string $replaced_string
*/
$replaced_string = call_user_func(
$function_id,
$first_arg->getSingleStringLiteral()->value,
$second_arg->getSingleStringLiteral()->value,
$subject_type->getSingleStringLiteral()->value,
);
return Type::getString($replaced_string);
}

return $return_type;
}

return Type::getMixed();
return null;
}
}
62 changes: 38 additions & 24 deletions stubs/CoreGenericFunctions.phpstub
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,10 +399,10 @@ function array_fill_keys(array $keys, $value): array
*
* @psalm-template TKey
*
* @param string $pattern
* @param array<TKey,string> $array
* @param non-empty-string $pattern
* @param array<TKey, string> $array
* @param 0|1 $flags 1=PREG_GREP_INVERT
* @return array<TKey,string>
* @return array<TKey, string>
*/
function preg_grep($pattern, array $array, $flags = 0)
{
Expand Down Expand Up @@ -619,7 +619,7 @@ function strtoupper(string $string) : string {}
* @param int|array<int> $offset
* @param null|int|array<int> $length
*
* @return ($string is array<string> ? array<string> : string)
* @return ($string is array ? array<string> : string)
*
* @psalm-flow ($string, $replace) -> return
*/
Expand Down Expand Up @@ -786,6 +786,8 @@ function explode(string $separator, string $string, int $limit = -1) : array {}
/**
* @psalm-pure
*
* @param non-empty-string $pattern
*
* @psalm-flow ($subject) -(array-assignment)-> return
*
* @template TFlags as int-mask<0, 1, 2, 4>
Expand Down Expand Up @@ -998,11 +1000,13 @@ function strlen(string $string) : int {}
/**
* @psalm-pure
*
* @template TKey of array-key
*
* @param string|array<string|int|float> $search
* @param string|array<string|int|float> $replace
* @param string|array<string|int|float> $subject
* @param int $count
* @return ($subject is array ? array<string> : string)
* @param string|array<TKey, string|int|float> $subject
* @param int<0, max> $count
* @return ($subject is array ? array<TKey, string> : string)
*
* @psalm-flow ($replace, $subject) -> return
*/
Expand All @@ -1011,11 +1015,13 @@ function str_replace($search, $replace, $subject, &$count = null) {}
/**
* @psalm-pure
*
* @template TKey of array-key
*
* @param string|array<string|int|float> $search
* @param string|array<string|int|float> $replace
* @param string|array<string|int|float> $subject
* @param int $count
* @return ($subject is array ? array<string> : string)
* @param string|array<TKey, string|int|float> $subject
* @param int<0, max> $count
* @return ($subject is array ? array<TKey, string> : string)
*
* @psalm-flow ($replace, $subject) -> return
*/
Expand Down Expand Up @@ -1169,10 +1175,10 @@ function str_word_count(string $string, int $format = 0, string|null $characters
/**
* @psalm-pure
*
* @param string|string[] $pattern
* @param non-empty-string|non-empty-string[] $pattern
* @param string|array<string|int|float> $replacement
* @param string|array<string|int|float> $subject
* @param int $count
* @param int<0, max> $count
* @return ($subject is array ? array<string> : string|null)
*
* @psalm-flow ($replacement, $subject) -> return
Expand All @@ -1182,22 +1188,30 @@ function preg_filter($pattern, $replacement, $subject, int $limit = -1, &$count
/**
* @psalm-pure
*
* @param string|string[] $pattern
* @template TKey of array-key
*
* @param non-empty-string|non-empty-string[] $pattern
* @param string|array<string|int|float> $replacement
* @param string|array<string|int|float> $subject
* @param int $count
* @return ($subject is array ? array<string>|null : string|null)
* @param string|array<TKey, string|int|float> $subject
* @param int<0, max> $count
* @return ($subject is array ? array<TKey, string>|null : string|null)
*
* @psalm-ignore-nullable-return
*
* @psalm-flow ($replacement, $subject) -> return
*/
function preg_replace($pattern, $replacement, $subject, int $limit = -1, &$count = null) {}

/**
* @param string|string[] $pattern
* @template TKey of array-key
*
* @param non-empty-string|non-empty-string[] $pattern
* @param callable(string[]):string $callback
* @param string|array<string|int|float> $subject
* @param int $count
* @return ($subject is array ? array<string>|null : string|null)
* @param string|array<TKey, string|int|float> $subject
* @param int<0, max> $count
* @return ($subject is array ? array<TKey, string>|null : string|null)
*
* @psalm-ignore-nullable-return
*
* @psalm-taint-specialize
* @psalm-flow ($subject) -> return
Expand All @@ -1208,7 +1222,7 @@ function preg_replace_callback($pattern, $callback, $subject, int $limit = -1, &
* @psalm-pure
* @template TFlags as int
*
* @param string $pattern
* @param non-empty-string $pattern
* @param string $subject
* @param mixed $matches
* @param TFlags $flags
Expand Down Expand Up @@ -1244,7 +1258,7 @@ function preg_match_all($pattern, $subject, &$matches = [], int $flags = 1, int
* @psalm-pure
* @template TFlags as int-mask<0, 256, 512>
*
* @param string $pattern
* @param non-empty-string $pattern
* @param string $subject
* @param mixed $matches
* @param TFlags $flags
Expand Down Expand Up @@ -1371,11 +1385,11 @@ function str_getcsv(string $string, string $separator = ',', string $enclosure =

/**
* @template TKey as array-key
* @template TArray as array<mixed, TKey>
* @template TArray as array<TKey>
*
* @param TArray $array
*
* @return (TArray is non-empty-array ? non-empty-array<TKey, positive-int> : array<TKey, positive-int>)
* @return (TArray is non-empty-array ? non-empty-array<TKey, int<1, max>> : array<TKey, int<1, max>>)
*
* @psalm-pure
*/
Expand Down
15 changes: 13 additions & 2 deletions tests/FunctionCallTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ function foo(array $arr): void {
*/
function route($callback) {
if (!is_callable($callback)) { }
$a = preg_match("", "", $b);
$a = preg_match("//", "", $b);
if ($b[0]) {}
}',
'assertions' => [],
Expand Down Expand Up @@ -501,6 +501,16 @@ function bat(string $s): ?string {
return $s;
}',
],
'pregReplaceArrayValueType' => [
'code' => '<?php
/**
* @param string[] $s
* @return string[]
*/
function foo($s): array {
return preg_replace("/hello/", "", $s);
}',
],
'extractVarCheck' => [
'code' => '<?php
function takesString(string $str): void {}
Expand Down Expand Up @@ -1679,7 +1689,7 @@ function (array $matches) : string {
*/
function(array $ids): array {
return \preg_replace_callback(
"",
"//",
fn (array $matches) => $matches[4],
$ids
);
Expand Down Expand Up @@ -1814,6 +1824,7 @@ function test() : void {
'writeArgsAllowed' => [
'code' => '<?php
/**
* @param non-empty-string $pattern
* @param 0|256|512|768 $flags
* @return false|int
*/
Expand Down

0 comments on commit 8d1876a

Please sign in to comment.