Skip to content
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

Suggestion: Dedicated phpunit assertion for instanceof in assertTrue #4845

Closed
Riimu opened this issue Feb 26, 2020 · 1 comment · Fixed by #6294
Closed

Suggestion: Dedicated phpunit assertion for instanceof in assertTrue #4845

Riimu opened this issue Feb 26, 2020 · 1 comment · Fixed by #6294

Comments

@Riimu
Copy link

Riimu commented Feb 26, 2020

PHP-CS-Fixer already has plenty of good fixers to use dedicated assertions in PHPUnit when applicable. However, one glaring omission seems to be the instanceof operator.

It would be nice if the php_unit_dedicate_assert would also look at the special case of using:

$this->assertTrue($instance instanceof SomeClass);

And fix it to use the dedicated assertion instead:

$this->assertInstanceOf(SomeClass::class, $instance);

I realize this is not necessarily as straightforward as the others, as it's not about using specific function, but a language construct. Additionally, there is the question of whether using SomeClass::class or "SomeClass" or 'SomeClass', but that could possibly be handled by other fixers. It might also make sense to create a separate fixer for this rather than including it in the php_unit_dedicate_assert.

Scouring through one legacy code base I was just surprised to find plenty of these tests, so it would nice to have automated way of correcting them.

@kubawerlos
Copy link
Contributor

Also these:

    $this->assertTrue(isset($foo['bar']));
    $this->assertTrue(isset($foo->bar]));

can be changed to:

    $this->assertArrayHasKey('bar', $foo);
    $this->assertClassHasAttribute('bar', $foo);

and similar for assertions assertClassHasStaticAttribute and assertObjectHasAttribute.

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

Successfully merging a pull request may close this issue.

3 participants