Skip to content

Return an intersection type when nameMock is used #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 21, 2020

Conversation

martinssipenko
Copy link
Contributor

@martinssipenko martinssipenko commented Jul 3, 2020

This will return intersetion type when namedMock function is used.

Currently when Mockery::namedMock() is used within tests PHPStan complains that the mocked object does not conform to what is expected.

interface Bar {}

class Foo
{
    public function foo(Bar $bar) {
        $this->bar = $bar;
    }
}

// Works
$mock = Mockery::mock(Bar::class);
(new Foo)->foo($mock);

// Does not work
// Parameter #1 $bar of method Foo::foo() expects Bar, Mockery\MockInterface given. 
$namedMock = Mockery::namedMock('Buzz', Bar::class);
(new Foo)->foo($namedMock);

@ondrejmirtes
Copy link
Member

Hi, this isn't sufficient as the extension will try to make a type from the first argument too.

@martinssipenko
Copy link
Contributor Author

martinssipenko commented Jul 3, 2020

Hi, this isn't sufficient as the extension will try to make a type from the first argument too.

Hm, I understand first argument also represents a type, essantually namedMock('Foo', 'Bar') is same as calss Foo implements Bar.

@ondrejmirtes
Copy link
Member

From the documentation I don't understand what the method does: http://docs.mockery.io/en/latest/cookbook/class_constants.html

There are only classes involved, and in PHP you can't create intersection types from classes.

@martinssipenko
Copy link
Contributor Author

When you mock something with Mockery::mock(SomeInterface::class) mockery creates a "fake" class that implements SomeInterface and a mock object from this "fake" class is created.

Now suppose you need to create two mock objects of same interface.

$mock1 = Mockery::mock(SomeInterface::class);
$mock2 = Mockery::mock(SomeInterface::class);

If you do get_class($mock1) and get_class($mock2) you will get the same result because the same "fake" class is used.

Using namedMock method you can get specify the name of the "fake" calss mockery creates, thus it allows to have two distinct objects that are created from different classes, but implement same interface.

@ondrejmirtes
Copy link
Member

Yeah but if class name in the first argument isn't an existing one, we have to skip it when creating the intersection...

@martinssipenko
Copy link
Contributor Author

Can you help me understand why? If we use mock method, then class/interface name (1st arg) can also be non-existant.

How do you think we can fix this?

@ondrejmirtes
Copy link
Member

 then class/interface name (1st arg) can also be non-existant.

I don't think that's true.

@martinssipenko
Copy link
Contributor Author

<?php
// x.php

require_once __DIR__ . '/vendor/autoload.php';

$mock1 = Mockery::mock('WhaterverRandomNameIComeUpWith');
$mock2 = Mockery::mock('WhaterverRandomNameIComeUpWith');

var_dump(get_class($mock1));
var_dump(get_class($mock2));
php x.php 
/Users/msipenko/src/msipenko/forks/phpstan-mockery/x.php:8:
string(41) "Mockery_0__WhaterverRandomNameIComeUpWith"
/Users/msipenko/src/msipenko/forks/phpstan-mockery/x.php:9:
string(41) "Mockery_0__WhaterverRandomNameIComeUpWith"

@ondrejmirtes
Copy link
Member

Yeah but PHPStan won't like it: https://phpstan.org/r/31d8e42c-5d2c-420a-b7d0-8161f271e686

Is it a valid use-case?

@martinssipenko
Copy link
Contributor Author

Ah ok, I understand now, strangely I edited the code locally in my projects vendor directory, and just adding that entry to array made PHPStan error go away.

Either way, what do you suggest we can do here?

@ondrejmirtes
Copy link
Member

There has to be another extension similar to MockDynamicReturnTypeExtension but that skips the first parameter.

@martinssipenko
Copy link
Contributor Author

martinssipenko commented Jul 3, 2020

Meybe we can just add something like this?

		$args = $methodCall->args;
		if ($methodReflection->getName() === 'namedMock') {
			array_shift($args);
		}

@ondrejmirtes
Copy link
Member

Nope, it'd be a mess, this needs a separate class.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some code added to https://github.com/phpstan/phpstan-mockery/blob/master/tests/Mockery/MockeryTest.php to verify by running vendor/bin/phing phpstan this actually works would be nice.

$defaultReturnType = new ObjectType('Mockery\\MockInterface');

$args = $methodCall->args;
array_shift($args);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to make sure the array has at least two elements.

@martinssipenko
Copy link
Contributor Author

@ondrejmirtes, any updates on this?

@ondrejmirtes ondrejmirtes merged commit a316610 into phpstan:master Jul 21, 2020
@ondrejmirtes
Copy link
Member

Merged and released, thanks.

@martinssipenko martinssipenko deleted the namedMock branch July 21, 2020 09:47
@martinssipenko
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants