Skip to content

Commit 7896011

Browse files
schlndhschlndh
authored
Check array functions which require stringish values
1 parent 27cbdd4 commit 7896011

16 files changed

+803
-2
lines changed

‎conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,6 @@ parameters:
5353
magicConstantOutOfContext: true
5454
paramOutType: true
5555
pure: true
56+
checkParameterCastableToStringFunctions: true
5657
stubFiles:
5758
- ../stubs/bleedingEdge/Rule.stub

‎conf/config.level5.neon

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@ conditionalTags:
1212
phpstan.rules.rule: %featureToggles.arrayValues%
1313
PHPStan\Rules\Functions\CallUserFuncRule:
1414
phpstan.rules.rule: %featureToggles.callUserFunc%
15+
PHPStan\Rules\Functions\ParameterCastableToStringFunctionRule:
16+
phpstan.rules.rule: %featureToggles.checkParameterCastableToStringFunctions%
1517

1618
rules:
1719
- PHPStan\Rules\DateTimeInstantiationRule
18-
- PHPStan\Rules\Functions\ImplodeFunctionRule
1920

2021
services:
2122
-
@@ -37,3 +38,11 @@ services:
3738

3839
-
3940
class: PHPStan\Rules\Functions\CallUserFuncRule
41+
-
42+
class: PHPStan\Rules\Functions\ImplodeFunctionRule
43+
arguments:
44+
disabled: %featureToggles.checkParameterCastableToStringFunctions%
45+
tags:
46+
- phpstan.rules.rule
47+
-
48+
class: PHPStan\Rules\Functions\ParameterCastableToStringFunctionRule

‎conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ parameters:
8888
magicConstantOutOfContext: false
8989
paramOutType: false
9090
pure: false
91+
checkParameterCastableToStringFunctions: false
9192
fileExtensions:
9293
- php
9394
checkAdvancedIsset: false

‎conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ parametersSchema:
8383
magicConstantOutOfContext: bool()
8484
paramOutType: bool()
8585
pure: bool()
86+
checkParameterCastableToStringFunctions: bool()
8687
])
8788
fileExtensions: listOf(string())
8889
checkAdvancedIsset: bool()

‎phpstan-baseline.neon

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1825,6 +1825,22 @@ parameters:
18251825
count: 1
18261826
path: tests/PHPStan/Rules/DeadCode/NoopRuleTest.php
18271827

1828+
-
1829+
message: """
1830+
#^Instantiation of deprecated class PHPStan\\\\Rules\\\\Functions\\\\ImplodeFunctionRule\\:
1831+
Replaced by PHPStan\\\\Rules\\\\Functions\\\\ParameterCastableToStringFunctionRule$#
1832+
"""
1833+
count: 1
1834+
path: tests/PHPStan/Rules/Functions/ImplodeFunctionRuleTest.php
1835+
1836+
-
1837+
message: """
1838+
#^Return type of method PHPStan\\\\Rules\\\\Functions\\\\ImplodeFunctionRuleTest\\:\\:getRule\\(\\) has typehint with deprecated class PHPStan\\\\Rules\\\\Functions\\\\ImplodeFunctionRule\\:
1839+
Replaced by PHPStan\\\\Rules\\\\Functions\\\\ParameterCastableToStringFunctionRule$#
1840+
"""
1841+
count: 1
1842+
path: tests/PHPStan/Rules/Functions/ImplodeFunctionRuleTest.php
1843+
18281844
-
18291845
message: "#^PHPDoc tag @var assumes the expression with type PHPStan\\\\Type\\\\Generic\\\\TemplateType is always PHPStan\\\\Type\\\\Generic\\\\TemplateMixedType but it's error\\-prone and dangerous\\.$#"
18301846
count: 1

‎src/Rules/Functions/ImplodeFunctionRule.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use function sprintf;
1818

1919
/**
20+
* @deprecated Replaced by PHPStan\Rules\Functions\ParameterCastableToStringFunctionRule
2021
* @implements Rule<Node\Expr\FuncCall>
2122
*/
2223
class ImplodeFunctionRule implements Rule
@@ -25,6 +26,7 @@ class ImplodeFunctionRule implements Rule
2526
public function __construct(
2627
private ReflectionProvider $reflectionProvider,
2728
private RuleLevelHelper $ruleLevelHelper,
29+
private bool $disabled,
2830
)
2931
{
3032
}
@@ -36,6 +38,10 @@ public function getNodeType(): string
3638

3739
public function processNode(Node $node, Scope $scope): array
3840
{
41+
if ($this->disabled) {
42+
return [];
43+
}
44+
3945
if (!($node->name instanceof Node\Name)) {
4046
return [];
4147
}

‎src/Rules/Functions/ParameterCastableToStringFunctionRule.php

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Functions;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\FuncCall;
7+
use PHPStan\Analyser\ArgumentsNormalizer;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Reflection\ParametersAcceptorSelector;
10+
use PHPStan\Reflection\ReflectionProvider;
11+
use PHPStan\Rules\Rule;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
use PHPStan\Rules\RuleLevelHelper;
14+
use PHPStan\Type\ErrorType;
15+
use PHPStan\Type\Type;
16+
use PHPStan\Type\VerbosityLevel;
17+
use function array_key_exists;
18+
use function count;
19+
use function in_array;
20+
use function sprintf;
21+
22+
/**
23+
* @implements Rule<Node\Expr\FuncCall>
24+
*/
25+
class ParameterCastableToStringFunctionRule implements Rule
26+
{
27+
28+
public function __construct(
29+
private ReflectionProvider $reflectionProvider,
30+
private RuleLevelHelper $ruleLevelHelper,
31+
)
32+
{
33+
}
34+
35+
public function getNodeType(): string
36+
{
37+
return FuncCall::class;
38+
}
39+
40+
public function processNode(Node $node, Scope $scope): array
41+
{
42+
if (!($node->name instanceof Node\Name)) {
43+
return [];
44+
}
45+
46+
if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
47+
return [];
48+
}
49+
50+
$functionReflection = $this->reflectionProvider->getFunction($node->name, $scope);
51+
$functionName = $functionReflection->getName();
52+
$implodeFunctions = ['implode', 'join'];
53+
$checkAllArgsFunctions = ['array_intersect', 'array_intersect_assoc', 'array_diff', 'array_diff_assoc'];
54+
$checkFirstArgFunctions = [
55+
'array_unique',
56+
'array_combine',
57+
'sort',
58+
'rsort',
59+
'asort',
60+
'arsort',
61+
'natcasesort',
62+
'natsort',
63+
'array_count_values',
64+
'array_fill_keys',
65+
];
66+
67+
if (
68+
!in_array($functionName, $checkAllArgsFunctions, true)
69+
&& !in_array($functionName, $checkFirstArgFunctions, true)
70+
&& !in_array($functionName, $implodeFunctions, true)
71+
) {
72+
return [];
73+
}
74+
75+
$origArgs = $node->getArgs();
76+
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs(
77+
$scope,
78+
$origArgs,
79+
$functionReflection->getVariants(),
80+
$functionReflection->getNamedArgumentsVariants(),
81+
);
82+
83+
$errorMessage = 'Parameter %s of function %s expects an array of values castable to string, %s given.';
84+
$getNormalizedArgs = static function () use ($parametersAcceptor, $node): array {
85+
$normalizedFuncCall = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $node);
86+
87+
if ($normalizedFuncCall === null) {
88+
return [];
89+
}
90+
91+
return $normalizedFuncCall->getArgs();
92+
};
93+
if (in_array($functionName, $implodeFunctions, true)) {
94+
$normalizedArgs = $getNormalizedArgs();
95+
$errorMessage = 'Parameter %s of function %s expects array<string>, %s given.';
96+
if (count($normalizedArgs) === 1) {
97+
$argsToCheck = [0 => $normalizedArgs[0]];
98+
} elseif (count($normalizedArgs) === 2) {
99+
$argsToCheck = [1 => $normalizedArgs[1]];
100+
} else {
101+
return [];
102+
}
103+
} elseif (in_array($functionName, $checkAllArgsFunctions, true)) {
104+
$argsToCheck = $origArgs;
105+
} elseif (in_array($functionName, $checkFirstArgFunctions, true)) {
106+
$normalizedArgs = $getNormalizedArgs();
107+
if ($normalizedArgs === []) {
108+
return [];
109+
}
110+
$argsToCheck = [0 => $normalizedArgs[0]];
111+
} else {
112+
return [];
113+
}
114+
115+
$origNamedArgs = [];
116+
foreach ($origArgs as $arg) {
117+
if ($arg->unpack || $arg->name === null) {
118+
continue;
119+
}
120+
121+
$origNamedArgs[$arg->name->toString()] = $arg;
122+
}
123+
124+
$errors = [];
125+
$functionParameters = $parametersAcceptor->getParameters();
126+
127+
foreach ($argsToCheck as $argIdx => $arg) {
128+
if ($arg->unpack) {
129+
continue;
130+
}
131+
132+
$typeResult = $this->ruleLevelHelper->findTypeToCheck(
133+
$scope,
134+
$arg->value,
135+
'',
136+
static fn (Type $type): bool => !$type->getIterableValueType()->toString() instanceof ErrorType,
137+
);
138+
139+
if ($typeResult->getType() instanceof ErrorType
140+
|| !$typeResult->getType()->getIterableValueType()->toString() instanceof ErrorType) {
141+
continue;
142+
}
143+
144+
if (in_array($functionName, $implodeFunctions, true)) {
145+
// implode has weird variants, so $array has to be fixed. It's especially weird with named arguments.
146+
if (array_key_exists('array', $origNamedArgs)) {
147+
$argName = '$array';
148+
} elseif (array_key_exists('separator', $origNamedArgs) && count($origArgs) === 1) {
149+
$argName = '$separator';
150+
} else {
151+
$argName = sprintf('#%d $array', $argIdx + 1);
152+
}
153+
} elseif (array_key_exists($argIdx, $functionParameters)) {
154+
$paramName = $functionParameters[$argIdx]->getName();
155+
$argName = array_key_exists($paramName, $origNamedArgs)
156+
? sprintf('$%s', $paramName)
157+
: sprintf('#%d $%s', $argIdx + 1, $paramName);
158+
} else {
159+
$argName = sprintf('#%d', $argIdx + 1);
160+
}
161+
162+
$errors[] = RuleErrorBuilder::message(
163+
sprintf($errorMessage, $argName, $functionName, $typeResult->getType()->describe(VerbosityLevel::typeOnly())),
164+
)->identifier('argument.type')->build();
165+
}
166+
167+
return $errors;
168+
}
169+
170+
}

‎tests/PHPStan/Rules/Functions/ImplodeFunctionRuleTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class ImplodeFunctionRuleTest extends RuleTestCase
1515
protected function getRule(): Rule
1616
{
1717
$broker = $this->createReflectionProvider();
18-
return new ImplodeFunctionRule($broker, new RuleLevelHelper($broker, true, false, true, false, false, true, false));
18+
return new ImplodeFunctionRule($broker, new RuleLevelHelper($broker, true, false, true, false, false, true, false), false);
1919
}
2020

2121
public function testFile(): void

0 commit comments

Comments
 (0)