Open mindplay-dk opened 7 years ago
Maybe just change the @return
hint in \Negotiation\AbstractNegotiator::getBest
to AcceptHeader|BaseAccept
?
What's the purpose of the empty AcceptHeader interface?
Just came across this with phpstan
as well. This is puzzling.
I'm pasting the example from the readme into my editor.
Here's how 3.0.0 looks in PhpStorm:
I am not quite sure what you're talking about to be honest (phpstan wasn't a thing when I was writing PHP..) but I'd be happy to see these issues fixed.
This issue occurs because of the following:
Negotiator
extends AbstractNegotiator
AbstractNegotiator::getBest()
is documented to return AcceptHeader|null
AcceptHeader
is a completely empty interface, so IDEs or static analysis don't understand that the type returned by getBest()
should have the getValue()
method.I think this can be solved by refactoring the inheritance of the project. I'll take a look into it when I've got a bit of time.
Edit: in fact, this can be fixed really easily by providing a more accurate return type in the doc blocks, but I think the project would benefit from some simple OO refactoring.
An easy fix would be to just add every public method of BaseAccept
into the AcceptHeader
interface.
It would solve the issue without introducing any BC.
I am not quite sure what you're talking about to be honest (phpstan wasn't a thing when I was writing PHP..) but I'd be happy to see these issues fixed.
The AbstractNegotiator::getBest method returns AcceptHeader interface, that doesn't prescribe any methods so you (as a user of the library) don't know what to call next and you have to rely on looking into implementation details or documentation to know that you can call getType on that.
I.e. this isn't about static analysis or phpstan but rather the flawed interface of the library and it hurts its usage even when no static analysis tools are involved. I indeed was puzzled when using the lib on Friday and was like: AcceptHeader interface and now what.
You should pull the methods shapes to the interface or maybe type-hint the getBest method to be returning a BaseAccept class instead of the interface.
What is the interface for anyway?
To get offline source inspection (phan, Php Storm, etc.) currently I have to manually type-hint the return-type of
getBest()
inline - for example:Without the
@var
annotation, theif
-statement will fail inspection, because the return-type ofgetBest()
was declared asAcceptHeader
rather thanAccept
, which appears to be the actual return-type.What's the purpose of the empty
AcceptHeader
interface?