sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.69k stars 2.2k forks source link

Mocking method with object return type throws warning #3706

Closed davidbyoung closed 5 years ago

davidbyoung commented 5 years ago
Q A
PHPUnit version 8.2-gd58761f7b
PHP version 7.3.0
Installation Method Composer

Mocking a method with an object return type throws a warning, eg Method resolve may not return value of type object. This was not happening in 8.1. For example, let's say I have this interface:

<?php

declare(strict_types=1);

namespace Foo;

interface IDependencyResolver
{
    public function resolve(string $className): object;
}

And let's say I have the following (contrived) test:

<?php

declare(strict_types=1);

namespace Foo\Tests;

use Foo\IDependencyResolver;
use PHPUnit\Framework\TestCase;

class SomeTest extends TestCase
{
    public function testResolve(): void
    {
        $expectedFoo = new class() { };
        $dependencyResolver = $this->createMock(IDependencyResolver::class);
        $dependencyResolver->expects($this->once())
            ->method('resolve')
            ->with('foo')
            ->willReturn($expectedFoo);

         // This is a silly, contrived test
         $this->assertSame($expectedFoo, $dependencyResolver->resolve('foo'));
    }
}

I would expect this to pass, but instead I get a warning Method resolve may not return value of type object. I'm guessing it has something to do with this commit.

composer info | sort:

aphiria/middleware                 dev-master ebe4ae8     The Aphiria middleware library
aphiria/net                        dev-master 1ad15b8     The Aphiria network library
aphiria/router                     dev-master d9fa90d     The Aphiria router library
aphiria/serialization              dev-master ae9144c     The Aphiria serialization library
composer/semver                    dev-master 37e4db2     Semver library that offers utilities, version constraint parsing and validation.
composer/xdebug-handler            1.3.2                  Restarts a process without xdebug.
doctrine/annotations               1.7.x-dev 3f35255      Docblock Annotations Parser
doctrine/instantiator              dev-master fb21dea     A small, lightweight utility to instantiate objects in PHP without invoking their constructors
doctrine/lexer                     dev-master ee614dd     PHP Doctrine Lexer parser library that can be used in Top-Down, Recursive Descent Parsers.
friendsofphp/php-cs-fixer          dev-master 3cd7ead     A tool to automatically fix PHP code style
monolog/monolog                    1.23.0                 Sends your logs to files, sockets, inboxes, databases and various web services
myclabs/deep-copy                  1.x-dev e6828ef        Create deep copies (clones) of your objects
opis/closure                       dev-master ccb8e39     A library that can be used to serialize closures (anonymous functions) and arbitrary objects.
opulence/collections               dev-master 719b55f     The Opulence collection library
opulence/io                        dev-master edd216e     The Opulence IO library
opulence/ioc                       dev-develop b4b66dd    The Opulence Ioc library
paragonie/random_compat            v9.99.99.x-dev 0947f25 PHP 5.x polyfill for random_bytes() and random_int() from PHP 7
phar-io/manifest                   dev-master 7761fca     Component for reading phar.io manifest information from a PHP Archive (PHAR)
phar-io/version                    2.0.1                  Library for handling version information and constraints
php-cs-fixer/diff                  v1.3.0                 sebastian/diff v2 backport support for PHP5.6
phpdocumentor/reflection-common    1.0.1                  Common reflection classes used by phpdocumentor to reflect the code structure
phpdocumentor/reflection-docblock  4.3.1                  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        0.4.0
phpspec/prophecy                   dev-master 7e27218     Highly opinionated mocking framework for PHP 5.3+
phpunit/php-code-coverage          dev-master a29e9d6     Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator          dev-master 80ca798     FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-text-template          1.2.1                  Simple template engine.
phpunit/php-timer                  dev-master e826e3a     Utility class for timing
phpunit/php-token-stream           dev-master 2d574d6     Wrapper around PHP's tokenizer extension.
phpunit/phpunit                    dev-master d58761f     The PHP Unit Testing framework.
psr/log                            dev-master c4421fc     Common interface for logging libraries
sebastian/code-unit-reverse-lookup dev-master aa16919     Looks up which function or method a line of code belongs to
sebastian/comparator               dev-master 6ddb43a     Provides the functionality to compare PHP values for equality
sebastian/diff                     dev-master 1d90f91     Diff implementation
sebastian/environment              dev-master 48c9235     Provides functionality to handle HHVM/PHP environments
sebastian/exporter                 dev-master cf3a70c     Provides the functionality to export PHP variables for visualization
sebastian/global-state             dev-master 6ddc744     Snapshotting of global state
sebastian/object-enumerator        dev-master eb45b4b     Traverses array structures and object graphs to enumerate all referenced objects
sebastian/object-reflector         dev-master a253986     Allows reflection of object attributes, including inherited and non-public ones
sebastian/recursion-context        dev-master b6a7e76     Provides functionality to recursively process PHP variables
sebastian/resource-operations      dev-master 366bbb1     Provides a list of PHP built-in functions that operate on resources
sebastian/type                     dev-master a857fc8     Collection of value objects that represent the types of the PHP type system
sebastian/version                  2.0.1                  Library that helps with managing the version number of Git-hosted PHP projects
symfony/console                    dev-master 5accd36     Symfony Console Component
symfony/contracts                  dev-master 30e42ac     A set of abstractions extracted out of the Symfony components
symfony/event-dispatcher           dev-master 02cec9f     Symfony EventDispatcher Component
symfony/filesystem                 dev-master 82cd8a3     Symfony Filesystem Component
symfony/finder                     dev-master dde9d15     Symfony Finder Component
symfony/options-resolver           dev-master 94cbb72     Symfony OptionsResolver Component
symfony/polyfill-ctype             dev-master 82ebae0     Symfony polyfill for ctype functions
symfony/polyfill-mbstring          dev-master fe5e94c     Symfony polyfill for the Mbstring extension
symfony/polyfill-php70             dev-master bc4858f     Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-php72             dev-master ab50dcf     Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions
symfony/polyfill-php73             dev-master d1fb4ab     Symfony polyfill backporting some PHP 7.3+ features to lower PHP versions
symfony/process                    dev-master c43b190     Symfony Process Component
symfony/stopwatch                  dev-master 7d5a6a8     Symfony Stopwatch Component
theseer/tokenizer                  1.1.2                  A small library for converting tokenized PHP source code into XML and potentially other formats
webmozart/assert                   1.4.0                  Assertions to validate method input/output with nice error messages.
davidbyoung commented 5 years ago

I believe this PR should fix the problem

mbaumgartl commented 5 years ago

I can still reproduce the warning in the most recent version of PHPUnit (3e9a1656a) using the example above.

Name Version
PHPUnit version 3e9a1656a
PHP version 7.2.19 + 7.3.6
Installation Method composer

composer info | sort

doctrine/instantiator              1.2.0  
myclabs/deep-copy                  1.9.1  
phar-io/manifest                   1.0.3  
phar-io/version                    2.0.1  
phpdocumentor/reflection-common    1.0.1  
phpdocumentor/reflection-docblock  4.3.1  
phpdocumentor/type-resolver        0.4.0  
phpspec/prophecy                   1.8.1  
phpunit/php-code-coverage          7.0.5  
phpunit/php-file-iterator          2.0.2  
phpunit/php-text-template          1.2.1  
phpunit/php-timer                  2.1.2  
phpunit/php-token-stream           3.0.1  
sebastian/code-unit-reverse-lookup 1.0.1  
sebastian/comparator               3.0.2  
sebastian/diff                     3.0.2  
sebastian/environment              4.2.2  
sebastian/exporter                 3.1.0  
sebastian/global-state             3.0.0  
sebastian/object-enumerator        3.0.3  
sebastian/object-reflector         1.1.1  
sebastian/recursion-context        3.0.0  
sebastian/resource-operations      2.0.1  
sebastian/type                     1.1.1  
sebastian/version                  2.0.1  
symfony/polyfill-ctype             v1.11.0
theseer/tokenizer                  1.1.3  
webmozart/assert                   1.4.0  
sebastianbergmann commented 5 years ago

I was able to reproduce the issue with

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

interface I
{
    public function m();
}

final class Test extends TestCase
{
    public function testOne(): void
    {
        $i = $this->createMock(I::class);

        $i->method('m')->willReturn(new stdClass);

        $this->assertInstanceOf(stdClass::class, $i->m());
    }
}
sebastianbergmann commented 5 years ago

https://github.com/sebastianbergmann/type/commit/26a5e767f2970d5651c40625a6e2407c18e19916 (part of version 1.1.2 of the type package) fixes https://github.com/sebastianbergmann/phpunit/issues/3706#issuecomment-503408399 for me.

davidkmenta commented 5 years ago

@sebastianbergmann it's still happening to me when a mock of an interface returns another mock...

interface FeatureRepositoryInterface
{
    public function findOneByNameAndContext(string $name, Context $context): ?FeatureInterface;
}
$featureMock = $this->createMock(FeatureInterface::class)
    ->expects($this->once())
    ->method('isActive')
    ->willReturn($expectedIsActive);

$this->createMock(FeatureRepositoryInterface::class)
    ->expects($this->once())
    ->method('findOneByNameAndContext')
    ->with($featureName, $context)
    ->willReturn($featureMock);

Method findOneByNameAndContext may not return value of type object

Version of phpunit: 8.2.3 Version of type: 1.1.3

sebastianbergmann commented 5 years ago

@davidkmenta Your example was incomplete. I have completed it like so:

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

interface FeatureInterface
{
    public function isActive(): bool;
}

interface FeatureRepositoryInterface
{
    public function findOneByNameAndContext(string $name, Context $context): ?FeatureInterface;
}

final class Test extends TestCase
{
    public function testOne(): void
    {
        $featureMock = $this->createMock(FeatureInterface::class)
            ->expects($this->once())
            ->method('isActive')
            ->willReturn(true);

        $x = $this->createMock(FeatureRepositoryInterface::class)
            ->expects($this->once())
            ->method('findOneByNameAndContext')
            ->willReturn($featureMock);
    }
}
PHPUnit 8.2.5 by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 46 ms, Memory: 6.00 MB

There was 1 warning:

1) Test::testOne
Method findOneByNameAndContext may not return value of type PHPUnit\Framework\MockObject\Builder\InvocationMocker, its return declaration is ": ?FeatureInterface"

The warning shown above is triggered because the createMock() API is used incorrectly. Its return value must be stored in a variable. Then, on this variable, expects() needs to be called:

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

interface FeatureInterface
{
    public function isActive(): bool;
}

interface FeatureRepositoryInterface
{
    public function findOneByNameAndContext(string $name, Context $context): ?FeatureInterface;
}

final class Test extends TestCase
{
    public function testOne(): void
    {
        $featureMock = $this->createMock(FeatureInterface::class);

        $featureMock->expects($this->once())
                    ->method('isActive')
                    ->willReturn(true);

        $x = $this->createMock(FeatureRepositoryInterface::class);

        $x->expects($this->once())
          ->method('findOneByNameAndContext')
          ->willReturn($featureMock);
    }
}
PHPUnit 8.2.5 by Sebastian Bergmann and contributors.

F                                                                   1 / 1 (100%)

Time: 34 ms, Memory: 6.00 MB

There was 1 failure:

1) Test::testOne
Expectation failed for method name is equal to 'isActive' when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

Before you were passing the result of willReturn($expectedIsActive) instead of the result of createMock() to the second willReturn().

davidkmenta commented 5 years ago

@sebastianbergmann you're right! I completely forgot about that behavior. Thank you!