Add then method to ExtendedPromiseInterface

[psafarov]

Fixes return type for then method in ExtendedPromiseInterface

[WyriHaximus]

To be perfectly honest I'm not sure what has to be fixed here since ExtendedPromiseInterface extends from PromiseInterface and that already has the return type declared in the docblock

[psafarov]

@WyriHaximus, the problem is, the return type of PromiseInterface::then is PromiseInterface. Maybe this snippet will be more explanatory:

function test(ExtendedPromiseInterface $promise) {
    $promise2 = $promise->then(...);
    // oops, static type check tells us that `otherwise` method is undefined,
    // as $promise2 is an instance of `PromiseInterface`
    $promise2->otherwise(...); 
}

In this example I assigned result of then to a variable, but same problem occurs when we have a chain of methods:

function test(ExtendedPromiseInterface $promise) {
    // neither `otherwise` nor `done` are recognized during static type check
    $promise->then(...)->otherwise(...)->done(...); 
}

[WyriHaximus]

Ah yes that makes sense, wouldnt't changing the return type to self on PromiseInterface do the same with a less invasive change?

[psafarov]

That was the first thing I did. Unfortunately it didn't fix the problem for PhpStorm, so I went with this solution. It will be understandable if this PR will be rejected, but at least PromiseInterface should be changed for self, in my opinion.

[WyriHaximus]

if this PR will be rejected

In it's current state yes, but I rather focus on finding a solution first and preferably update this PR 😄 . Let me have a look at it as well

[psafarov]

PR is updated. PromiseInterface::then doc follows phpdoc standard now. It doesn't fix incorrect type hinting in PhpStorm though, but I don't see what can be done here.

[clue]

@psafarov Thanks for reporting, that's an interesting issue and I can see where you're coming from! 👍

That being said, while I do agree that the current type definition is inaccurate, I'm not sure this is something we can address in a minor release without a BC break. The updated type definition is technically a BC break that consumers of this package could possibly miss when using custom implementations of said interfaces.

This has been addressed via #75 for the upcoming v3.0 release, so we may as well document this as a known limitation for the current version. What do you think? 👍

[psafarov]

@clue isn't self more open than PromiseInterface in this case? Just not sure what BC we are talking about, but if it is true, then off course it would make perfect sense to delay this PR.

[clue]

@psafarov It's my understanding self will not use late static binding, i.e. it references the exact class name it's defined in. In other words, self and PromiseInterface should be equivalent in this case.

[psafarov]

I see, that means my change doesn't help in any way. And using static means breaking BC. I am closing this PR then. Thanks for explanation.