search

psalm / psalm-plugin-phpunit

A PHPUnit plugin for Psalm
77 stars 33 forks source link

Change in behavior with PHPUnit 10 #137

Closed Xerkus closed 1 year ago

Xerkus commented 1 year ago

TLDR: type change with phpunit 10 assertions when mixed present in variable types

While migrating from phpunit 9 to phpunit 10 change in behavior was observed:

https://github.com/laminas/laminas-psr7bridge/blob/9d13ea47f3768c225cb70f7eb8b2a97424ae2800/test/Psr7ServerRequestTest.php#L202-L204

/**
 * @psalm-trace $test
 */
$test = $laminasRequest->getFiles();
/**
 * @psalm-trace $test
 */
$this->assertCount(1, $test);
$this->assertTrue(isset($test['foo']));

With phpunit 9 psalm produces following complaints:

ERROR: Trace - test/Psr7ServerRequestTest.php:205:9 - $test: Laminas\Stdlib\ParametersInterface|mixed (see https://psalm.dev/224)
        /**
         * @psalm-trace $test
         */
        $test = $laminasRequest->getFiles();

ERROR: Trace - test/Psr7ServerRequestTest.php:209:9 - $test: Laminas\Stdlib\ParametersInterface|mixed (see https://psalm.dev/224)
        /**
         * @psalm-trace $test
         */
        $this->assertCount(1, $test);

ERROR: MixedArgument - test/Psr7ServerRequestTest.php:209:31 - Argument 2 of LaminasTest\Psr7Bridge\Psr7ServerRequestTest::assertCount cannot be Laminas\Stdlib\ParametersInterface|mixed, expecting Countable|iterable<mixed, mixed> (see https://psalm.dev/030)
        $this->assertCount(1, $test);

  The type of $test is sourced from here - vendor/laminas/laminas-http/src/Request.php:342:21
    public function getFiles($name = null, $default = null)

However after upgrading to phpunit 10 same code now produces

ERROR: Trace - test/Psr7ServerRequestTest.php:205:9 - $test: Laminas\Stdlib\ParametersInterface|mixed (see https://psalm.dev/224)
        /**
         * @psalm-trace $test
         */
        $test = $laminasRequest->getFiles();

ERROR: Trace - test/Psr7ServerRequestTest.php:209:9 - $test: Countable|iterable<mixed, mixed> (see https://psalm.dev/224)
        /**
         * @psalm-trace $test
         */
        $this->assertCount(1, $test);

ERROR: MixedArgument - test/Psr7ServerRequestTest.php:209:31 - Argument 2 of LaminasTest\Psr7Bridge\Psr7ServerRequestTest::assertCount cannot be Laminas\Stdlib\ParametersInterface|mixed, expecting Countable|iterable<mixed, mixed> (see https://psalm.dev/030)
        $this->assertCount(1, $test);

  The type of $test is sourced from here - vendor/laminas/laminas-http/src/Request.php:342:21
    public function getFiles($name = null, $default = null)

ERROR: UndefinedInterfaceMethod - test/Psr7ServerRequestTest.php:210:33 - Method Countable::offsetGet does not exist (see https://psalm.dev/181)
        $this->assertTrue(isset($test['foo']));

As evident from the trace, with phpunit 9 type of $test is not changed by call to assertCount(). However with phpunit 10 type information is lost.

To narrow it down I changed type declaration of getFiles() method to omit mixed. In the absence of mixed behavior is identical with both phpunit versions, so this seems to be some kind of interplay of mixed in union type and assertions in phpunit 10.

ERROR: Trace - test/Psr7ServerRequestTest.php:205:9 - $test: Laminas\Stdlib\ParametersInterface (see https://psalm.dev/224)
        /**
         * @psalm-trace $test
         */
        $test = $laminasRequest->getFiles();

ERROR: Trace - test/Psr7ServerRequestTest.php:209:9 - $test: Laminas\Stdlib\ParametersInterface (see https://psalm.dev/224)
        /**
         * @psalm-trace $test
         */
        $this->assertCount(1, $test);

This does not appear as intended behavior. I do not know how psalm or this plugin works to dig deeper.

Installed dependencies ``` amphp/amp v2.6.2 A non-blocking concurrency framework for PHP applications. amphp/byte-stream v1.8.1 A stream abstraction to make working with non-blocking I/O simple. composer/package-versions-deprecated 1.11.99.5 Composer plugin that provides efficient querying for installed package versions (no runtime IO) composer/pcre 3.1.0 PCRE wrapping library that offers type-safe preg_* replacements. composer/semver 3.3.2 Semver library that offers utilities, version constraint parsing and validation. composer/xdebug-handler 3.0.3 Restarts a process without Xdebug. dealerdirect/phpcodesniffer-composer-installer v0.7.2 PHP_CodeSniffer Standards Composer Installer Plugin dnoegel/php-xdg-base-dir v0.1.1 implementation of xdg base directory specification for php felixfbecker/advanced-json-rpc v3.2.1 A more advanced JSONRPC implementation felixfbecker/language-server-protocol v1.5.2 PHP classes for the Language Server Protocol fidry/cpu-core-counter 0.5.1 Tiny utility to get the number of CPU cores. laminas/laminas-coding-standard 2.5.0 Laminas Coding Standard laminas/laminas-diactoros 2.25.2 PSR HTTP Message implementations laminas/laminas-escaper 2.12.0 Securely and safely escape HTML, HTML attributes, JavaScript, CSS, and URLs laminas/laminas-http 2.18.0 Provides an easy interface for performing Hyper-Text Transfer Protocol (HTTP) requests laminas/laminas-loader 2.9.0 Autoloading and plugin loading strategies laminas/laminas-servicemanager 3.15.0 Factory-Driven Dependency Injection Container laminas/laminas-stdlib 3.17.0 SPL extensions, array utilities, error handlers, and more laminas/laminas-uri 2.10.0 A component that aids in manipulating and validating » Uniform Resource Identifiers (URIs) laminas/laminas-validator 2.30.1 Validation classes for a wide range of domains, and the ability to chain validators to create complex validation criteria myclabs/deep-copy 1.11.1 Create deep copies (clones) of your objects netresearch/jsonmapper v4.2.0 Map nested JSON structures onto PHP classes nikic/php-parser v4.15.4 A PHP parser written in PHP phar-io/manifest 2.0.3 Component for reading phar.io manifest information from a PHP Archive (PHAR) phar-io/version 3.2.1 Library for handling version information and constraints phpdocumentor/reflection-common 2.2.0 Common reflection classes used by phpdocumentor to reflect the code structure phpdocumentor/reflection-docblock 5.3.0 With this component, a library can provide support for annotations via DocBlocks or otherwise retrieve information that is embedded in a DocBlock. phpdocumentor/type-resolver 1.6.2 A PSR-5 based resolver of Class names, Types and Structural Element Names phpstan/phpdoc-parser 1.5.1 PHPDoc parser with support for nullable, intersection and generic types phpunit/php-code-coverage 10.1.1 Library that provides collection, processing, and rendering functionality for PHP code coverage information. phpunit/php-file-iterator 4.0.1 FilterIterator implementation that filters files based on a list of suffixes. phpunit/php-invoker 4.0.0 Invoke callables with a timeout phpunit/php-text-template 3.0.0 Simple template engine. phpunit/php-timer 6.0.0 Utility class for timing phpunit/phpunit 10.1.1 The PHP Unit Testing framework. psalm/plugin-phpunit 0.18.4 Psalm plugin for PHPUnit psr/container 2.0.2 Common Container Interface (PHP FIG PSR-11) psr/http-factory 1.0.2 Common interfaces for PSR-7 HTTP message factories psr/http-message 1.1 Common interface for HTTP messages psr/log 3.0.0 Common interface for logging libraries sebastian/cli-parser 2.0.0 Library for parsing CLI options sebastian/code-unit 2.0.0 Collection of value objects that represent the PHP code units sebastian/code-unit-reverse-lookup 3.0.0 Looks up which function or method a line of code belongs to sebastian/comparator 5.0.0 Provides the functionality to compare PHP values for equality sebastian/complexity 3.0.0 Library for calculating the complexity of PHP code units sebastian/diff 5.0.1 Diff implementation sebastian/environment 6.0.1 Provides functionality to handle HHVM/PHP environments sebastian/exporter 5.0.0 Provides the functionality to export PHP variables for visualization sebastian/global-state 6.0.0 Snapshotting of global state sebastian/lines-of-code 2.0.0 Library for counting the lines of code in PHP source code sebastian/object-enumerator 5.0.0 Traverses array structures and object graphs to enumerate all referenced objects sebastian/object-reflector 3.0.0 Allows reflection of object attributes, including inherited and non-public ones sebastian/recursion-context 5.0.0 Provides functionality to recursively process PHP variables sebastian/type 4.0.0 Collection of value objects that represent the types of the PHP type system sebastian/version 4.0.1 Library that helps with managing the version number of Git-hosted PHP projects slevomat/coding-standard 7.2.1 Slevomat Coding Standard for PHP_CodeSniffer complements Consistence Coding Standard by providing sniffs with additional checks. spatie/array-to-xml 3.1.5 Convert an array to xml squizlabs/php_codesniffer 3.7.2 PHP_CodeSniffer tokenizes PHP, JavaScript and CSS files and detects violations of a defined set of coding standards. symfony/console v6.2.8 Eases the creation of beautiful and testable command line interfaces symfony/deprecation-contracts v3.2.1 A generic function and convention to trigger deprecation notices symfony/filesystem v6.2.7 Provides basic utilities for the filesystem symfony/polyfill-ctype v1.27.0 Symfony polyfill for ctype functions symfony/polyfill-intl-grapheme v1.27.0 Symfony polyfill for intl's grapheme_* functions symfony/polyfill-intl-normalizer v1.27.0 Symfony polyfill for intl's Normalizer class and related functions symfony/polyfill-mbstring v1.27.0 Symfony polyfill for the Mbstring extension symfony/service-contracts v3.2.1 Generic abstractions related to writing services symfony/string v6.2.8 Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a unified way theseer/tokenizer 1.2.1 A small library for converting tokenized PHP source code into XML and potentially other formats vimeo/psalm 5.9.0 A static analysis tool for finding errors in PHP applications webimpress/coding-standard 1.3.1 Webimpress Coding Standard webmozart/assert 1.11.0 Assertions to validate method input/output with nice error messages. ```
orklah commented 1 year ago

The issue that is raised is kind of correct when mixed is in the type. Doing $test['...'] is indeed risky if $test is mixed.

However, this should disappear when you remove mixed from $test. Could you display the output for when you remove mixed from the type?

Xerkus commented 1 year ago

Last codeblock has the result of removal of mixed.

And of course only after I opened issue I realized what is going on. phpunit 10 added native typehints which psalm happily uses to narrow down types.

What I complained about was not the error due to ambiguous type but the change of the type that discarded ParameterInterface and mixed to be replaced with Countable, leading to pretty confusing errors down the line.

Particularly, since call to assertCount() with mixed was previously baselined, the remaining errors were misleading. https://github.com/laminas/laminas-psr7bridge/actions/runs/4731059549/jobs/8395553258?pr=32#step:3:324