Skip to content

Commit 93a9f4d

Browse files
staabmstaabm
authored
Improve ReplaceSafeFunctions type inference (#70)
1 parent 669fc1c commit 93a9f4d

8 files changed

Lines changed: 225 additions & 45 deletions

‎composer.json‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@
2929
"autoload-dev": {
3030
"psr-4": {
3131
"TheCodingMachine\\Safe\\PHPStan\\": "tests/"
32-
}
32+
},
33+
"classmap": [
34+
"tests/Type"
35+
]
3336
},
3437
"scripts": {
3538
"phpstan": "phpstan analyse -c phpstan.neon --no-progress -vv",

‎src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php‎

Lines changed: 168 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,26 @@
33

44
namespace TheCodingMachine\Safe\PHPStan\Type\Php;
55

6-
use PhpParser\Node\Arg;
76
use PhpParser\Node\Expr\FuncCall;
87
use PHPStan\Analyser\Scope;
98
use PHPStan\Reflection\FunctionReflection;
109
use PHPStan\Reflection\ParametersAcceptorSelector;
10+
use PHPStan\ShouldNotHappenException;
11+
use PHPStan\Type\Accessory\AccessoryLowercaseStringType;
12+
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
13+
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
14+
use PHPStan\Type\Accessory\AccessoryUppercaseStringType;
1115
use PHPStan\Type\ArrayType;
1216
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
17+
use PHPStan\Type\IntersectionType;
1318
use PHPStan\Type\MixedType;
1419
use PHPStan\Type\StringType;
1520
use PHPStan\Type\Type;
1621
use PHPStan\Type\TypeCombinator;
1722
use PHPStan\Type\TypeUtils;
23+
use function array_key_exists;
24+
use function count;
25+
use function in_array;
1826

1927
/**
2028
* This file has been copy-pasted from PHPStan's source code but with isFunctionSupported changed.
@@ -24,33 +32,40 @@
2432
class ReplaceSafeFunctionsDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
2533
{
2634

27-
/** @var array<string, int> The function name associated with the typed argument position used to type the return */
28-
private array $functions = [
35+
private const FUNCTIONS_SUBJECT_POSITION = [
2936
'Safe\preg_replace' => 2,
3037
'Safe\preg_replace_callback' => 2,
3138
'Safe\preg_replace_callback_array' => 1,
39+
/*
40+
'str_replace' => 2,
41+
'str_ireplace' => 2,
42+
'substr_replace' => 0,
43+
'strtr' => 0,
44+
*/
45+
];
46+
47+
private const FUNCTIONS_REPLACE_POSITION = [
48+
'preg_replace' => 1,
49+
'str_replace' => 1,
50+
'str_ireplace' => 1,
51+
'substr_replace' => 1,
52+
'strtr' => 2,
3253
];
3354

3455
public function isFunctionSupported(FunctionReflection $functionReflection): bool
3556
{
36-
return array_key_exists($functionReflection->getName(), $this->functions);
57+
return array_key_exists($functionReflection->getName(), self::FUNCTIONS_SUBJECT_POSITION);
3758
}
3859

3960
public function getTypeFromFunctionCall(
4061
FunctionReflection $functionReflection,
4162
FuncCall $functionCall,
42-
Scope $scope
43-
): Type {
63+
Scope $scope,
64+
): ?Type
65+
{
4466
$type = $this->getPreliminarilyResolvedTypeFromFunctionCall($functionReflection, $functionCall, $scope);
4567

46-
$possibleTypes = ParametersAcceptorSelector::selectFromArgs(
47-
$scope,
48-
$functionCall->getArgs(),
49-
$functionReflection->getVariants()
50-
)
51-
->getReturnType();
52-
53-
if (TypeCombinator::containsNull($possibleTypes)) {
68+
if ($type !== null && $this->canReturnNull($functionReflection, $functionCall, $scope)) {
5469
$type = TypeCombinator::addNull($type);
5570
}
5671

@@ -60,36 +75,155 @@ public function getTypeFromFunctionCall(
6075
private function getPreliminarilyResolvedTypeFromFunctionCall(
6176
FunctionReflection $functionReflection,
6277
FuncCall $functionCall,
63-
Scope $scope
64-
): Type {
65-
$argumentPosition = $this->functions[$functionReflection->getName()];
66-
78+
Scope $scope,
79+
): ?Type
80+
{
81+
$subjectArgumentType = $this->getSubjectType($functionReflection, $functionCall, $scope);
6782
$args = $functionCall->getArgs();
68-
$variants = $functionReflection->getVariants();
69-
$defaultReturnType = ParametersAcceptorSelector::selectFromArgs($scope, $args, $variants)
70-
->getReturnType();
7183

72-
if (count($args) <= $argumentPosition) {
73-
return $defaultReturnType;
84+
if ($subjectArgumentType === null) {
85+
return null;
7486
}
7587

76-
$subjectArgument = $args[$argumentPosition];
77-
$subjectArgumentType = $scope->getType($subjectArgument->value);
78-
$mixedType = new MixedType();
79-
if ($subjectArgumentType->isSuperTypeOf($mixedType)->yes()) {
88+
if ($subjectArgumentType instanceof MixedType) {
89+
$defaultReturnType = ParametersAcceptorSelector::selectFromArgs(
90+
$scope,
91+
$args,
92+
$functionReflection->getVariants(),
93+
)->getReturnType();
94+
8095
return TypeUtils::toBenevolentUnion($defaultReturnType);
8196
}
8297

83-
$stringType = new StringType();
84-
if ($stringType->isSuperTypeOf($subjectArgumentType)->yes()) {
85-
return $stringType;
98+
$replaceArgumentType = null;
99+
if (array_key_exists($functionReflection->getName(), self::FUNCTIONS_REPLACE_POSITION)) {
100+
$replaceArgumentPosition = self::FUNCTIONS_REPLACE_POSITION[$functionReflection->getName()];
101+
102+
if (count($args) > $replaceArgumentPosition) {
103+
$replaceArgumentType = $scope->getType($args[$replaceArgumentPosition]->value);
104+
if ($replaceArgumentType->isArray()->yes()) {
105+
$replaceArgumentType = $replaceArgumentType->getIterableValueType();
106+
}
107+
} elseif ($functionReflection->getName() === 'strtr' && isset($functionCall->getArgs()[1])) {
108+
// `strtr` has two signatures: `strtr($string1, $string2, $string3)` and `strtr($string1, $array)`
109+
$secondArgumentType = TypeCombinator::intersect(
110+
new ArrayType(new MixedType(), new MixedType()),
111+
$scope->getType($functionCall->getArgs()[1]->value),
112+
);
113+
$replaceArgumentType = $secondArgumentType->getIterableValueType();
114+
}
86115
}
87116

88-
$arrayType = new ArrayType($mixedType, $mixedType);
89-
if ($arrayType->isSuperTypeOf($subjectArgumentType)->yes()) {
90-
return $arrayType;
117+
$result = [];
118+
119+
if ($subjectArgumentType->isString()->yes()) {
120+
$stringArgumentType = $subjectArgumentType;
121+
} else {
122+
$stringArgumentType = TypeCombinator::intersect(new StringType(), $subjectArgumentType);
123+
}
124+
if ($stringArgumentType->isString()->yes()) {
125+
$result[] = $this->getReplaceType($stringArgumentType, $replaceArgumentType);
91126
}
92127

93-
return $defaultReturnType;
128+
if ($subjectArgumentType->isArray()->yes()) {
129+
$arrayArgumentType = $subjectArgumentType;
130+
} else {
131+
$arrayArgumentType = TypeCombinator::intersect(new ArrayType(new MixedType(), new MixedType()), $subjectArgumentType);
132+
}
133+
if ($arrayArgumentType->isArray()->yes()) {
134+
$keyShouldBeOptional = in_array(
135+
$functionReflection->getName(),
136+
['preg_replace', 'preg_replace_callback', 'preg_replace_callback_array'],
137+
true,
138+
);
139+
140+
$mapped = $arrayArgumentType->mapValueType(
141+
fn (Type $value): Type => $this->getReplaceType($value, $replaceArgumentType),
142+
);
143+
if ($keyShouldBeOptional) {
144+
$mapped = $mapped->makeAllArrayKeysOptional();
145+
}
146+
147+
$result[] = $mapped;
148+
}
149+
150+
return TypeCombinator::union(...$result);
94151
}
152+
153+
private function getReplaceType(
154+
Type $subjectArgumentType,
155+
?Type $replaceArgumentType,
156+
): Type
157+
{
158+
if ($replaceArgumentType === null) {
159+
return new StringType();
160+
}
161+
162+
$accessories = [];
163+
if ($subjectArgumentType->isNonFalsyString()->yes() && $replaceArgumentType->isNonFalsyString()->yes()) {
164+
$accessories[] = new AccessoryNonFalsyStringType();
165+
} elseif ($subjectArgumentType->isNonEmptyString()->yes() && $replaceArgumentType->isNonEmptyString()->yes()) {
166+
$accessories[] = new AccessoryNonEmptyStringType();
167+
}
168+
169+
if ($subjectArgumentType->isLowercaseString()->yes() && $replaceArgumentType->isLowercaseString()->yes()) {
170+
$accessories[] = new AccessoryLowercaseStringType();
171+
}
172+
173+
if ($subjectArgumentType->isUppercaseString()->yes() && $replaceArgumentType->isUppercaseString()->yes()) {
174+
$accessories[] = new AccessoryUppercaseStringType();
175+
}
176+
177+
if (count($accessories) > 0) {
178+
$accessories[] = new StringType();
179+
return new IntersectionType($accessories);
180+
}
181+
182+
return new StringType();
183+
}
184+
185+
private function getSubjectType(
186+
FunctionReflection $functionReflection,
187+
FuncCall $functionCall,
188+
Scope $scope,
189+
): ?Type
190+
{
191+
if (!array_key_exists($functionReflection->getName(), self::FUNCTIONS_SUBJECT_POSITION)) {
192+
throw new ShouldNotHappenException();
193+
}
194+
195+
$argumentPosition = self::FUNCTIONS_SUBJECT_POSITION[$functionReflection->getName()];
196+
$args = $functionCall->getArgs();
197+
if (count($args) <= $argumentPosition) {
198+
return null;
199+
}
200+
return $scope->getType($args[$argumentPosition]->value);
201+
}
202+
203+
private function canReturnNull(
204+
FunctionReflection $functionReflection,
205+
FuncCall $functionCall,
206+
Scope $scope,
207+
): bool
208+
{
209+
$args = $functionCall->getArgs();
210+
if (
211+
in_array($functionReflection->getName(), ['preg_replace', 'preg_replace_callback', 'preg_replace_callback_array'], true)
212+
&& count($args) > 0
213+
) {
214+
$subjectArgumentType = $this->getSubjectType($functionReflection, $functionCall, $scope);
215+
216+
if (
217+
$subjectArgumentType !== null
218+
&& $subjectArgumentType->isArray()->yes()
219+
) {
220+
return false;
221+
}
222+
223+
return true;
224+
}
225+
226+
return false;
227+
}
228+
95229
}

‎tests/Type/Php/TypeAssertionsTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public static function dataFileAsserts(): iterable
1313
{
1414
yield from self::gatherAssertTypes(__DIR__ . '/data/preg_match_unchecked.php');
1515
yield from self::gatherAssertTypes(__DIR__ . '/data/preg_match_checked.php');
16+
yield from self::gatherAssertTypes(__DIR__ . '/data/preg_replace.php');
1617
yield from self::gatherAssertTypes(__DIR__ . '/data/preg_replace_return.php');
1718
yield from self::gatherAssertTypes(__DIR__ . '/data/json_decode_return.php');
1819
}

‎tests/Type/Php/data/preg_match_checked.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212

1313
// @phpstan-ignore-next-line - use of unsafe is intentional
1414
if(\preg_match($pattern, $string, $matches)) {
15-
\PHPStan\Testing\assertSuperType($type, $matches);
15+
\PHPStan\Testing\assertType($type, $matches);
1616
}
1717

1818
if(\Safe\preg_match($pattern, $string, $matches)) {
19-
\PHPStan\Testing\assertSuperType($type, $matches);
19+
\PHPStan\Testing\assertType($type, $matches);
2020
}

‎tests/Type/Php/data/preg_match_identity_check.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222

2323
// BUG: $matches is NOT narrowed — actual type is array{}|array{...} instead of array{...}
2424
if (1 === preg_match($pattern, $string, $matches)) {
25-
\PHPStan\Testing\assertSuperType($expectedType, $matches);
25+
\PHPStan\Testing\assertType($expectedType, $matches);
2626
}
2727

2828
// BUG: same issue via FQCN
2929
if (1 === \Safe\preg_match($pattern, $string, $matches2)) {
30-
\PHPStan\Testing\assertSuperType($expectedType, $matches2);
30+
\PHPStan\Testing\assertType($expectedType, $matches2);
3131
}

‎tests/Type/Php/data/preg_match_unchecked.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
$string = 'Hello World';
88

99
// when return value isn't checked, we may-or-may-not have matches
10-
$type = "list{0?: string, 1?: non-empty-string, 2?: 'o', 3?: 'World'}";
10+
$type = "array{}|array{0: non-falsy-string, 1: non-empty-string, 2: 'o', 3?: 'World'}";
1111

1212
// @phpstan-ignore-next-line - use of unsafe is intentional
1313
\preg_match($pattern, $string, $matches);
14-
\PHPStan\Testing\assertSuperType($type, $matches);
14+
\PHPStan\Testing\assertType($type, $matches);
1515

1616
\Safe\preg_match($pattern, $string, $matches);
17-
\PHPStan\Testing\assertSuperType($type, $matches);
17+
\PHPStan\Testing\assertType($type, $matches);

‎tests/Type/Php/data/preg_replace.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
namespace PregReplace;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class HelloWorld
8+
{
9+
// `|` must be escaped FIRST to avoid double-escaping.
10+
private const array CHARACTERS_TO_ESCAPE = ['|', "'", "\n", "\r", '[', ']'];
11+
12+
private const array ESCAPED_CHARACTERS = ['||', "|'", '|n', '|r', '|[', '|]'];
13+
14+
private const string UNICODE_CHARACTER_REGEX = '/\\\\u(?<hexadecimalDigits>[0-9A-Fa-f]{4})/';
15+
16+
/**
17+
* @return non-empty-string
18+
*/
19+
private static function escapeValue(string|int|float $value): string
20+
{
21+
$escapedValue = sprintf(
22+
'\'%s\'',
23+
str_replace(
24+
self::CHARACTERS_TO_ESCAPE,
25+
self::ESCAPED_CHARACTERS,
26+
(string) $value,
27+
),
28+
);
29+
assertType('non-falsy-string', $escapedValue);
30+
31+
if (is_string($value) && str_contains($value, '\u')) {
32+
$escapedValue = preg_replace(
33+
self::UNICODE_CHARACTER_REGEX,
34+
'|0x$1',
35+
$escapedValue,
36+
);
37+
}
38+
assertType('non-falsy-string|null', $escapedValue);
39+
40+
return $escapedValue;
41+
}
42+
}

‎tests/Type/Php/data/preg_replace_return.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
// preg_replace with an array pattern should return an array
1212
$x = \Safe\preg_replace(['/foo/'], ['bar'], ['baz']);
13-
\PHPStan\Testing\assertType("array", $x);
13+
\PHPStan\Testing\assertType("array{string}", $x);
1414

1515
// preg_replace_callback
1616

@@ -20,7 +20,7 @@
2020

2121
// preg_replace_callback with an array pattern should return an array
2222
$x = \Safe\preg_replace_callback(['/foo/'], fn (array $matches) => 'bar', ['baz']);
23-
\PHPStan\Testing\assertType("array", $x);
23+
\PHPStan\Testing\assertType("array{string}", $x);
2424

2525

2626
// preg_replace_callback_array
@@ -31,4 +31,4 @@
3131

3232
// preg_replace_callback with an array pattern should return an array
3333
$x = \Safe\preg_replace_callback_array(['/foo/' => fn (array $matches) => 'bar'], ['baz']);
34-
\PHPStan\Testing\assertType("array", $x);
34+
\PHPStan\Testing\assertType("array{string}", $x);

0 commit comments

Comments
 (0)