-
Notifications
You must be signed in to change notification settings - Fork 666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow specifying a custom default template type/begin implementing @satisfies
#10577
base: master
Are you sure you want to change the base?
Conversation
I found these snippets: https://psalm.dev/r/fcc7e48e65<?php
/**
* @template TValue as mixed = never
*/
class FutureSplQueue {
/**
* @template TNewValue as satisfied-by<TValue>
*
* @param TNewValue $value
*
* @psalm-this-out self<(TValue|TNewValue): satisfied-by<TValue>>
*/
public function enqueue($value): void {}
}
/** @satisfies FutureSplQueue<int> */
$a = new FutureSplQueue;
// Equivalent to
/** @var FutureSplQueue<never: int> */
$a = new FutureSplQueue;
/** @psalm-trace $a */; // FutureSplQueue<never: int>
$a->enqueue(1);
/** @psalm-trace $a */; // FutureSplQueue<123: int>
$a->enqueue(2);
/** @psalm-trace $a */; // FutureSplQueue<123|321: int>
// InvalidArguments: Argument 1 of FutureSplQueue::enqueue expects int, but "str" provided
$a->enqueue("str");
https://psalm.dev/r/59402fff1a<?php
/** @satisfies int */
$a = 0;
// Equivalent to
/** @var (0: int) */
$a = 0;
// In a way, somewhat similar to the following in a possible future where PHP gets typed variables...
$a: int = 0;
/** @psalm-trace $a */; // 0: int
if (random_int(0, 1)) {
$a = 1;
}
/** @psalm-trace $a */; // (0|1): int
// Some kind of InvalidAssignment...
$a = "str";
|
@satisfies
I think this is a great effort that would handle the edge cases presented in #9838, along with generally proviting greater accuracy when dealing with values inside containers. I think it is important that |
Why it should be limited to the |
I can't undestand how it solves this issue: https://psalm.dev/r/7da42be7d7 /** @satisfies MyContainer<int> */
$t = new MyContainer;
/** @psalm-trace $t */; // MyContainer<never: int>
// OK!
$t->addValue(123);
/** @psalm-trace $t */; // MyContainer<123: int>
/**
* @param MyContainer<int> $container
*/
function test(MyContainer $container): void {}
test($t);
/** @psalm-trace $t */; // What type will be here?
|
I found these snippets: https://psalm.dev/r/7da42be7d7<?php
/**
* @template T
*/
final readonly class MyCollection
{
}
/**
* @param MyCollection<int> $_
*/
function test(MyCollection $_): void {}
/** @var MyCollection<1|2|3> */
$t = new MyCollection();
/** @psalm-trace $t */;
test($t);
/** @psalm-trace $t */;
|
As I wrote in my comment, I don't think it should be limited to the But you shouldn't be able to do this. When using Edit: Instead of just asking questions and pointing out requirements, I thought I'd spend some time creating a proposal to fulfill these requirements: ProposalThe Why mutation-free?To ensure a function doesn't save a reference to the object that Factory functions?There is another case however that we have to consider: What if the value returned by the function is the same as one that has been provided as a parameter? In this case, it is not really a factory function, but a function that "converts" the value somehow. The /**
* @var array<int,string>
*/
$a = ['test];
/**
* @var array<int,string>
*/
$referenceToA = $a;
function identity(array $a) {
return $a;
}
/**
* @satisfies array<array-key, string> $a
*/
$a = identity($a);
$a['lol'] = 'lel';
// I can calculate the sum of all array keys of $referenceToA, since they're all ints, right?
array_sum(array_keys($referenceToA));
// right? o.O I don't think Psalm already has the capabilities to ensure a return value does not contain a reference to any of the parameters. Maybe we need some sort of |
Great idea!
|
|
The choice of the syntax is backward incompatible as previously |
If I understand correctly, there are two or three things happening in this change:
Given that only the first thing is backward incompatible, maybe we could focus efforts on just this part, in order to make progress toward Psalm 6.0? This by itself still seems valuable, and the rest can be done later without breaking compatibility. A couple other thoughts on the overall idea: This seems a little different from TypeScript's I wonder if it would be worth further separating the second and third concepts above? It's not obvious to me that safely specifying template parameters should also cause the variable's type to be unchangeable. |
This is the first step in my plan to fix #9838.
First of all, the default template types all SPL datastructures will be changed to
never
; then, a new@satisfies
assertion will be introduced to avoid the (ab)use of@var
.The semantics of
@satisfies
will be entirely similar to typescript's satisfies operator, and will allow, among other things, typechecked call-site specification of template parameters.Here's a few examples of how I generally plan
@satifies
to work:I'm totally open to suggestions and feedback on this structure!
===
Explanation of the rationale for custom template defaults (btw, a custom default of
never
is already hardcoded forSplObjectStorage
in the current codebase of Psalm):Default template values
By default, if a class template parameter is not initialized in the constructor or by a
@var
annotation, Psalm will initialize it tomixed
.However, a custom default initial type can also by provided using the following syntax:
This can be often useful, like in the above example, to always force specification of a template type when constructing generic objects by specifying
never
as default type:never
is the bottom type, and thus all usages of methods relying on an uninitialized template type will usenever
and will emit Psalm issues, essentially warning the user to explicitly specify a template parameter when constructing the class.