Detect flags in FilesystemIterator to get a more correct iterator type

[xPaw]

https://psalm.dev/r/8930178275
https://psalm.dev/r/de250db5e3

See #4725 (comment)

Depending on the flag, it may return different things:
FilesystemIterator::CURRENT_AS_PATHNAME, FilesystemIterator::CURRENT_AS_FILEINFO, FilesystemIterator::CURRENT_AS_SELF.

Currently psalm will complain that methods are being called on strings, despite fileinfo being the default flag of FilesystemIterator.

cc @orklah

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/8930178275

<?php

$Images = new FilesystemIterator( '/tmp' );

foreach( $Images as $Image )
{
    echo $Image->getRealPath();
}

Psalm output (using commit ef0d19e):

ERROR: PossiblyInvalidMethodCall - 7:18 - Cannot call method on possible string variable $Image

https://psalm.dev/r/de250db5e3

<?php

$Images = new FilesystemIterator( '/tmp' );

foreach( $Images as $Image )
{
    /** @psalm-trace $Image */
    echo $Image->getRealPath();
}

Psalm output (using commit ef0d19e):

ERROR: PossiblyInvalidMethodCall - 8:18 - Cannot call method on possible string variable $Image

INFO: Trace - 8:5 - $Image: FilesystemIterator|SplFileInfo|null|string

[orklah]

I guess we could do something like that:
https://psalm.dev/r/ab96d2798b

It should work pretty well until a certain point: conditional returns don't seem to take masks for flags.

It means that if there is only 1 flag, (or even 0 for the default) it should work well, otherwise it won't.

It will still be an improvement compared to current situation but it's not optimal

@weirdan @muglug Do you have other ideas? Should psalm support something like @return (T&1 is 1 ? type1 : type2)

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/ab96d2798b

<?php
/**
 * @template T
 */
class test{
    /**
     * @var T
     */
    private $flag;
    
    /**
     * @param T $flag
     */
    public function __construct($flag){
        $this->flag = $flag;
    }
    
    /**
     * @return (T is 1 ? int : stdClass)
     */
    public function getthing(){
        if($this->flag === 1){
            return 0;
        }
        else{
            return new stdClass();
        }
    }
}

Psalm output (using commit ef0d19e):

No issues!

[weirdan]

Do you propose to use flags value as template parameter? I don't think we should do that - just consider the inheritance and parameter types. Is FilesystemIterator<123> allowed where FilesystemIterator<456> expected? We shouldn't need to have to answer that.

[orklah]

Mmh... didn't thought about that. It's unfortunate, it was nearly working.

I don't have any other idea...

[weirdan]

I think we need to figure out what actual type parameter(s) should be. I'd say it probably has a single type parameter that matches the type of current(), giving us the following:

/**
 * @template T of static|SplFileInfo|string
 * @template-implements SeekableIterator<string,T>
 * will probably need to adjust DirectoryIterator type and pass T to it as well
 */
class FilesystemIterator extends DirectoryIterator implements SeekableIterator {}

This way users will need to use explicit docblock types, turning OP's code into

/** @var FilesystemIterator<SplFileInfo> $Images */
$Images = new FilesystemIterator( '/tmp' );

foreach( $Images as $Image )
{
    echo $Image->getRealPath();
}

which looks like acceptable interim solution to me until the time Psalm learns to infer T.

[soyuka]

I have a similar issue, there is

psalm/stubs/CoreGenericFunctions.phpstub

Lines 593 to 606 in ab5ddb1

	/**
	* @psalm-pure
	*
	* @psalm-flow ($subject) -(array-assignment)-> return
	*
	* @template TFlags as int-mask<0, 1, 2, 4>
	*
	* @param TFlags $flags
	*
	* @return (TFlags is 0|2 ? non-empty-list<string>|false : (TFlags is 1|3 ? list<string>|false : list<array{string,int}>|false))
	*
	* @psalm-ignore-falsable-return
	*/
	function preg_split(string $pattern, string $subject, int $limit = -1, int $flags = 0) {}

but doesn't look to be working see https://psalm.dev/r/1e66b3769d or https://psalm.dev/r/fc3d2b9db2

EDIT: The following is working on Psalm 4.5.1: https://psalm.dev/r/ea8b934a44

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/1e66b3769d

<?php

class Test {
    
    const AS_STRING = 1;
    const AS_ARRAY = 2;
    const WITHOUT_ALIASES = 4;

    /**
     * @param int-mask<Test::AS_STRING, Test::AS_ARRAY, Test::WITHOUT_ALIASES> $output
     * @return ($output is 1 ? string : array)
     */
    public function columns(?array $fields = null, int $output = Test::AS_STRING)
    {
        $fields = $fields ?? ['a', 'b'];
        return $output === Test::AS_STRING ? implode(', ', $fields) : $fields;
    }
}

$t = new Test();

echo implode(', ', $t->columns(null, Test::AS_ARRAY));
echo explode(', ', $t->columns());

Psalm output (using commit 257a1ca):

ERROR: PossiblyInvalidArgument - 22:20 - Argument 2 of implode expects array<array-key, mixed>, possibly different type array<array-key, mixed>|string provided

ERROR: InvalidCast - 23:6 - non-empty-list<string> cannot be cast to string

https://psalm.dev/r/fc3d2b9db2

<?php

class Test {
    
    const AS_STRING = 1;
    const AS_ARRAY = 2;
    const WITHOUT_ALIASES = 4;

    /**
     * @template TFlags as int-mask<Test::AS_STRING, Test::AS_ARRAY, Test::WITHOUT_ALIASES>
     * @param TFlags $output
     * @return (TFlags is 1|4 ? string : array)
     */
    public function columns(?array $fields = null, int $output = Test::AS_STRING)
    {
        $fields = $fields ?? ['a', 'b'];
        return $output === Test::AS_STRING ? implode(', ', $fields) : $fields;
    }
}

$t = new Test();

echo implode(', ', $t->columns(null, Test::AS_ARRAY));
echo explode(', ', $t->columns());

Psalm output (using commit 257a1ca):

ERROR: PossiblyInvalidArgument - 23:20 - Argument 2 of implode expects array<array-key, mixed>, possibly different type array<array-key, mixed>|string provided

ERROR: InvalidCast - 24:6 - non-empty-list<string> cannot be cast to string

[orklah]

You should create a new issue for that, it doesn't seem related to this one