Psalm: False positives after last release 3 days ago

[dereuromark]

See https://travis-ci.org/cakephp/cakephp/jobs/467742190

ERROR: UndefinedClass - src/Log/LogEngineRegistry.php:82:25 - Type Psr\Log\LoggerInterface cannot be called as a class
$instance = new $class($settings);

for

    @param string|\Psr\Log\LoggerInterface $class
    ...
        if (is_object($class)) {
            $instance = $class;
        }

        if (!isset($instance)) {
            $instance = new $class($settings);
        }

and

ERROR: EmptyArrayAccess - src/View/Helper/HtmlHelper.php:832:21 - Cannot access value on empty array variable $cellOptions
$cellOptions['class'] .= ' column-' . $i;

for

                if (isset($cellOptions['class'])) {
                    $cellOptions['class'] .= ' column-' . $i;

etc are all invalid and wrong to be reported.

Please be a bit more conservative with the errors.
If not sure, it should rather not voice an error, especially not on a low level.

[tomtomau]

LogEngineRegistry code in question
In your first example, I imagine rather than string, you're probably looking for class-string which is a special meta-type that represents a ::class string - more info in docs

Further, I imagine the control flow is quite complicated for Psalm to be reasonably be able to infer that by then it is a class-string. As a user, I personally would have restructured this control flow.

---

HtmlHelper code in question

I extracted this to a reproducible example on getpsalm.org: here

I believe $cellOptions in the above example would be inferred as array<mixed, mixed> and thus your attempt to append with .= would be trying to append to a mixed. When I annotated $cellOptions as array<mixed, string>, this issue is resolved. I think this might be a false positive in the sense that it is perhaps classifying it as the wrong error?

---

Please be a bit more conservative with the errors.
If not sure, it should rather not voice an error, especially not on a low level.

The error levels for these issue handlers are fully configurable in your psalm.xml configuration. If you'd prefer to be more conservative, you can reduce the errorLevel of particular issues to info or suppress or use the @psalm-suppress annotation in specific places too.

The documentation covers how to deal with code issues

[muglug]

Smaller reproduction: https://getpsalm.org/r/c77e16fd4e

[muglug]

That second one is now fixed in master, @dereuromark, but you can make this go away by typing $line more strictly

[muglug]

I'm going to close this. The error you're seeing with src/Log/LogEngineRegistry.php would have always been raised as an issue in Psalm had everything been typed properly (e.g. https://getpsalm.org/r/6051bbe0aa) but it's only being raised in 3.0.3 because Psalm now checks unions of mixed variables (the same feature caused the bug fixed in https://github.com/vimeo/psalm/commit/15320430dbc6770928374b95a94f067de5ed185b).

I don't have an easy suggestion for you with src/Log/LogEngineRegistry.php - I don't know exactly what exactly you allow $class to be, but adding @psalm-param annotation and using an if...else seems the best bet to make Psalm happy: https://getpsalm.org/r/7128aa3ff8

[muglug]

While @tomtomau’s right that everything is configurable, you shouldn’t have to change your configuration to adjust for Psalm bugs.

Currently all releases are checked against Vimeo’s codebase, but there’s clearly a need for more comprehensive pre-release testing. To that end I’ll create some some sort of testbed that includes large Psalm-using open source projects so as to avoid regressions.