Skip to content

Commit 4dd9bba

Browse files
schlndhschlndh
authored
Bleeding edge - Check printf parameter types
1 parent 4432832 commit 4dd9bba

File tree

9 files changed

+465
-17
lines changed

9 files changed

+465
-17
lines changed

‎conf/bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ parameters:
66
skipCheckGenericClasses!: []
77
stricterFunctionMap: true
88
reportPreciseLineForUnusedFunctionParameter: true
9+
checkPrintfParameterTypes: true
910
internalTag: true
1011
newStaticInAbstractClassStaticMethod: true
1112
checkExtensionsForComparisonOperators: 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
@@ -30,6 +30,7 @@ parameters:
3030
- DOMNamedNodeMap
3131
stricterFunctionMap: false
3232
reportPreciseLineForUnusedFunctionParameter: false
33+
checkPrintfParameterTypes: false
3334
internalTag: false
3435
newStaticInAbstractClassStaticMethod: false
3536
checkExtensionsForComparisonOperators: false

‎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
checkExtensionsForComparisonOperators: bool()

‎src/Rules/Functions/PrintfHelper.php‎

Lines changed: 77 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
use PHPStan\DependencyInjection\AutowiredService;
77
use PHPStan\Php\PhpVersion;
88
use function array_filter;
9+
use function array_keys;
910
use function count;
11+
use function in_array;
1012
use function max;
1113
use function sprintf;
1214
use function strlen;
@@ -16,21 +18,30 @@
1618
final class PrintfHelper
1719
{
1820

21+
private const PRINTF_SPECIFIER_PATTERN = '(?<specifier>[bs%s]|l?[cdeEgfFGouxX])';
22+
1923
public function __construct(private PhpVersion $phpVersion)
2024
{
2125
}
2226

2327
public function getPrintfPlaceholdersCount(string $format): int
2428
{
25-
return $this->getPlaceholdersCount('(?:[bs%s]|l?[cdeEgfFGouxX])', $format);
29+
return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format);
30+
}
31+
32+
/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
33+
public function getPrintfPlaceholders(string $format): array
34+
{
35+
return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format);
2636
}
2737

2838
public function getScanfPlaceholdersCount(string $format): int
2939
{
30-
return $this->getPlaceholdersCount('(?:[cdDeEfinosuxX%s]|\[[^\]]+\])', $format);
40+
return $this->getPlaceholdersCount('(?<specifier>[cdDeEfinosuxX%s]|\[[^\]]+\])', $format);
3141
}
3242

33-
private function getPlaceholdersCount(string $specifiersPattern, string $format): int
43+
/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
44+
private function parsePlaceholders(string $specifiersPattern, string $format): array
3445
{
3546
$addSpecifier = '';
3647
if ($this->phpVersion->supportsHhPrintfSpecifier()) {
@@ -44,34 +55,83 @@ private function getPlaceholdersCount(string $specifiersPattern, string $format)
4455
$matches = Strings::matchAll($format, $pattern, PREG_SET_ORDER);
4556

4657
if (count($matches) === 0) {
47-
return 0;
58+
return [];
4859
}
4960

5061
$placeholders = array_filter($matches, static fn (array $match): bool => strlen($match['before']) % 2 === 0);
5162

52-
if (count($placeholders) === 0) {
53-
return 0;
54-
}
63+
$result = [];
64+
$parsedPlaceholders = [];
65+
$parameterIdx = 0;
66+
$placeholderNumber = 0;
5567

56-
$maxPositionedNumber = 0;
57-
$maxOrdinaryNumber = 0;
5868
foreach ($placeholders as $placeholder) {
69+
$placeholderNumber++;
70+
$showValueSuffix = false;
71+
5972
if (isset($placeholder['width']) && $placeholder['width'] !== '') {
60-
$maxOrdinaryNumber++;
73+
$parsedPlaceholders[] = new PrintfPlaceholder(
74+
sprintf('"%s" (width)', $placeholder[0]),
75+
$parameterIdx++,
76+
$placeholderNumber,
77+
'strict-int',
78+
);
79+
$showValueSuffix = true;
6180
}
6281

6382
if (isset($placeholder['precision']) && $placeholder['precision'] !== '') {
64-
$maxOrdinaryNumber++;
83+
$parsedPlaceholders[] = new PrintfPlaceholder(
84+
sprintf('"%s" (precision)', $placeholder[0]),
85+
$parameterIdx++,
86+
$placeholderNumber,
87+
'strict-int',
88+
);
89+
$showValueSuffix = true;
6590
}
6691

67-
if (isset($placeholder['position']) && $placeholder['position'] !== '') {
68-
$maxPositionedNumber = max((int) $placeholder['position'], $maxPositionedNumber);
69-
} else {
70-
$maxOrdinaryNumber++;
71-
}
92+
$parsedPlaceholders[] = new PrintfPlaceholder(
93+
sprintf('"%s"', $placeholder[0]) . ($showValueSuffix ? ' (value)' : ''),
94+
isset($placeholder['position']) && $placeholder['position'] !== ''
95+
? $placeholder['position'] - 1
96+
: $parameterIdx++,
97+
$placeholderNumber,
98+
$this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? ''),
99+
);
100+
}
101+
102+
foreach ($parsedPlaceholders as $placeholder) {
103+
$result[$placeholder->parameterIndex][] = $placeholder;
104+
}
105+
106+
return $result;
107+
}
108+
109+
/** @phpstan-return 'string'|'int'|'float'|'mixed' */
110+
private function getAcceptingTypeBySpecifier(string $specifier): string
111+
{
112+
if ($specifier === 's') {
113+
return 'string';
114+
}
115+
116+
if (in_array($specifier, ['d', 'u', 'c', 'o', 'x', 'X', 'b'], true)) {
117+
return 'int';
118+
}
119+
120+
if (in_array($specifier, ['e', 'E', 'f', 'F', 'g', 'G', 'h', 'H'], true)) {
121+
return 'float';
72122
}
73123

74-
return max($maxPositionedNumber, $maxOrdinaryNumber);
124+
return 'mixed';
125+
}
126+
127+
private function getPlaceholdersCount(string $specifiersPattern, string $format): int
128+
{
129+
$paramIndices = array_keys($this->parsePlaceholders($specifiersPattern, $format));
130+
131+
return $paramIndices === []
132+
? 0
133+
// The indices start from 0
134+
: max($paramIndices) + 1;
75135
}
76136

77137
}

‎src/Rules/Functions/PrintfParameterTypeRule.php‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
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\Type;
18+
use PHPStan\Type\TypeCombinator;
19+
use PHPStan\Type\VerbosityLevel;
20+
use function array_key_exists;
21+
use function count;
22+
use function sprintf;
23+
24+
/**
25+
* @implements Rule<Node\Expr\FuncCall>
26+
*/
27+
final class PrintfParameterTypeRule implements Rule
28+
{
29+
30+
private const FORMAT_ARGUMENT_POSITIONS = [
31+
'printf' => 0,
32+
'sprintf' => 0,
33+
'fprintf' => 1,
34+
];
35+
private const MINIMUM_NUMBER_OF_ARGUMENTS = [
36+
'printf' => 1,
37+
'sprintf' => 1,
38+
'fprintf' => 2,
39+
];
40+
41+
public function __construct(
42+
private PrintfHelper $printfHelper,
43+
private ReflectionProvider $reflectionProvider,
44+
private RuleLevelHelper $ruleLevelHelper,
45+
)
46+
{
47+
}
48+
49+
public function getNodeType(): string
50+
{
51+
return Node\Expr\FuncCall::class;
52+
}
53+
54+
public function processNode(Node $node, Scope $scope): array
55+
{
56+
if (!($node->name instanceof Node\Name)) {
57+
return [];
58+
}
59+
60+
if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
61+
return [];
62+
}
63+
64+
$functionReflection = $this->reflectionProvider->getFunction($node->name, $scope);
65+
$name = $functionReflection->getName();
66+
if (!array_key_exists($name, self::FORMAT_ARGUMENT_POSITIONS)) {
67+
return [];
68+
}
69+
70+
$formatArgumentPosition = self::FORMAT_ARGUMENT_POSITIONS[$name];
71+
72+
$args = $node->getArgs();
73+
foreach ($args as $arg) {
74+
if ($arg->unpack) {
75+
return [];
76+
}
77+
}
78+
$argsCount = count($args);
79+
if ($argsCount < self::MINIMUM_NUMBER_OF_ARGUMENTS[$name]) {
80+
return []; // caught by CallToFunctionParametersRule
81+
}
82+
83+
$formatArgType = $scope->getType($args[$formatArgumentPosition]->value);
84+
$formatArgTypeStrings = $formatArgType->getConstantStrings();
85+
86+
// Let's start simple for now.
87+
if (count($formatArgTypeStrings) !== 1) {
88+
return [];
89+
}
90+
91+
$formatString = $formatArgTypeStrings[0];
92+
$format = $formatString->getValue();
93+
$placeholderMap = $this->printfHelper->getPrintfPlaceholders($format);
94+
$errors = [];
95+
$typeAllowedByCallToFunctionParametersRule = TypeCombinator::union(
96+
new StringAlwaysAcceptingObjectWithToStringType(),
97+
new IntegerType(),
98+
new FloatType(),
99+
new BooleanType(),
100+
new NullType(),
101+
);
102+
// Type on the left can go to the type on the right, but not vice versa.
103+
$allowedTypeNameMap = [
104+
'strict-int' => 'int',
105+
'int' => 'castable to int',
106+
'float' => 'castable to float',
107+
// These are here just for completeness. They won't be used because, these types are already enforced by
108+
// CallToFunctionParametersRule.
109+
'string' => 'castable to string',
110+
'mixed' => 'castable to string',
111+
];
112+
113+
for ($i = $formatArgumentPosition + 1, $j = 0; $i < $argsCount; $i++, $j++) {
114+
// Some arguments may be skipped entirely.
115+
foreach ($placeholderMap[$j] ?? [] as $placeholder) {
116+
$argType = $this->ruleLevelHelper->findTypeToCheck(
117+
$scope,
118+
$args[$i]->value,
119+
'',
120+
static fn (Type $t) => $placeholder->doesArgumentTypeMatchPlaceholder($t),
121+
)->getType();
122+
123+
if ($argType instanceof ErrorType || $placeholder->doesArgumentTypeMatchPlaceholder($argType)) {
124+
continue;
125+
}
126+
127+
// This is already reported by CallToFunctionParametersRule
128+
if (
129+
!$this->ruleLevelHelper->accepts(
130+
$typeAllowedByCallToFunctionParametersRule,
131+
$argType,
132+
$scope->isDeclareStrictTypes(),
133+
)->result
134+
) {
135+
continue;
136+
}
137+
138+
$errors[] = RuleErrorBuilder::message(
139+
sprintf(
140+
'Parameter #%d of function %s is expected to be %s by placeholder #%d (%s), %s given.',
141+
$i + 1,
142+
$name,
143+
$allowedTypeNameMap[$placeholder->acceptingType],
144+
$placeholder->placeholderNumber,
145+
$placeholder->label,
146+
$argType->describe(VerbosityLevel::typeOnly()),
147+
),
148+
)->identifier('argument.type')->build();
149+
}
150+
}
151+
152+
return $errors;
153+
}
154+
155+
}

‎src/Rules/Functions/PrintfPlaceholder.php‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Functions;
4+
5+
use PHPStan\ShouldNotHappenException;
6+
use PHPStan\Type\ErrorType;
7+
use PHPStan\Type\IntegerType;
8+
use PHPStan\Type\Type;
9+
10+
final class PrintfPlaceholder
11+
{
12+
13+
/** @phpstan-param 'strict-int'|'int'|'float'|'string'|'mixed' $acceptingType */
14+
public function __construct(
15+
public readonly string $label,
16+
public readonly int $parameterIndex,
17+
public readonly int $placeholderNumber,
18+
public readonly string $acceptingType,
19+
)
20+
{
21+
}
22+
23+
public function doesArgumentTypeMatchPlaceholder(Type $argumentType): bool
24+
{
25+
switch ($this->acceptingType) {
26+
case 'strict-int':
27+
return (new IntegerType())->accepts($argumentType, true)->yes();
28+
case 'int':
29+
return ! $argumentType->toInteger() instanceof ErrorType;
30+
case 'float':
31+
return ! $argumentType->toFloat() instanceof ErrorType;
32+
// The function signature already limits the parameters to stringable types, so there's
33+
// no point in checking string again here.
34+
case 'string':
35+
case 'mixed':
36+
return true;
37+
// Without this PHPStan with PHP 7.4 reports "...should return bool but return statement is missing."
38+
// Presumably, because promoted properties are turned into regular properties and the phpdoc isn't applied to the property.
39+
default:
40+
throw new ShouldNotHappenException('Unexpected type ' . $this->acceptingType);
41+
}
42+
}
43+
44+
}

0 commit comments

Comments
 (0)