Psalm: `substr()` compare against string literal

[staabm]

when comparing a substr-string against a string literal, a error should be emitted, when comparison cannot be true.

I would expect https://psalm.dev/r/50ab25c47e to emit a error, because a substring which is 6 chars long can never be equal to a string literal which is 8 chars long.

so in fact such a check would guard against off-by-one errors or similar of the substr() int arguments.
it is pretty common to see bugs regarding incorrect int arguments to these string functions.

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/50ab25c47e

<?php declare(strict_types = 1);

class HelloWorld
{
    public function sayHello(): void
    {
        $imgsrc  = 'asdfasdfasdasdfasfdafsfdas';
        $a = substr($imgsrc, 0, 6) === 'file:///';
    }
}

Psalm output (using commit 344a732):

INFO: UnusedVariable - 8:3 - Variable $a is never referenced

[ondrejmirtes]

My response on PHPStan's issue tracker:

Hi, I don't think it's that common to call substr() with all arguments literal, because you can just write fasdfa instead :)

[staabm]

the thing to validate would be whether substr($whateverString, 0, 3) can acutally result in the same string length then the string-literal we compare against.

looking e.g. at symfony I can see at least the following places where it would be usefull (just to pick a few examples of a popular codebase)

https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Translation/Loader/PoFileLoader.php#L86
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Translation/Loader/PoFileLoader.php#L94
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Translation/Loader/PoFileLoader.php#L105
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Translation/Loader/PoFileLoader.php#L107
https://github.com/symfony/symfony/blob/c650fe6dfc15c9faf7756514ef5bdc490cb926f4/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php#L47
https://github.com/symfony/symfony/blob/1866a2a88c6c47ce64cd92a48cbc7a434eeec776/src/Symfony/Component/Console/Formatter/OutputFormatter.php#L47
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/HttpFoundation/UrlHelper.php#L34
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/HttpFoundation/UrlHelper.php#L63
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Config/Exception/LoaderLoadException.php#L35
https://github.com/symfony/symfony/blob/d5bbdca755e80dbe9df2c6197858bba35f6c7a4d/src/Symfony/Component/DependencyInjection/Extension/Extension.php#L70
https://github.com/symfony/symfony/blob/c650fe6dfc15c9faf7756514ef5bdc490cb926f4/src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php#L34
https://github.com/symfony/symfony/blob/d6dd06ba8937b6a5e7d9d64a5c5a5d71473563f1/src/Symfony/Component/Asset/UrlPackage.php#L124
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/PropertyInfo/Util/PhpDocTypeHelper.php#L111
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php#L47
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php#L57
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Bundle/TwigBundle/TemplateIterator.php#L52
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Mime/Tests/Encoder/Rfc2231EncoderTest.php#L105
https://github.com/symfony/symfony/blob/e60a876201b5b306d0c81a24d9a3db997192079c/src/Symfony/Component/Mime/Tests/Encoder/Rfc2231EncoderTest.php#L113
https://github.com/symfony/symfony/blob/092632dc1435ff2ab50d27ea5106ece7afe1a6df/src/Symfony/Component/HttpKernel/DependencyInjection/AddAnnotatedClassesToCachePass.php#L65

(this list is not exhaustive - stopped searching on search result page 10 of at least 31)

[weirdan]

This would require having a type string-of-N-characters(n) (discussed in #3855), which was recently rejected.

[muglug]

This could just be a very direct check when encountering substr(..., 0, 5) === “aa”, which wouldn’t require Psalm to store information about string length

[muglug]

@staabm I’d welcome a PR to BinaryOpAnalyzer

[muglug]

Actually I did it myself

[muglug]

This is a good check which caught a bug in Psalm itself

[staabm]

Awesome, thx!