phpstan-wordpress icon indicating copy to clipboard operation
phpstan-wordpress copied to clipboard

Treat assertNotWPError() and assertWPError() as type-narrowing functions

Open johnbillion opened this issue 4 years ago • 12 comments

In the WordPress core test suite the following assertion methods are available:

  • $this->assertNotWPError()
  • $this->assertWPError()

It would be great if this extension treated these methods as type-narrowing functions, so the following code would not throw an error in PHPStan:

$terms = wp_get_post_terms( $post_id, $taxonomy );
$this->assertNotWPError( $terms );
$this->assertCount( 0, $terms );

Currently this triggers the following error because $terms is assumed to be array|WP_Error on the last line.

[phpstan] Parameter #2 $haystack of method PHPUnit\Framework\Assert::assertCount() expects Countable|iterable, array|WP_Error given.

johnbillion avatar Dec 11 '20 14:12 johnbillion

All right. So you ask for parameter in WP_UnitTestCase_Base::assertNotWPError() never be of WP_Error and assertWPError() should do the opposite?

szepeviktor avatar Dec 11 '20 14:12 szepeviktor

Could you try installing phpstan/phpstan-phpunit? https://github.com/phpstan/phpstan-phpunit#readme

szepeviktor avatar Dec 11 '20 15:12 szepeviktor

Yes the idea is that any parameter passed to assertNotWPError() cannot be a WP_Error in subsequent lines.

https://phpstan.org/writing-php-code/narrowing-types

I tried phpstan-phpunit but it had no effect, but I didn't play with it much.

johnbillion avatar Dec 11 '20 15:12 johnbillion

I hope we can tell PHPStan that assertNotWPError is really $this->assertNotInstanceOf( 'WP_Error', $actual, $message ); from https://github.com/phpstan/phpstan-phpunit/blob/master/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php

@ondrejmirtes Could you give us directions?

szepeviktor avatar Dec 11 '20 16:12 szepeviktor

@johnbillion Could you point me to a simple WordPress plugin that has a unit test with assertNotWPError ?

szepeviktor avatar Dec 11 '20 16:12 szepeviktor

Yes, you need to write and register a similar extension that phpstan-phpunit has: https://github.com/phpstan/phpstan-phpunit/blob/master/src/Type/PHPUnit/Assert/AssertMethodTypeSpecifyingExtension.php

ondrejmirtes avatar Dec 11 '20 16:12 ondrejmirtes

Some info about them also here: https://phpstan.org/developing-extensions/type-specifying-extensions

ondrejmirtes avatar Dec 11 '20 16:12 ondrejmirtes

Thank you. Now we need those hands writing code.

szepeviktor avatar Dec 11 '20 17:12 szepeviktor

@monojp Please advise. What would you do in this case?

szepeviktor avatar Mar 01 '21 18:03 szepeviktor

I'm not an expert and never used those WP test helpers. But it looks they are at least not part of the official WP release. Are they WP internal? Does WP advise to use them for plugin developers and if the do - how? I quickly searched for it but did not find out much UPDATE: I just realised that @johnbillion wants to use this in core :) Well I think the extension could be just added then.. I can take a look later today or tomorrow. If you can provide an example / link to a test case or so that would be perfect.

herndlm avatar Mar 02 '21 08:03 herndlm

@mono Here are your suspects https://github.com/WordPress/wordpress-develop/blob/7d0cb86544dd28df8eaa0cb21f271e99e7e7cc47/tests/phpunit/includes/abstract-testcase.php#L598-L619 part of the WordPress Test Suite.

The problem is these method narrow down types.

szepeviktor avatar Mar 02 '21 09:03 szepeviktor

@johnbillion are you still interested in this? If you could just provide me with an easy to work on example I can implement this next Wednesday, should not be hard. I just don't want to spend ages setting up the WP testing framework that I don't know anything about yet

herndlm avatar Jul 30 '21 17:07 herndlm