Add support for PHPUnit 12.0.9

[VincentLanglet]

See sebastianbergmann/phpunit#6156 (comment)

In 12.0.9 PHPUnit removed the InvocationMocker classes, the PR is sebastianbergmann/phpunit@fb4df53

Current references are

• phpstan-phpunit/stubs/InvocationMocker.stub

  Line 10 in 0f857bf

  class InvocationMocker

• phpstan-phpunit/src/Type/PHPUnit/InvocationMockerDynamicReturnTypeExtension.php

  Line 12 in 0f857bf

  class InvocationMockerDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension

• phpstan-phpunit/extension.neon

  Line 15 in 0f857bf

  - stubs/InvocationMocker.stub

• phpstan-phpunit/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php

  Line 12 in 0f857bf

  use PHPUnit\Framework\MockObject\Builder\InvocationMocker;

• phpstan-phpunit/src/Rules/PHPUnit/MockMethodCallRule.php

  Line 10 in 0f857bf

  use PHPUnit\Framework\MockObject\Builder\InvocationMocker;

I feel like PHPUnit\Framework\MockObject\Builder\InvocationMocker was basically replaced by PHPUnit\Framework\MockObject\InvocationStubberImplementation

Currently don't know if it will require a StubFilesExtensionLoader like this one
https://github.com/phpstan/phpstan-doctrine/blob/2.0.x/src/Stubs/Doctrine/StubFilesExtensionLoader.php
or if a there is a better strategy.

One first step would be to test this lib with phpunit 10/11 (and 12) but it's currently forbidden by the hardcoded php config

phpstan-phpunit/composer.json

Lines 19 to 24 in 0f857bf

	"phpunit/phpunit": "^9.6"
	},
	"config": {
	"platform": {
	"php": "7.4.6"
	},

[sebastianbergmann]

I also remember talking to Ondrej and/or Markus last year about whether the stubs in phpstan-phpunit are still needed. Ideally, this part of this extension can just be removed.

[VincentLanglet]

I also remember talking to Ondrej and/or Markus last year about whether the stubs in phpstan-phpunit are still needed. Ideally, this part of this extension can just be removed.

I assume that some of this code might be conditionally removed for recent version of phpunit but not all of them.

For instance the MockMethodCallRule is made to report code like

$this->createMock(Bar::class)->method('nonExistingMethod');

Those file were introduced in #72,

• MockMethodCallRule for the rule
• InvocationMocker.stub in order to add @template TMockedClass in order to keep trace of the original class mocked
• MockObjectDynamicReturnTypeExtension to tell phpstan that MockObject&Foo::expects will returns InvocationMocker<Foo>.
• InvocationMockerDynamicReturnTypeExtension to tell phpstan that InvocationMocker methods are returning the same object with the same generic. => Maybe not needed anymore since most of method are annotated with @return $this ?

[sebastianbergmann]

Maybe not needed anymore since most of method are annotated with @return $this?

I just found one more method (see sebastianbergmann/phpunit@4b7f2a8) that is part of the public API for configuring test doubles which was missing @return $this. So hopefully with the next release, PHPUnit 12.0.10, all relevant methods will have this.

[SamMousa]

I can confirm that without the extension PHPStan works fine for the mocks.

[OskarStark]

So is there still a need for the extension then?

[SamMousa]

I imagine it depends on which features of phpunit you use.
Simple way to test is to just remove it and rerun phpstan.

[VincentLanglet]

So is there still a need for the extension then?

The extension is not just about mock stubs but also

• some type refinement, cf readme:

Specifies types of expressions passed to various assert methods like assertInstanceOf, assertTrue, assertInternalType etc.
Combined with PHPStan's level 4, it points out always-true and always-false asserts like assertTrue(true)

• about the extra rules added, cf readme:

Check that you are not using assertSame() with true as expected value. assertTrue() should be used instead.
Check that you are not using assertSame() with false as expected value. assertFalse() should be used instead.
Check that you are not using assertSame() with null as expected value. assertNull() should be used instead.
Check that you are not using assertSame() with count($variable) as second parameter. assertCount($variable) should be used instead.
Check that you are not using assertEquals() with same types (assertSame() should be used)
Check that you are not using assertNotEquals() with same types (assertNotSame() should be used)

• the MockMethodCallRule, cf Add support for PHPUnit 12.0.9 #222 (comment)
• and certainly already more (or more in the future)

I'm okay trying to work on this issue in the next days/week(s). I would say that the first step would be to run the CI with phpunit 10, 11 and 12. But currently it's not possible because of config.platform.php: "7.4.6":

phpstan-phpunit/composer.json

Lines 19 to 24 in 0f857bf

	"phpunit/phpunit": "^9.6"
	},
	"config": {
	"platform": {
	"php": "7.4.6"
	},

@ondrejmirtes any advice about how you want to solve this ? Should I override the confirm.platform.php in the CI and try installing more recent phpunit versions ? Or should I remove the config.platform.php from composer (?).

[ondrejmirtes]

I added testing with multiple PHPUnit versions: #223

I started testing v12: #224

But I feel like not enough tests are failing.

@VincentLanglet The biggest help for me would if you contributed some new tests in a separate PR that simply pass with PHPUnit v9-v11, but fail with v12. That way I can verify if the fixes I actually plan to do are valid.

[ondrejmirtes]

So ideally you need to look at the places that currently mention InvocationMocker, and figure out what breaks:

1. When InvocationMocker class is removed.
2. Or when the stub or extension gets removed.

[VincentLanglet]

I started testing v12: #224

But I feel like not enough tests are failing.

@VincentLanglet The biggest help for me would if you contributed some new tests in a separate PR that simply pass with PHPUnit v9-v11, but fail with v12.

The current failure

1) PHPStan\Rules\PHPUnit\MockMethodCallRuleTest::testRule
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 '15: Trying to mock an undefined method doBadThing() on class MockMethodCall\Bar.
-20: Trying to mock an undefined method doBadThing() on class MockMethodCall\Bar.
+[36](https://github.com/phpstan/phpstan-phpunit/actions/runs/14019937135/job/39250517251?pr=224#step:7:37): Trying to mock an undefined method doBadThing() on class MockMethodCall\Bar.
 '

phar:///home/runner/work/phpstan-phpunit/phpstan-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Testing/RuleTestCase.php:104
/home/runner/work/phpstan-phpunit/phpstan-phpunit/tests/Rules/PHPUnit/MockMethodCallRuleTest.php:[40](https://github.com/phpstan/phpstan-phpunit/actions/runs/14019937135/job/39250517251?pr=224#step:7:41)

from your PR @ondrejmirtes is legit because the MockMethodCallRule was implemented around the InvocationMocker and need now to be adapted for the new PHPUnit implementation.

This was the "only" test failing because it's the only code which rely on InvocationMocker.
Indeed occurrences are

• MockObjectDynamicReturnTypeExtension
• InvocationMockerDynamicReturnTypeExtension
• MockMethodCallRule

and all three of them were introduced for the MockMethodCallRule (as explained in #222 (comment))

I'm not sure there is a lot more tests to add then, but still I added #225 as a non-regression test for this issue.

That way I can verify if the fixes I actually plan to do are valid.

Do you want to first try your solution or should I try a fix for the PHPUnit 12 build ?

[ondrejmirtes]

I want to look into it myself. Thanks!