Feature request: Conditionals for psalm-assert and psalm-out

[gmazzap]

Conditional assertions is something that is somehow partially implemented via psalm-assert-if-true / psalm-assert-if-false, however that regards the methods' return type, not parameters.

Imagine a code like this:

/** @template T of string|null */
class StringOrNull
{
  /** @var T */
  public ?string $value;

  /** @param T $value */
  public function __construct(?string $value) {
    $this->value = $value;
  }

  public function setFromStringOrInt(string|int $value): void {
    if (is_string($value)) {
        $this->value = $value;
    }
  }
}

$stringOrNull = new StringOrNull(null);
$stringOrNull->setFromStringOrInt('Test');

After last line, $stringOrNull is StringOrNull<string>, but Psalm still sees it as StringOrNull<null>. See https://psalm.dev/r/17f5693767

To solve this, we could have something like:

/** 
 * @psalm-this-out-if($value is string) StringOrNull<string>
 */
public function setFromStringOrInt(string|int $value): void {
    if (is_string($value->value)) {
        $this->value = $value->value;
    }
}

(I mean, there could be different syntaxes, I tried to imagine the one similar to existing Psalm syntax)

Something very similar could be implemented for psalm-assert.

/** 
 * @psalm-assert-if($value is string) StringOrNull<string> $this
 * @psalm-assert-if($value is string) string $this->value
 */
public function setFromStringOrInt(string|int $value): void {
    if (is_string($value->value)) {
        $this->value = $value->value;
    }
}

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/17f5693767

```php value = $value; } public function setFromStringOrInt(string|int $value): void { if (is_string($value)) { // This is a separate issue, probably https://github.com/vimeo/psalm/issues/7739 $this->value = $value; } } } /** @psalm-trace $stringOrNull */ $stringOrNull = new StringOrNull(null); $stringOrNull->setFromStringOrInt('Test'); /** * $nowShouldBeWrapString should be StringOrNull but is still StringOrNull * * @psalm-trace $nowShouldBeWrapString */ $nowShouldBeWrapString = $stringOrNull; print $nowShouldBeWrapString->value; ``` ``` Psalm output (using commit 10ea05a): INFO: Trace - 24:1 - $stringOrNull: StringOrNull INFO: Trace - 32:1 - $nowShouldBeWrapString: StringOrNull ERROR: InvalidPropertyAssignmentValue - 17:24 - $this->value with declared type 'T:StringOrNull as null|string' cannot be assigned type 'string' ```

[AndrolGenhald]

You can do this which almost gets you there, but it doesn't handle ints how you'd want. This pattern is actually not safe though, so I'm not sure the cast issue should be considered a false positive: https://3v4l.org/mKN4o

We can't guarantee that there aren't other references to an object, so changing the template type isn't possible to do safely. I think we need to spend some time thinking through how this feature should work and what guarantees we want to provide, and make clear to the user what their responsibilities are to ensure the code is correct. I'm not sure @psalm-this-out is actually a good idea in general.

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/e571dfee0c

```php value = $value; } /** * @template T2 of string|int * @param T2 $value * @psalm-this-out self */ public function setFromStringOrInt(string|int $value): void { if (is_string($value)) { // This is a separate issue, probably https://github.com/vimeo/psalm/issues/7739 $this->value = $value; } } } /** @psalm-trace $stringOrNull */ $stringOrNull = new StringOrNull(null); $stringOrNull->setFromStringOrInt('Test'); /** * $nowShouldBeWrapString should be StringOrNull but is still StringOrNull * * @psalm-trace $nowShouldBeWrapString */ $nowShouldBeWrapString = $stringOrNull; print $nowShouldBeWrapString->value; ``` ``` Psalm output (using commit 10ea05a): INFO: Trace - 29:1 - $stringOrNull: StringOrNull INFO: Trace - 37:1 - $nowShouldBeWrapString: StringOrNull<'Test'> ERROR: InvalidPropertyAssignmentValue - 22:24 - $this->value with declared type 'T:StringOrNull as null|string' cannot be assigned type 'T2:fn-stringornull::setfromstringorint as string' ```

https://psalm.dev/r/780a7361fd

```php value = $value; } /** * @template T2 of string|int * @param T2 $value * @psalm-this-out self */ public function setFromStringOrInt(string|int $value): void { if (is_string($value)) { // This is a separate issue, probably https://github.com/vimeo/psalm/issues/7739 $this->value = $value; } } } /** @psalm-trace $stringOrNull */ $stringOrNull = new StringOrNull(null); $stringOrNull->setFromStringOrInt(1); /** * $nowShouldBeWrapString should be StringOrNull but is still StringOrNull * * @psalm-trace $nowShouldBeWrapString */ $nowShouldBeWrapString = $stringOrNull; print $nowShouldBeWrapString->value; ``` ``` Psalm output (using commit 10ea05a): INFO: Trace - 29:1 - $stringOrNull: StringOrNull INFO: Trace - 37:1 - $nowShouldBeWrapString: StringOrNull<1> ERROR: InvalidPropertyAssignmentValue - 22:24 - $this->value with declared type 'T:StringOrNull as null|string' cannot be assigned type 'T2:fn-stringornull::setfromstringorint as string' ```

[gmazzap]

We can't guarantee that there aren't other references to an object

Sure thing.

I'm not sure @psalm-this-out is actually a good idea in general.

Me neither :) Mutable design is bad, because of the issue you described: that is a runtime issue, which you can't find using a static analysis, and that is why many people (inlcuding me) advocates for immutability. In my company code-styling we've banned setters for a reason :)

I used psalm-this-out in my example because I realized that was the smallest code I could use to describe the feature request. If you want to implement conditional assertions only for @psalm-assert, I would be happy anyway.

Still, I think @psalm-assert-if would be very useful, as it could save writing code, after all it is a combination of two assertions, which could be merged in one.

Take:

/** @psalm-assert-if($param is Foo) <something> */
function ifParamIsFooAssertSomething($param) {}

it would be equivalent to:

/** @psalm-assert-if-true Foo $param  */
function assertParamIsFoo($param): bool {}

/** @psalm-assert <something> */
function assertSomething(): bool {}

function ifParamIsFooAssertSomething($param) {
   if (assertParamIsFoo($param)) {
     assertSomething();
   }
}

So we can already obtain what I'm asking, but what I'm asking would make it easier/less verbose.