psalm-assert-if-true with boolean properties

[franmomu]

It would be nice if to be able to apply psalm-assert-if-true|false in boolean properties, something like:

https://psalm.dev/r/0d9e50fd84

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/0d9e50fd84

<?php

class A
{
    /**
     * @psalm-assert-if-true string $this->versionField
     */
    public bool $isVersioned = true;
    
    public ?string $versionField = null;
    
    /**
     * @psalm-assert-if-true string $this->versionField
     */
    public function isVersioned(): bool
    {
        return $this->isVersioned;
    }
}

$a = new A();

if ($a->isVersioned()) {
    /** @psalm-trace $a->versionField */
}

if ($a->isVersioned) {
    /** @psalm-trace $a->versionField */
}

Psalm output (using commit b5b5c20):

INFO: Trace - 24:0 - $a->versionField: string

INFO: Trace - 28:0 - $a->versionField: null|string

[AndrolGenhald]

I don't think this is really the intent of assertions. A function asserting something should check that that thing is true. For this to be safe Psalm would need to always know the value of A::$isVersioned for every instance, which is more like a template, but unfortunately templates also don't work very well for this use-case.

I've run into scenarios where something like this would be useful as well, but I don't think this is the solution.

[rzvc]

I also want this, and not only for properties, but for any bool. From the user code's POV, there's no difference between if (foo()) and if ($foo).

The most common use case for this is when one bool indicates that a range of other stuff is valid:

/**
 * @return null | array{ foo: Foo, bar: Bar }
 */
function test() : mixed
{
	/**
	 * @psalm-assert Foo $foo
	 * @psalm-assert Bar $bar
	 */
	$b = false;

	if (...)
	{
		$foo = new Foo();

		if (...)
		{
			$bar = new Bar();
			$b = true;
		}
	}

	// $foo and $bar would need to be both checked at this point,
	// but that is unnecessary because we know that when $b is true,
	// they're both fine,
	return $b ? ['foo' => $foo, 'bar' => $bar] : null;
}

Edit: Added a bit more complexity to the example, to illustrate the issue better.

Edit 2: Another example, since I hit this issue again: https://psalm.dev/r/5ab78c4f38

[still-dreaming-1]

This request is within the spirit of already existing features. It is similar to the example in https://psalm.dev/docs/annotating_code/adding_assertions/ where ->hasException() is called to know that ->getException() will return Exception and not ?Exception. However, there is a note under that example which says

Please note that the example above only works if you enable method call memoization in the config file or annotate the class as immutable.

Therefore I think this feature might only make sense in a similar scenario, such as when the class is immutable or the property is readonly. Here is an example that illustrates how this feature could be used/useful: https://psalm.dev/r/8aecb78824

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/8aecb78824

<?php

/**
 * @psalm-immutable
 * @template TKey of array-key
 * @template TVal
 */
final class ImmutableCollection
{
    /**
     * @psalm-assert-if-true non-empty-list<TKey> $this->keys()
     */
    public readonly bool $empty;
    
    /**
     * @param array<TKey, TVal> $arr
     */
    public function __construct(private readonly array $arr) {
        $this->empty = empty($arr);
    }
    
    /**
     * @return list<TKey>
     */
    public function keys(): array {
        return array_keys($this->arr);
    }
}

/**
 * @param non-empty-list $nonEmptyList
 */
function doNonEmptyListyThings(array $nonEmptyList): void {
    foreach ($nonEmptyList as $_) {
    }
}

$coll = new ImmutableCollection(['msg' => 'hola']);
if (!$coll->empty) {
    // with new feature, this would not be an issue because already confirmed not empty
    doNonEmptyListyThings($coll->keys());
    
    // showing Psalm is almost smart enough:
    $keys = $coll->keys();
    if (!empty($keys)) {
        doNonEmptyListyThings($keys); // no issue here since already confirmed not empty
    }
}

Psalm output (using commit a75d26a):

ERROR: InvalidArgument - 41:27 - Argument 1 of doNonEmptyListyThings expects non-empty-list<mixed>, but list<'msg'> provided