\YouTrack\Attachment::setUrl is incorrectly annotated, $url parameter can accept not only string

[ArtemGoutsoul]

$url parameter clearly accepts other types in addition to string. Please fix the annotation since this throws off static code analyzers like psalm.

[nepda]

Thanks for your feedback! Please let me know where the problem exactly exists (file & line) and how you run psalm.

[ArtemGoutsoul]

Hi @nepda, thanks for your reply!

The issue is in this line: https://github.com/nepda/youtrack-client/blob/master/YouTrack/Attachment.php#L58

The type for $url is specified to be a string, but the method actually accepts array too. When you supply an array

$attachment = new YouTrack\Attachment();
$attachment->setUrl($some_array);

psalm generates InvalidArgument error.

PHPStorm also complains about it, it gives PhpParamsInspection error.

At the very least adding array to https://github.com/nepda/youtrack-client/blob/master/YouTrack/Attachment.php#L58 would make it better :)

[nepda]

Sorry, i don't get it. Is your suggestion, throwing exceptions on invalid (not-string) input?

[nepda]

Because we don't use a new PHP version for sure in this library, we can't use real type hints.

[ArtemGoutsoul]

Sorry for not clarifying it, basically the setUrl() method accepts array in addition to string, but the annotation does not specify it. This means that when we use it with array, all kinds of code analysis tools complain, because they expect that setUrl() only accepts string. This does not affect the actual code execution, and PHP versions have no effect on this, this only has an effect on static code analysis tools like psalm, phan, PhpStorm, etc.

So what could probably solve the static code analysis issue is just changing line https://github.com/nepda/youtrack-client/blob/master/YouTrack/Attachment.php#L58 to:

     * @param string|array $url

[nepda]

Interesting. In wich situation do you pass an array as argument? The method doesn't handle arrays properly... maybe.

[ArtemGoutsoul]

This is how we're using it, and it clearly works fine:

$attachment->setUrl(['filename' => $filename, 'file' => $filepath]);

You can see this being handled down the line in \YouTrack\Connection::request on line 303:

            if (is_array($body) && isset($body['filename']) && isset($body['file'])) {
                $filename = $body['filename'];
                $body = $body['file'];
            }

Where the resulting $body is actually the file path that is going to be picked up.

[nepda]

Great! Thanks a lot for explaining my lib :) I don't use it this much anymore.

[ArtemGoutsoul]

@nepda no probs :) thanks for fixing this and making our life a bit better!