Skip to content

Commit dd176fe

Browse files
authored
Merge pull request #12077 from danog/purity-template-arg-count
Purity templates: omitted purity arguments, instanceof narrowing, inference of templated calls
2 parents 8bd13a0 + 35435b0 commit dd176fe

10 files changed

Lines changed: 338 additions & 7 deletions

File tree

‎src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallPurityAnalyzer.php‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,16 +149,17 @@ public static function analyze(
149149

150150
// @psalm-purity-from-template: the call also needs the capabilities of the closures the
151151
// templates are bound to here; this can only make the call less pure, never more
152-
$method_capabilities = CallPurityResolver::getCallCapabilities(
152+
$template_capabilities = CallPurityResolver::getCallCapabilities(
153153
$statements_analyzer,
154154
$codebase,
155155
$method_storage,
156-
$method_capabilities,
156+
Capabilities::NONE,
157157
$template_result,
158158
$class_template_params,
159159
self::isThis($stmt->var),
160160
self::isFromGlobalState($statements_analyzer, $stmt->var),
161161
);
162+
$method_capabilities |= $template_capabilities;
162163

163164
// whether the result may come from global state depends on what this call reads,
164165
// not on what it writes through its by-reference arguments
@@ -199,6 +200,11 @@ public static function analyze(
199200
self::receiverAllowsInternalMutations($statements_analyzer, $stmt->var),
200201
);
201202

203+
// the callee's level does not include what its purity templates are bound to here
204+
if ($template_capabilities !== Capabilities::NONE) {
205+
$statements_analyzer->signalMutationOnlyInferred($template_capabilities);
206+
}
207+
202208
if ($reads_globals) {
203209
$stmt->setAttribute(GlobalStateAnalyzer::ATTRIBUTE, true);
204210
}
@@ -251,6 +257,7 @@ public static function analyze(
251257
&& !$method_storage->throws
252258
&& !$method_storage->return_type?->isNever()
253259
&& !$method_storage->signature_return_type?->isNever()
260+
&& !self::isCalledForItsTemplatesEffects($method_storage)
254261
) {
255262
IssueBuffer::maybeAdd(
256263
new UnusedMethodCall(
@@ -319,4 +326,18 @@ public static function analyze(
319326
}
320327
}
321328
}
329+
330+
/**
331+
* A void method whose purity comes from purity templates (`Iterator::next()`) is called for the
332+
* effects of what they are bound to, or of its own engine state (the cursor of an iterator):
333+
* its call is not unused even when they turn out to be none.
334+
*
335+
* @psalm-mutation-free
336+
*/
337+
public static function isCalledForItsTemplatesEffects(MethodStorage $method_storage): bool
338+
{
339+
return $method_storage->purity_from_templates !== []
340+
&& ($method_storage->return_type?->isVoid() === true
341+
|| $method_storage->signature_return_type?->isVoid() === true);
342+
}
322343
}

‎src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -836,13 +836,15 @@ private static function analyzeConstructorPurity(
836836
$cased_method_id = 'constructor ' . $codebase->methods->getCasedMethodId($declaring_method_id);
837837

838838
// the constructor only mutates the new object: what it does to it is fine
839-
$resolved_capabilities = CallPurityResolver::getCallCapabilities(
839+
$template_capabilities = CallPurityResolver::getCallCapabilities(
840840
$statements_analyzer,
841841
$codebase,
842842
$method_storage,
843-
$method_storage->capabilities & ~(Capabilities::READ_PROPS | Capabilities::WRITE_THIS_PROPS),
843+
Capabilities::NONE,
844844
$template_result,
845845
);
846+
$resolved_capabilities = $template_capabilities
847+
| ($method_storage->capabilities & ~(Capabilities::READ_PROPS | Capabilities::WRITE_THIS_PROPS));
846848

847849
$args = $stmt->getArgs();
848850

@@ -868,6 +870,11 @@ private static function analyzeConstructorPurity(
868870
true,
869871
);
870872

873+
// the constructor's level does not include what its purity templates are bound to here
874+
if ($template_capabilities !== Capabilities::NONE) {
875+
$statements_analyzer->signalMutationOnlyInferred($template_capabilities);
876+
}
877+
871878
// the constructor may have stored global state in the new object: what it reads, not
872879
// what it writes through its by-reference arguments
873880
if (($resolved_capabilities & Capabilities::READ_GLOBALS) !== 0) {

‎src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,14 +295,16 @@ public static function analyze(
295295

296296
$call_args = $stmt->isFirstClassCallable() ? [] : $stmt->getArgs();
297297

298-
$resolved_capabilities = CallPurityResolver::getCallCapabilities(
298+
// what the purity templates of the method are bound to here
299+
$template_capabilities = CallPurityResolver::getCallCapabilities(
299300
$statements_analyzer,
300301
$codebase,
301302
$method_storage,
302-
$method_storage->capabilities,
303+
Capabilities::NONE,
303304
$template_result,
304305
$found_generic_params ?? [],
305306
);
307+
$resolved_capabilities = $method_storage->capabilities | $template_capabilities;
306308

307309
$call_capabilities = ByRefArgumentAnalyzer::adjustCapabilities(
308310
$statements_analyzer,
@@ -329,6 +331,11 @@ public static function analyze(
329331
$method_storage,
330332
);
331333

334+
// the callee's level does not include what its purity templates are bound to here
335+
if ($template_capabilities !== Capabilities::NONE) {
336+
$statements_analyzer->signalMutationOnlyInferred($template_capabilities);
337+
}
338+
332339
// what this call reads, not what it writes through its by-reference arguments
333340
if (($resolved_capabilities & Capabilities::READ_GLOBALS) !== 0) {
334341
$stmt->setAttribute(GlobalStateAnalyzer::ATTRIBUTE, true);

‎src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,12 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
585585
}
586586

587587
if (!isset($docblock_info->purity_template_defaults[$purity_template])) {
588+
// without a default, a purity argument left out (`Box`) stands for the
589+
// template's upper bound
590+
if ($bound !== null && Capabilities::isPurityType($bound)) {
591+
$storage->template_defaults[$purity_template] = $bound;
592+
}
593+
588594
continue;
589595
}
590596

‎src/Psalm/Internal/Type/AssertionReconciler.php‎

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@
6565
use Psalm\Type\Union;
6666

6767
use function array_intersect_key;
68+
use function array_keys;
6869
use function array_merge;
70+
use function array_search;
6971
use function count;
7072
use function is_string;
7173

@@ -435,7 +437,19 @@ private static function refine(
435437
&& ($codebase->classExists($existing_var_type_part->value)
436438
|| $codebase->interfaceExists($existing_var_type_part->value))
437439
) {
438-
$existing_var_type_part = $existing_var_type_part->addIntersectionType($new_type_part);
440+
$intersected_type_part = $new_type_part;
441+
442+
if ($existing_var_type_part instanceof TGenericObject
443+
&& !$new_type_part instanceof TGenericObject
444+
) {
445+
$intersected_type_part = self::inferTemplateParamsFromParent(
446+
$codebase,
447+
$new_type_part,
448+
$existing_var_type_part,
449+
);
450+
}
451+
452+
$existing_var_type_part = $existing_var_type_part->addIntersectionType($intersected_type_part);
439453
$acceptable_atomic_types[] = $existing_var_type_part;
440454
}
441455

@@ -598,6 +612,80 @@ private static function filterTypeWithAnother(
598612
return null;
599613
}
600614

615+
/**
616+
* Narrowing a `Parent` to a child class whose templates are passed straight to Parent's
617+
* (`@extends Parent`, `@implements Iterator[P]`) gives the child those arguments:
618+
* `Iterator[pure]` narrowed to `Child` is a `Child[pure]`.
619+
*
620+
* @psalm-capabilities read-props
621+
*/
622+
private static function inferTemplateParamsFromParent(
623+
Codebase $codebase,
624+
TNamedObject $child,
625+
TGenericObject $parent,
626+
): TNamedObject {
627+
if (!$codebase->classlike_storage_provider->has($child->value)
628+
|| !$codebase->classlike_storage_provider->has($parent->value)
629+
) {
630+
return $child;
631+
}
632+
633+
$child_storage = $codebase->classlike_storage_provider->get($child->value);
634+
$parent_storage = $codebase->classlike_storage_provider->get($parent->value);
635+
636+
$extended_params = $child_storage->template_extended_params[$parent_storage->name] ?? null;
637+
638+
if ($child_storage->template_types === null
639+
|| $child_storage->template_types === []
640+
|| $parent_storage->template_types === null
641+
|| $parent_storage->template_types === []
642+
|| $extended_params === null
643+
) {
644+
return $child;
645+
}
646+
647+
$parent_template_names = array_keys($parent_storage->template_types);
648+
$type_params = [];
649+
650+
foreach (array_keys($child_storage->template_types) as $template_name) {
651+
$inferred = null;
652+
653+
foreach ($extended_params as $parent_template_name => $extended_type) {
654+
$offset = array_search($parent_template_name, $parent_template_names, true);
655+
656+
if ($offset === false || !isset($parent->type_params[$offset]) || !$extended_type->isSingle()) {
657+
continue;
658+
}
659+
660+
$extended_atomic = $extended_type->getSingleAtomic();
661+
662+
if ($extended_atomic instanceof TTemplateParam
663+
&& $extended_atomic->param_name === $template_name
664+
&& $extended_atomic->defining_class === $child_storage->name
665+
) {
666+
$inferred = $parent->type_params[$offset];
667+
break;
668+
}
669+
}
670+
671+
if ($inferred === null) {
672+
// a template not passed straight to the parent: nothing is known about it
673+
return $child;
674+
}
675+
676+
$type_params[] = $inferred;
677+
}
678+
679+
return new TGenericObject(
680+
$child->value,
681+
$type_params,
682+
false,
683+
$child->is_static,
684+
$child->extra_types,
685+
$child->from_docblock,
686+
);
687+
}
688+
601689
private static function filterAtomicWithAnother(
602690
Atomic &$type_1_atomic,
603691
Atomic $type_2_atomic,
@@ -662,6 +750,10 @@ private static function filterAtomicWithAnother(
662750
&& ($codebase->interfaceExists($type_1_atomic->value)
663751
|| $codebase->interfaceExists($type_2_atomic->value))
664752
) {
753+
if ($type_1_atomic instanceof TGenericObject && !$type_2_atomic instanceof TGenericObject) {
754+
$type_2_atomic = self::inferTemplateParamsFromParent($codebase, $type_2_atomic, $type_1_atomic);
755+
}
756+
665757
return $type_2_atomic->addIntersectionType($type_1_atomic);
666758
}
667759

‎src/Psalm/Internal/Type/PurityArguments.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ public static function fit(array $type_params, ClassLikeStorage $storage): bool
7878
&& count($purity_args) <= count($storage->template_types ?? []) - $type_template_count;
7979
}
8080

81+
/**
82+
* The type parameters of a use of the class without the purity arguments it has no purity
83+
* templates for: what `static[P]` becomes for a late static class that binds `P` itself
84+
* (`@extends Base[pure]`) and so declares no purity template of its own.
85+
*
86+
* @param array $type_params
87+
* @return list
88+
* @psalm-mutation-free
89+
*/
90+
public static function trim(array $type_params, ClassLikeStorage $storage): array
91+
{
92+
[$type_args, $purity_args] = self::split($type_params);
93+
$purity_template_count = count($storage->template_types ?? []) - self::countTypeTemplates($storage);
94+
95+
return [...$type_args, ...array_slice($purity_args, 0, $purity_template_count)];
96+
}
97+
8198
/**
8299
* The type parameters of a use of the class, with the purity arguments in the positions of the
83100
* purity templates: `Foo[pure]`, for a class with type templates, gives them their bounds.

‎src/Psalm/Internal/Type/TypeExpander.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,17 @@ private static function expandNamedObject(
693693
$is_static,
694694
$is_static_resolved,
695695
);
696+
697+
if ($codebase->classlike_storage_provider->has($static_class_type->value)) {
698+
$type_params = PurityArguments::trim(
699+
$return_type->type_params,
700+
$codebase->classlike_storage_provider->get($static_class_type->value),
701+
);
702+
703+
if ($type_params !== [] && count($type_params) !== count($return_type->type_params)) {
704+
$return_type = $return_type->setTypeParams($type_params);
705+
}
706+
}
696707
} elseif ($static_class_type instanceof TNamedObject) {
697708
$return_type = $static_class_type->setIsStatic(
698709
$is_static,

‎tests/FileManipulation/PureAnnotationAdditionTest.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,49 @@ public function hook(): int {
262262
'issues_to_fix' => ['MissingPureAnnotation'],
263263
'safe_types' => true,
264264
],
265+
'dontSuggestWhatAClassBoundPurityTemplateExceeds' => [
266+
'input' => '
267+
/**
268+
* @psalm-purity-template P
269+
*/
270+
abstract class Base {
271+
/**
272+
* @psalm-capabilities read-props
273+
* @psalm-purity-from-template P
274+
*/
275+
public function limit(): int {
276+
return 1;
277+
}
278+
}
279+
280+
final class Db extends Base {
281+
public function limit(): int {
282+
return parent::limit() + 1;
283+
}
284+
}',
285+
'output' => '
286+
/**
287+
* @psalm-purity-template P
288+
*/
289+
abstract class Base {
290+
/**
291+
* @psalm-capabilities read-props
292+
* @psalm-purity-from-template P
293+
*/
294+
public function limit(): int {
295+
return 1;
296+
}
297+
}
298+
299+
final class Db extends Base {
300+
public function limit(): int {
301+
return parent::limit() + 1;
302+
}
303+
}',
304+
'php_version' => '8.1',
305+
'issues_to_fix' => ['MissingPureAnnotation'],
306+
'safe_types' => true,
307+
],
265308
'addPureAnnotationToFunction' => [
266309
'input' => '
267310
function foo(string $s): string {

0 commit comments

Comments
 (0)