Skip to content

Commit fd12705

Browse files
schlndhschlndh
committed
fix printf type rule error messages
1 parent c0915cb commit fd12705

File tree

5 files changed

+150
-136
lines changed

5 files changed

+150
-136
lines changed

‎src/Rules/Functions/PrintfHelper.php

Lines changed: 35 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,13 @@
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;
118
use function array_filter;
12-
use function array_flip;
139
use function array_keys;
14-
use function array_map;
15-
use function array_reduce;
1610
use function count;
1711
use function in_array;
1812
use function max;
19-
use function sort;
2013
use function sprintf;
2114
use function strlen;
22-
use function usort;
2315
use const PREG_SET_ORDER;
2416

2517
/** @phpstan-type AcceptingTypeString 'strict-int'|'int'|'float'|'string'|'mixed' */
@@ -38,71 +30,18 @@ public function getPrintfPlaceholdersCount(string $format): int
3830
return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format);
3931
}
4032

41-
/** @return array<int, array{string, callable(Type): bool}> position => [type name, matches callback] */
42-
public function getPrintfPlaceholderAcceptingTypes(string $format): array
33+
/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
34+
public function getPrintfPlaceholders(string $format): array
4335
{
44-
$placeholders = $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format);
45-
$result = [];
46-
// Type on the left can go to the type on the right, but not vice versa.
47-
$typeSequenceMap = array_flip(['int', 'float', 'string', 'mixed']);
48-
49-
foreach ($placeholders as $position => $types) {
50-
sort($types);
51-
$typeNames = array_map(
52-
static fn (string $t) => $t === 'strict-int'
53-
? 'int'
54-
: $t,
55-
$types,
56-
);
57-
$typeName = array_reduce(
58-
$typeNames,
59-
static fn (string $carry, string $type) => $typeSequenceMap[$carry] < $typeSequenceMap[$type]
60-
? $carry
61-
: $type,
62-
'mixed',
63-
);
64-
$result[$position] = [
65-
$typeName,
66-
static function (Type $t) use ($types): bool {
67-
foreach ($types as $acceptingType) {
68-
switch ($acceptingType) {
69-
case 'strict-int':
70-
$subresult = (new IntegerType())->accepts($t, true)->yes();
71-
break;
72-
case 'int':
73-
$subresult = ! $t->toInteger() instanceof ErrorType;
74-
break;
75-
case 'float':
76-
$subresult = ! $t->toFloat() instanceof ErrorType;
77-
break;
78-
// The function signature already limits the parameters to stringable types, so there's
79-
// no point in checking string again here.
80-
case 'string':
81-
case 'mixed':
82-
default:
83-
$subresult = true;
84-
break;
85-
}
86-
87-
if (!$subresult) {
88-
return false;
89-
}
90-
}
91-
92-
return true;
93-
},
94-
];
95-
}
96-
97-
return $result;
36+
return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format);
9837
}
9938

10039
public function getScanfPlaceholdersCount(string $format): int
10140
{
10241
return $this->getPlaceholdersCount('(?<specifier>[cdDeEfinosuxX%s]|\[[^\]]+\])', $format);
10342
}
10443

105-
/** @phpstan-return array<int, non-empty-list<AcceptingTypeString>> position => type */
44+
/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
10645
private function parsePlaceholders(string $specifiersPattern, string $format): array
10746
{
10847
$addSpecifier = '';
@@ -123,38 +62,49 @@ private function parsePlaceholders(string $specifiersPattern, string $format): a
12362
$placeholders = array_filter($matches, static fn (array $match): bool => strlen($match['before']) % 2 === 0);
12463

12564
$result = [];
126-
$positionalPlaceholders = [];
127-
$idx = 0;
65+
$parsedPlaceholders = [];
66+
$parameterIdx = 0;
67+
$placeholderNumber = 0;
12868

12969
foreach ($placeholders as $placeholder) {
70+
$placeholderNumber++;
71+
$showValueSuffix = false;
72+
13073
if (isset($placeholder['width']) && $placeholder['width'] !== '') {
131-
$result[$idx++] = ['strict-int' => 1];
74+
$parsedPlaceholders[] = new PrintfPlaceholder(
75+
sprintf('"%s" (width)', $placeholder[0]),
76+
$parameterIdx++,
77+
$placeholderNumber,
78+
'strict-int',
79+
);
80+
$showValueSuffix = true;
13281
}
13382

13483
if (isset($placeholder['precision']) && $placeholder['precision'] !== '') {
135-
$result[$idx++] = ['strict-int' => 1];
136-
}
137-
138-
if (isset($placeholder['position']) && $placeholder['position'] !== '') {
139-
// It may reference future position, so we have to process them later.
140-
$positionalPlaceholders[] = $placeholder;
141-
continue;
84+
$parsedPlaceholders[] = new PrintfPlaceholder(
85+
sprintf('"%s" (precision)', $placeholder[0]),
86+
$parameterIdx++,
87+
$placeholderNumber,
88+
'strict-int',
89+
);
90+
$showValueSuffix = true;
14291
}
14392

144-
$result[$idx++][$this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? '')] = 1;
93+
$parsedPlaceholders[] = new PrintfPlaceholder(
94+
sprintf('"%s"', $placeholder[0]) . ($showValueSuffix ? ' (value)' : ''),
95+
isset($placeholder['position']) && $placeholder['position'] !== ''
96+
? $placeholder['position'] - 1
97+
: $parameterIdx++,
98+
$placeholderNumber,
99+
$this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? ''),
100+
);
145101
}
146102

147-
usort(
148-
$positionalPlaceholders,
149-
static fn (array $a, array $b) => (int) $a['position'] <=> (int) $b['position'],
150-
);
151-
152-
foreach ($positionalPlaceholders as $placeholder) {
153-
$idx = $placeholder['position'] - 1;
154-
$result[$idx][$this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? '')] = 1;
103+
foreach ($parsedPlaceholders as $placeholder) {
104+
$result[$placeholder->parameterIndex][] = $placeholder;
155105
}
156106

157-
return array_map(static fn (array $a) => array_keys($a), $result);
107+
return $result;
158108
}
159109

160110
/** @phpstan-return 'string'|'int'|'float'|'mixed' */

‎src/Rules/Functions/PrintfParameterTypeRule.php

Lines changed: 46 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPStan\Type\IntegerType;
1515
use PHPStan\Type\NullType;
1616
use PHPStan\Type\StringAlwaysAcceptingObjectWithToStringType;
17+
use PHPStan\Type\Type;
1718
use PHPStan\Type\TypeCombinator;
1819
use PHPStan\Type\VerbosityLevel;
1920
use function array_key_exists;
@@ -89,7 +90,7 @@ public function processNode(Node $node, Scope $scope): array
8990

9091
$formatString = $formatArgTypeStrings[0];
9192
$format = $formatString->getValue();
92-
$acceptingTypes = $this->printfHelper->getPrintfPlaceholderAcceptingTypes($format);
93+
$placeholderMap = $this->printfHelper->getPrintfPlaceholders($format);
9394
$errors = [];
9495
$typeAllowedByCallToFunctionParametersRule = TypeCombinator::union(
9596
new StringAlwaysAcceptingObjectWithToStringType(),
@@ -98,45 +99,54 @@ public function processNode(Node $node, Scope $scope): array
9899
new BooleanType(),
99100
new NullType(),
100101
);
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+
];
101112

102113
for ($i = $formatArgumentPosition + 1, $j = 0; $i < $argsCount; $i++, $j++) {
103114
// Some arguments may be skipped entirely.
104-
if (! array_key_exists($j, $acceptingTypes)) {
105-
continue;
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->acceptedType],
144+
$placeholder->placeholderNumber,
145+
$placeholder->label,
146+
$argType->describe(VerbosityLevel::typeOnly()),
147+
),
148+
)->identifier('argument.type')->build();
106149
}
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();
140150
}
141151

142152
return $errors;

‎src/Rules/Functions/PrintfPlaceholder.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Functions;
4+
5+
use PHPStan\Type\ErrorType;
6+
use PHPStan\Type\IntegerType;
7+
use PHPStan\Type\Type;
8+
9+
final class PrintfPlaceholder
10+
{
11+
12+
/** @phpstan-param 'strict-int'|'int'|'float'|'string'|'mixed' $acceptedType */
13+
public function __construct(
14+
public readonly string $label,
15+
public readonly int $parameterIndex,
16+
public readonly int $placeholderNumber,
17+
public readonly string $acceptedType,
18+
)
19+
{
20+
}
21+
22+
public function doesArgumentTypeMatchPlaceholder(Type $argumentType): bool
23+
{
24+
switch ($this->acceptedType) {
25+
case 'strict-int':
26+
return (new IntegerType())->accepts($argumentType, true)->yes();
27+
case 'int':
28+
return ! $argumentType->toInteger() instanceof ErrorType;
29+
case 'float':
30+
return ! $argumentType->toFloat() instanceof ErrorType;
31+
// The function signature already limits the parameters to stringable types, so there's
32+
// no point in checking string again here.
33+
case 'string':
34+
case 'mixed':
35+
default:
36+
return true;
37+
}
38+
}
39+
40+
}

0 commit comments

Comments
 (0)