Phpinspectionsea: Asserting instanceof should not result in "argument implicitly declares return type" if a method has a return type

[kobelobster]

| Subject | Details |
| :------------- | :---------------------------------------------------------------------------- |
| Issue type | False-Positive (?) |
| Plugin | Php Inspections (EA Extended) 3.0.6.1 |
| Language level | 7.2 |

Current behaviour (description/screenshot:)

Let's say you have the following method

public function getStdClass(): \stdClass
{
    return new \stdClass();
}

and write a test case like this

public function getStdClass(): void
{
    $this->assertInstanceOf(\stdClass::class, $this->certificationFactory->getStdClass());
}

A warning is thrown argument implicitly declares return type

Expected behaviour

In my opinion this warning shouldn't exist in the first place, because if I change the return type of the method, the test should fail. So it should be okay to have this assertion, shouldn't it?

[kalessil]

If the method would return an interface and assertion tested certain implementation class, this would be ok.

But this specific case smells or probably indicates recent migration to return types. Let other tests cover this: if the return type changed, the business logic somewhere will break ;)

[kobelobster]

But this specific case smells or probably indicates recent migration to return types

Do you mean because every class should have an interface? I mean, while it's probably good practice I wouldn't declare that as a code smell. Not following SOLID should not be a code smell IMO.

if the return type changed, the business logic somewhere will break ;)

It will, but I'm not quite sure if it should only be noticed through other methods.

Just my two cents.

[kalessil]

Not following SOLID should not be a code smell IMO.
Fair enough =)

Just my two cents.
I think the best solution will be to introduce a new setting for this inspection, so you can disable this specific behaviour in specific projects.

[kobelobster]

Sounds good :+1:

[kalessil]

Good news, it's already a separate inspection (PhpUnit: unnecessary assertion), which you can disable.
Hence, closing the issue.

[kobelobster]

@kalessil Thanks! Does this only deactivate the one I talked about or even more? Because unnecessary assertions sounds like it can make sense in specific cases, so I don't know if it's only related to the return type?

[kalessil]

@tzfrs, the inspection does only this check. So: yes.