Skip to content

Commit da2e9f7

Browse files
committed
Bleeding edge - check printf parameter types
1 parent a758cf3 commit da2e9f7

File tree

8 files changed

+447
-2
lines changed

8 files changed

+447
-2
lines changed

conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@ parameters:
66
skipCheckGenericClasses!: []
77
stricterFunctionMap: true
88
reportPreciseLineForUnusedFunctionParameter: true
9+
checkPrintfParameterTypes: true
910
internalTag: true
1011
newStaticInAbstractClassStaticMethod: true

conf/config.level5.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ parameters:
88
conditionalTags:
99
PHPStan\Rules\Functions\ParameterCastableToNumberRule:
1010
phpstan.rules.rule: %featureToggles.checkParameterCastableToNumberFunctions%
11+
PHPStan\Rules\Functions\PrintfParameterTypeRule:
12+
phpstan.rules.rule: %featureToggles.checkPrintfParameterTypes%
1113

1214
autowiredAttributeServices:
1315
# registers rules with #[RegisteredRule] attribute
@@ -16,3 +18,5 @@ autowiredAttributeServices:
1618
services:
1719
-
1820
class: PHPStan\Rules\Functions\ParameterCastableToNumberRule
21+
-
22+
class: PHPStan\Rules\Functions\PrintfParameterTypeRule

conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ parameters:
2929
skipCheckGenericClasses: []
3030
stricterFunctionMap: false
3131
reportPreciseLineForUnusedFunctionParameter: false
32+
checkPrintfParameterTypes: false
3233
internalTag: false
3334
newStaticInAbstractClassStaticMethod: false
3435
fileExtensions:

conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ parametersSchema:
3333
skipCheckGenericClasses: listOf(string()),
3434
stricterFunctionMap: bool()
3535
reportPreciseLineForUnusedFunctionParameter: bool()
36+
checkPrintfParameterTypes: bool()
3637
internalTag: bool()
3738
newStaticInAbstractClassStaticMethod: bool()
3839
])

src/Rules/Functions/PrintfHelper.php

Lines changed: 143 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,29 +5,170 @@
55
use Nette\Utils\Strings;
66
use PHPStan\DependencyInjection\AutowiredService;
77
use PHPStan\Php\PhpVersion;
8+
use PHPStan\Type\ErrorType;
9+
use PHPStan\Type\IntegerType;
10+
use PHPStan\Type\Type;
811
use function array_filter;
12+
use function array_flip;
13+
use function array_keys;
14+
use function array_map;
15+
use function array_reduce;
916
use function count;
1017
use function max;
18+
use function sort;
1119
use function sprintf;
1220
use function strlen;
21+
use function usort;
1322
use const PREG_SET_ORDER;
1423

24+
/** @phpstan-type AcceptingTypeString 'strict-int'|'int'|'float'|'string'|'mixed' */
1525
#[AutowiredService]
1626
final class PrintfHelper
1727
{
1828

29+
private const PRINTF_SPECIFIER_PATTERN = '(?<specifier>[bs%s]|l?[cdeEgfFGouxX])';
30+
1931
public function __construct(private PhpVersion $phpVersion)
2032
{
2133
}
2234

2335
public function getPrintfPlaceholdersCount(string $format): int
2436
{
25-
return $this->getPlaceholdersCount('(?:[bs%s]|l?[cdeEgfFGouxX])', $format);
37+
return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format);
38+
}
39+
40+
/** @return array<int, array{string, callable(Type): bool}> position => [type name, matches callback] */
41+
public function getPrintfPlaceholderAcceptingTypes(string $format): array
42+
{
43+
$placeholders = $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format);
44+
$result = [];
45+
// int can go into float, string and mixed as well.
46+
// float can't go into int, but it can go to string/mixed.
47+
// string can go into mixed, but not into int/float.
48+
// mixed can only go into mixed.
49+
$typeSequenceMap = array_flip(['int', 'float', 'string', 'mixed']);
50+
51+
foreach ($placeholders as $position => $types) {
52+
sort($types);
53+
$typeNames = array_map(
54+
static fn (string $t) => $t === 'strict-int'
55+
? 'int'
56+
: $t,
57+
$types,
58+
);
59+
$typeName = array_reduce(
60+
$typeNames,
61+
static fn (string $carry, string $type) => $typeSequenceMap[$carry] < $typeSequenceMap[$type]
62+
? $carry
63+
: $type,
64+
'mixed',
65+
);
66+
$result[$position] = [
67+
$typeName,
68+
static function (Type $t) use ($types): bool {
69+
foreach ($types as $acceptingType) {
70+
$subresult = match ($acceptingType) {
71+
'strict-int' => (new IntegerType())->accepts($t, true)->yes(),
72+
// This allows float, constant non-numeric string, ...
73+
'int' => ! $t->toInteger() instanceof ErrorType,
74+
'float' => ! $t->toFloat() instanceof ErrorType,
75+
// The function signature already limits the parameters to stringable types, so there's
76+
// no point in checking it again here.
77+
'string', 'mixed' => true,
78+
};
79+
80+
if (!$subresult) {
81+
return false;
82+
}
83+
}
84+
85+
return true;
86+
},
87+
];
88+
}
89+
90+
return $result;
2691
}
2792

2893
public function getScanfPlaceholdersCount(string $format): int
2994
{
30-
return $this->getPlaceholdersCount('(?:[cdDeEfinosuxX%s]|\[[^\]]+\])', $format);
95+
return $this->getPlaceholdersCount('(?<specifier>[cdDeEfinosuxX%s]|\[[^\]]+\])', $format);
96+
}
97+
98+
/** @phpstan-return array<int, non-empty-list<AcceptingTypeString>> position => type */
99+
private function parsePlaceholders(string $specifiersPattern, string $format): array
100+
{
101+
$addSpecifier = '';
102+
if ($this->phpVersion->supportsHhPrintfSpecifier()) {
103+
$addSpecifier .= 'hH';
104+
}
105+
106+
$specifiers = sprintf($specifiersPattern, $addSpecifier);
107+
108+
$pattern = '~(?<before>%*)%(?:(?<position>\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?<width>\*)?-?\d*(?:\.(?:\d+|(?<precision>\*))?)?' . $specifiers . '~';
109+
110+
$matches = Strings::matchAll($format, $pattern, PREG_SET_ORDER);
111+
112+
if (count($matches) === 0) {
113+
return [];
114+
}
115+
116+
$placeholders = array_filter($matches, static fn (array $match): bool => strlen($match['before']) % 2 === 0);
117+
118+
$result = [];
119+
$positionToIdxMap = [];
120+
$positionalPlaceholders = [];
121+
$idx = $position = 0;
122+
123+
foreach ($placeholders as $placeholder) {
124+
if (isset($placeholder['width']) && $placeholder['width'] !== '') {
125+
$result[$idx] = ['strict-int' => 1];
126+
$positionToIdxMap[$position++] = $idx++;
127+
}
128+
129+
if (isset($placeholder['precision']) && $placeholder['precision'] !== '') {
130+
$result[$idx] = ['strict-int' => 1];
131+
$positionToIdxMap[$position++] = $idx++;
132+
}
133+
134+
if (isset($placeholder['position']) && $placeholder['position'] !== '') {
135+
// It may reference future position, so we have to process them later.
136+
$positionalPlaceholders[] = $placeholder;
137+
continue;
138+
}
139+
140+
$position++;
141+
$positionToIdxMap[$position] = $idx;
142+
$result[$idx++][$this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? '')] = 1;
143+
}
144+
145+
usort(
146+
$positionalPlaceholders,
147+
static fn (array $a, array $b) => (int) $a['position'] <=> (int) $b['position'],
148+
);
149+
150+
foreach ($positionalPlaceholders as $placeholder) {
151+
$idx = $positionToIdxMap[$placeholder['position']] ?? null;
152+
153+
if ($idx === null) {
154+
continue;
155+
}
156+
157+
$result[$idx][$this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? '')] = 1;
158+
}
159+
160+
return array_map(static fn (array $a) => array_keys($a), $result);
161+
}
162+
163+
/** @phpstan-return 'string'|'int'|'float'|'mixed' */
164+
private function getAcceptingTypeBySpecifier(string $specifier): string
165+
{
166+
return match ($specifier) {
167+
's' => 'string',
168+
'd', 'u', 'c', 'o', 'x', 'X', 'b' => 'int',
169+
'e', 'E', 'f', 'F', 'g', 'G', 'h', 'H' => 'float',
170+
default => 'mixed',
171+
};
31172
}
32173

33174
private function getPlaceholdersCount(string $specifiersPattern, string $format): int
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Functions;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Reflection\ReflectionProvider;
8+
use PHPStan\Rules\Rule;
9+
use PHPStan\Rules\RuleErrorBuilder;
10+
use PHPStan\Rules\RuleLevelHelper;
11+
use PHPStan\Type\BooleanType;
12+
use PHPStan\Type\ErrorType;
13+
use PHPStan\Type\FloatType;
14+
use PHPStan\Type\IntegerType;
15+
use PHPStan\Type\NullType;
16+
use PHPStan\Type\StringAlwaysAcceptingObjectWithToStringType;
17+
use PHPStan\Type\TypeCombinator;
18+
use PHPStan\Type\VerbosityLevel;
19+
use function array_key_exists;
20+
use function count;
21+
use function sprintf;
22+
23+
/**
24+
* @implements Rule<Node\Expr\FuncCall>
25+
*/
26+
final class PrintfParameterTypeRule implements Rule
27+
{
28+
29+
private const FORMAT_ARGUMENT_POSITIONS = [
30+
'printf' => 0,
31+
'sprintf' => 0,
32+
'fprintf' => 1,
33+
];
34+
private const MINIMUM_NUMBER_OF_ARGUMENTS = [
35+
'printf' => 1,
36+
'sprintf' => 1,
37+
'fprintf' => 2,
38+
];
39+
40+
public function __construct(
41+
private PrintfHelper $printfHelper,
42+
private ReflectionProvider $reflectionProvider,
43+
private RuleLevelHelper $ruleLevelHelper,
44+
)
45+
{
46+
}
47+
48+
public function getNodeType(): string
49+
{
50+
return Node\Expr\FuncCall::class;
51+
}
52+
53+
public function processNode(Node $node, Scope $scope): array
54+
{
55+
if (!($node->name instanceof Node\Name)) {
56+
return [];
57+
}
58+
59+
if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
60+
return [];
61+
}
62+
63+
$functionReflection = $this->reflectionProvider->getFunction($node->name, $scope);
64+
$name = $functionReflection->getName();
65+
if (!array_key_exists($name, self::FORMAT_ARGUMENT_POSITIONS)) {
66+
return [];
67+
}
68+
69+
$formatArgumentPosition = self::FORMAT_ARGUMENT_POSITIONS[$name];
70+
71+
$args = $node->getArgs();
72+
foreach ($args as $arg) {
73+
if ($arg->unpack) {
74+
return [];
75+
}
76+
}
77+
$argsCount = count($args);
78+
if ($argsCount < self::MINIMUM_NUMBER_OF_ARGUMENTS[$name]) {
79+
return []; // caught by CallToFunctionParametersRule
80+
}
81+
82+
$formatArgType = $scope->getType($args[$formatArgumentPosition]->value);
83+
$formatArgTypeStrings = $formatArgType->getConstantStrings();
84+
85+
// Let's start simple for now.
86+
if (count($formatArgTypeStrings) !== 1) {
87+
return [];
88+
}
89+
90+
$formatString = $formatArgTypeStrings[0];
91+
$format = $formatString->getValue();
92+
$acceptingTypes = $this->printfHelper->getPrintfPlaceholderAcceptingTypes($format);
93+
$errors = [];
94+
$typeAllowedByCallToFunctionParametersRule = TypeCombinator::union(
95+
new StringAlwaysAcceptingObjectWithToStringType(),
96+
new IntegerType(),
97+
new FloatType(),
98+
new BooleanType(),
99+
new NullType(),
100+
);
101+
102+
for ($i = $formatArgumentPosition + 1, $j = 0; $i < $argsCount; $i++, $j++) {
103+
// Some arguments may be skipped entirely.
104+
if (! array_key_exists($j, $acceptingTypes)) {
105+
continue;
106+
}
107+
108+
[$acceptingName, $acceptingCb] = $acceptingTypes[$j];
109+
$argType = $this->ruleLevelHelper->findTypeToCheck(
110+
$scope,
111+
$args[$i]->value,
112+
'',
113+
$acceptingCb,
114+
)->getType();
115+
116+
if ($argType instanceof ErrorType || $acceptingCb($argType)) {
117+
continue;
118+
}
119+
120+
// This is already reported by CallToFunctionParametersRule
121+
if (
122+
!$this->ruleLevelHelper->accepts(
123+
$typeAllowedByCallToFunctionParametersRule,
124+
$argType,
125+
$scope->isDeclareStrictTypes(),
126+
)->result
127+
) {
128+
continue;
129+
}
130+
131+
$errors[] = RuleErrorBuilder::message(
132+
sprintf(
133+
'Placeholder #%d of function %s expects %s, %s given',
134+
$j + 1,
135+
$name,
136+
$acceptingName,
137+
$argType->describe(VerbosityLevel::typeOnly()),
138+
),
139+
)->identifier('argument.type')->build();
140+
}
141+
142+
return $errors;
143+
}
144+
145+
}

0 commit comments

Comments
 (0)