Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix mixed replace return types for arrays #9981

Merged
merged 6 commits into from
Jul 2, 2023
Merged

fix mixed replace return types for arrays #9981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
26eb870
fix mixed replace return types for arrays
kkmuffme Jun 30, 2023
09a3b4b
code style
kkmuffme Jun 30, 2023
c2388b4
fix failing tests and shepherd
kkmuffme Jul 2, 2023
7593b5b
more code style and shepherd
kkmuffme Jul 2, 2023
d24df1a
improve config validation and fix wrong condition
kkmuffme Jul 2, 2023
0377dd9
code style
kkmuffme Jul 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Psalm/Config/Creator.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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