incorrect `InvalidTemplateParam` using when a `Closure` argument uses `T` as an argument.
given the following example: https://psalm.dev/r/7f1d36d1f8, the error here is incorrect.
Option type is immutable by itself, and adding @psalm-immutable annotation solves the problem: https://psalm.dev/r/3deed330bf
however, this leads to another issue, at this point, Option<T>::map<Tu>((Closure(T): Tu) $f): Option<Tu> is considered pure, allowing to declare get_organization_owner() function below as pure ( see: https://psalm.dev/r/4396258d6f ), this leads to another problem: get_organization_owner() is not actually pure, neither is map() method, get_organization_owner() calls Option::map(), which calls $f, which is an impure function ( in the example above it is indeed pure, but Organization::getOwner() can have side effects ), declaring just Option::map() as impure based on the fact that it calls an impure function ( $f ) that could result in side effects elsewhere but not in the option object itself is not possible, meaning we are forced to remove the @psalm-immutable annotation from the class Option, leading us back to the first issue.
I found these snippets:
https://psalm.dev/r/7f1d36d1f8
<?php
/**
* @template-covariant T
*/
interface Option
{
/**
* Maps an `Option<T>` to `Option<Tu>` by applying a function to a contained value.
*
* @template Tu
*
* @param (Closure(T): Tu) $closure
*
* @return Option<Tu>
*/
public function map(Closure $closure): Option;
}
class Owner {}
class Organization { public function getOwner(): Owner { return new Owner(); } }
class GovernmentOrganization extends Organization {}
/** @return Option<Organization> */
function get_org(): Option { exit(0); }
/** @return Option<GovernmentOrganization> */
function get_gov_org(): Option { exit(0); }
/**
* @param Option<Organization> $organization
*
* @return Option<Owner>
*/
function get_organization_owner(Option $organization): Option
{
return $organization->map(static fn(Organization $organization) => $organization->getOwner());
}
$_ = get_organization_owner(get_org());
$_ = get_organization_owner(get_gov_org());
Psalm output (using commit fbd240b):
ERROR: InvalidTemplateParam - 13:15 - Template param T of Option is marked covariant and cannot be used here
https://psalm.dev/r/3deed330bf
<?php
/**
* @template-covariant T
* @psalm-immutable
*/
interface Option
{
/**
* Maps an `Option<T>` to `Option<Tu>` by applying a function to a contained value.
*
* @template Tu
*
* @param (Closure(T): Tu) $closure
*
* @return Option<Tu>
*/
public function map(Closure $closure): Option;
}
class Owner {}
class Organization { public function getOwner(): Owner { return new Owner(); } }
class GovernmentOrganization extends Organization {}
/** @return Option<Organization> */
function get_org(): Option { exit(0); }
/** @return Option<GovernmentOrganization> */
function get_gov_org(): Option { exit(0); }
/**
* @param Option<Organization> $organization
*
* @return Option<Owner>
*/
function get_organization_owner(Option $organization): Option
{
return $organization->map(static fn(Organization $organization) => $organization->getOwner());
}
$_ = get_organization_owner(get_org());
$_ = get_organization_owner(get_gov_org());
Psalm output (using commit fbd240b):
No issues!
https://psalm.dev/r/4396258d6f
<?php
/**
* @template-covariant T
* @psalm-immutable
*/
interface Option
{
/**
* Maps an `Option<T>` to `Option<Tu>` by applying a function to a contained value.
*
* @template Tu
*
* @param (Closure(T): Tu) $closure
*
* @return Option<Tu>
*/
public function map(Closure $closure): Option;
}
class Owner {}
class Organization { public function getOwner(): Owner { return new Owner(); } }
class GovernmentOrganization extends Organization {}
/** @return Option<Organization> */
function get_org(): Option { exit(0); }
/** @return Option<GovernmentOrganization> */
function get_gov_org(): Option { exit(0); }
/**
* @param Option<Organization> $organization
*
* @return Option<Owner>
*
* @pure
*/
function get_organization_owner(Option $organization): Option
{
return $organization->map(static fn(Organization $organization) => $organization->getOwner());
}
$_ = get_organization_owner(get_org());
$_ = get_organization_owner(get_gov_org());
Psalm output (using commit fbd240b):
No issues!
I think the issue boils down to this: https://psalm.dev/r/eb422e250d
not allowing an immutable object to call an impure function, even though we know for a fact that it cannot change the state of the object.
maybe a new annotation ( e.g: @psalm-internal-mutation-free ) can be used in such a function to declare that it is not totally pure, but it won't change the object itself, allowing the use of it in @psalm-immutable classes only.
I found these snippets:
https://psalm.dev/r/eb422e250d
<?php
/**
* @psalm-template-covariant T
* @psalm-immutable
*/
final class ValueReference {
/**
* @param T $value
*/
public function __construct(
public readonly mixed $value
) {}
/**
* @template Tu
*
* @param (Closure(T): Tu) $f
*
* @return ValueReference<Tu>
*/
public function map(Closure $f): ValueReference
{
$new = $f($this->value);
return new ValueReference($new);
}
}
$a = new ValueReference('hello');
$b = $a->map(fn(string $val) => $val . ' world');
print $b->value; // hello world
Psalm output (using commit fbd240b):
ERROR: ImpureFunctionCall - 24:16 - Cannot call an impure function from a mutation-free context
for example, there's no difference technically between: https://psalm.dev/r/eb422e250d and https://psalm.dev/r/7be3725e19, but one is valid, while the other is not.
I found these snippets:
https://psalm.dev/r/eb422e250d
<?php
/**
* @psalm-template-covariant T
* @psalm-immutable
*/
final class ValueReference {
/**
* @param T $value
*/
public function __construct(
public readonly mixed $value
) {}
/**
* @template Tu
*
* @param (Closure(T): Tu) $f
*
* @return ValueReference<Tu>
*/
public function map(Closure $f): ValueReference
{
$new = $f($this->value);
return new ValueReference($new);
}
}
$a = new ValueReference('hello');
$b = $a->map(fn(string $val) => $val . ' world');
print $b->value; // hello world
Psalm output (using commit fbd240b):
ERROR: ImpureFunctionCall - 24:16 - Cannot call an impure function from a mutation-free context
https://psalm.dev/r/7be3725e19
<?php
/**
* @psalm-template-covariant T
* @psalm-immutable
*/
final class ValueReference {
/**
* @param T $value
*/
public function __construct(
public readonly mixed $value
) {}
}
/**
* @template T
* @template Tu
*
* @param ValueReference<T> $ref
* @param (Closure(T): Tu) $f
*
* @return ValueReference<Tu>
*/
function map_reference(ValueReference $ref, Closure $f): ValueReference
{
$new = $f($ref->value);
return new ValueReference($new);
}
$a = new ValueReference('hello');
$b = map_reference($a, fn(string $val) => $val . ' world');
print $b->value; // hello world
Psalm output (using commit fbd240b):
No issues!
Seems like the same sort of problem as #8116.
I think i have a better solution for this problem.
The Option class given in the first example, is not immutable ( by psalm definition, which is completely side-effect free ), but rather a readonly class, what we can do is add support for @psalm-readonly annotation on classes ( same as readonly class Foo { .. } in PHP 8.2 ), so we would allow Closure arguments to take T as an argument themselves, and we know that the class is not side effect free, it's just that it's own state doesn't change.
I'll send a PR with tests which might help you understand what i mean more ^^"
I think the issue boils down to this: https://psalm.dev/r/eb422e250d
not allowing an immutable object to call an impure function, even though we know for a fact that it cannot change the state of the object.
If $this->value is an object, it is passed by reference to $f. Therefore, it may be mutated. I think Psalm behaves correctly, and map() has to accept something like pure-callable but for Closure. Introduce pure-Closure type?
I found these snippets:
https://psalm.dev/r/eb422e250d
<?php
/**
* @psalm-template-covariant T
* @psalm-immutable
*/
final class ValueReference {
/**
* @param T $value
*/
public function __construct(
public readonly mixed $value
) {}
/**
* @template Tu
*
* @param (Closure(T): Tu) $f
*
* @return ValueReference<Tu>
*/
public function map(Closure $f): ValueReference
{
$new = $f($this->value);
return new ValueReference($new);
}
}
$a = new ValueReference('hello');
$b = $a->map(fn(string $val) => $val . ' world');
print $b->value; // hello world
Psalm output (using commit fbd240b):
ERROR: ImpureFunctionCall - 24:16 - Cannot call an impure function from a mutation-free context
No, it actually has nothing to do with objects: https://psalm.dev/r/3033cae1ca
I found these snippets:
https://psalm.dev/r/3033cae1ca
<?php
/**
* @psalm-template-covariant T of string|int|float
* @psalm-immutable
*/
final class ValueReference {
/**
* @param T $value
*/
public function __construct(
public readonly mixed $value
) {}
/**
* @template Tu of string|int|float
*
* @param (Closure(T): Tu) $f
*
* @return ValueReference<Tu>
*/
public function map(Closure $f): ValueReference
{
$new = $f($this->value);
return new ValueReference($new);
}
}
$a = new ValueReference('hello');
$b = $a->map(fn(string $val) => $val . ' world');
print $b->value; // hello world
Psalm output (using commit fbd240b):
ERROR: ImpureFunctionCall - 24:16 - Cannot call an impure function from a mutation-free context
Closure could still take its argument by reference though, and again mutate it. https://psalm.dev/r/3423b8e3e9
@someniatko huh, you're right. I'd consider that a separate bug though. I think we need to add annotation for references in callables and make callable(&int) not be a subtype of callable(int), because the example I just gave crashes...
I found these snippets:
https://psalm.dev/r/e59d68ba57
<?php
/** @param callable(int): int $closure */
function takesClosure($closure): int
{
return $closure(123);
}
function takesIntByRef(int &$i): int
{
return $i**2;
}
takesClosure("takesIntByRef");
Psalm output (using commit 85fe7e8):
No issues!