Negotiator::getBest() return type hint?

[mindplay-dk]

To get offline source inspection (phan, Php Storm, etc.) currently I have to manually type-hint the return-type of getBest() inline - for example:

            $negotiator = new Negotiator();

            /** @var Accept $result */
            $result = $negotiator->getBest(
                $request->getHeaderLine("Accept"),
                ["application/json", "text/html"]
            );

            if ($result->getValue() === "application/json") {
                // ...
            }

Without the @var annotation, the if-statement will fail inspection, because the return-type of getBest() was declared as AcceptHeader rather than Accept, which appears to be the actual return-type.

What's the purpose of the empty AcceptHeader interface?

[willemstuursma]

Maybe just change the @return hint in \Negotiation\AbstractNegotiator::getBest to AcceptHeader|BaseAccept?

[teohhanhui]

What's the purpose of the empty AcceptHeader interface?

Just came across this with phpstan as well. This is puzzling.

[g105b]

I'm pasting the example from the readme into my editor.

Here's how 3.0.0 looks in PhpStorm:

[willdurand]

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.

[g105b]

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.

[nreynis]

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.

[josefsabl]

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?