Call to method PHPStan\Mockery\Type\Expects::never() on a separate line has no effect.

[phpstan]

Call to method PHPStan\Mockery\Type\Expects::never() on a separate line has no effect.

[ondrejmirtes]

Yes, just changed it: aa02896

Will tag a new release in a moment. Thank you.

[ondrejmirtes]

One more point I forgot about: Methods that return void are marked as always having side effects :)

[AJenbo]

This also affects regular PHPStan 0.12.9, Mockery 1.3.1 and phpstan-mockery 0.12.3, seen with the following methods:

PHPStan\Mockery\Type\Allows::andReturn()    
PHPStan\Mockery\Type\Allows::andReturns()   
PHPStan\Mockery\Type\Allows::andReturnTrue()
PHPStan\Mockery\Type\Allows::byDefault()    
PHPStan\Mockery\Type\Allows::with()         
PHPStan\Mockery\Type\Expects::andReturn()   
PHPStan\Mockery\Type\Expects::andReturns()  
PHPStan\Mockery\Type\Expects::andThrows()   
PHPStan\Mockery\Type\Expects::never()       
PHPStan\Mockery\Type\Expects::once()        
PHPStan\Mockery\Type\Expects::ordered()     
PHPStan\Mockery\Type\Expects::with()        

@spawnia the issue appears to be that PHPStan thinks never() is a getter that will not have an effect by it self and since you do not do anything with the result it concludes that the call is redundant. But the resturn value is self ($this) since it's a fluid interface, and not a getter. In short, your code is correct, this is a false positive.

[ramsey]

I'm having a similar problem that may be related.

Whenever I use the Mockery alternative shouldReceive() syntax, PHPStan complains with messages like:

Call to method PHPStan\Mockery\Type\Expects::setVariables() on a separate line has no effect.

In this particular case, setVariables() is the method I'm trying to call on the mock object (the Example class in this example), like this:

/** @var Example|MockInterface $build */
$build = Mockery::mock(Example::class);
$build->expects()->setVariables($foo);

If I change to using the shouldReceive() syntax, then PHPStan does not complain:

/** @var Example|MockInterface $build */
$build = Mockery::mock(Example::class);
$build->shouldReceive('setVariables')->once()->with($foo);

My phpstan.neon is very simple:

parameters:
    tmpDir: ./build/cache/phpstan
    level: max
    paths:
        - ./src
        - ./tests

And I have phpstan/extension-installer (version 1.0.4) and phpstan/phpstan-mockery (version 0.12.3) as dependencies.

The README implies that the expects() syntax should work properly with PHPStan:

shouldReceive(), allows() and expects() methods can be called on the mock object and they work as expected.

Do I have something configured incorrectly?

[ondrejmirtes]

What if you try to remove the /** @var line from this example? Does it work?

/** @var Example|MockInterface $build */
$build = Mockery::mock(Example::class);
$build->expects()->setVariables($foo);

[ramsey]

I added the @var line to see if that would help. It does not work with or without that line.

[AJenbo]

@ondrejmirtes would it make sens to not throw this warning if the return type is self/$this?

[ramsey]

It shouldn’t matter what the return type is for these tests. They’re assertions. They shouldn’t be considered to return a value.

[spawnia]

It seems like PHPStan currently assumes that whenever a function has a return type, it should/must be used. That assumption does make sense for pure functions, but produces false positives for cases like this.

Generally speaking, separating side effects from pure computation seems like a good idea and i would not mind PHPStan enforcing it. However, the prevailing paradigm in PHP seems to be that it should do "whatever is the most convenient". That is at least partially driven because of a lack of functional constructs such as Monads.

I think PHPStan should loosen its validation rules here. Whereas using the result of a function(): void is a clear error, not using the result of a function with a return value can be just fine in many cases.

[ondrejmirtes]

@spawnia Thank you for this analysis.

PHPStan currently assumes that whenever a function has a return type, it should/must be used

It's not that simple. There's MethodReflection::hasSideEffects(): TrinaryLogic method. Most implementations return maybe and nothing is done based on that. no means that the result must always be used. For built-in PHP functions, there's a short hardcoded list of functions that don't have any side effects: https://github.com/phpstan/phpstan-src/blob/master/src/Reflection/SignatureMap/functionMetadata.php (we could crowdsource the rest of the list).

PHPStan's method class reflection extensions can say that a method doesn't have side effects. Which is what phpstan-mockery does here. Looks like it doesn't always apply and we should change the logic but we need to look into it more.

hasSideEffects yes means that the method call invalidates the state of the object, so for example any memorized property and method calls are forgotten after that method call. See here.

[spawnia]

@ondrejmirtes what you are saying makes sense.

Off the top of my head, i think that most if not all of the methods upon MockInterface have side effects. Skimming through the documentation, i could not find a call to it that is used to produce a return value (apart from $this).

The methods of the original object might be side effect free, so possibly returning maybe is correct here?

[spawnia]

Thanks for the quick response, updating to 0.12.4 fixed the error.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.