Remove non-sound "__invoke can be type hinted as callable" reasoning

[schnittstabil]

This is not about pro/contra __invoke.

The issue here is: the reasoning is not sound – @shadowhand if you believe the reasoning contains anything valid, then you have to clarify/rewrite that paragraph, the current version seems to be just an (unproven) opinion.

Explanation

If we apply the same reasoning to the proposed interfaces, then we see that reasoning itself is unsound:

Why doesn't middleware use process?

… In addition, classes that implement MiddlewareInterface can be type hinted as mixed, which results in less strict typing. This is generally undesirable, especially when the process method uses strict typing.

The first sentence is still valid, whereas the second sentence is clearly unrelated to the first one – same applies to the current version. Or in other words, the current version is just a fallacious argument.

[shadowhand]

There's nothing incorrect about the argument. Objects that have __invoke can be type hinted as callable.

[schnittstabil]

And classes that implement MiddlewareInterface can be type hinted as mixed. Both result in less strict typing – Implementers can type hint callable/mixed, thus there is no argument at all here.

[shadowhand]

We're not talking about docblock type hinting, we are talking about language type hinting:

// bad
public function process(ServerRequestInterface $request, callable $delegate);

// good
public function process(ServerRequestInterface $request, DelegateInterface $delegate);

[schnittstabil]

I'm aware of that, but your bad example is not a valid MiddlwareInterface instance at all.

[schnittstabil]

For example, at Equip\Dispatch\MiddlewarePipe:

class MiddlewarePipe implements ServerMiddlewareInterface
{
    /**
     * Add a middleware to the end of the stack.
     */
    public function append(ServerMiddlewareInterface $middleware) {…}

Implementers, can type-hint append that way:

class MiddlewarePipe implements ServerMiddlewareInterface
{
    /**
     * Add a middleware to the end of the stack.
     */
    public function append($middleware) {…}

The two sentences have nothing to do with the question:

Why doesn't middleware use __invoke?

[atanvarno69]

@schnittstabil I see the point you are driving at and would agree, were the paragraph considered in isolation. However, I think (and this is just my opinion, no guarantee that I am correct) the paragraph should be read as a callback to the points made towards the end of section 4.4 Chosen Approach.

It is contrasting the PSR MiddlewareInterface with the common double pass standard. i.e. where a typical double pass middleware is acceptable input, the receiving function/method will type hint callable; whereas a function/method that wants a PSR middleware has to type hint MiddlewareInterface.

Whether this point should be under 'Why doesn't middleware use __invoke?' to me is a fair question, since that does not seem to be what it is driving at. Unless it is trying to protect middleware consumers from type hinting callable (in a world where the interface names its method __invoke) and accidentally accepting any callable. If so, it should be clearer about that (and also -- this is a tangent that has no bearing on the subject at hand -- is it the responsibility of the spec's design to insure against consumers' incompetence? If it's a side effect of a design decision, then I'm fine with it, but it should never be the main driver for a design decision).

Briefly, I think it is trying to say something along the lines of "If you want middleware, you type hint MiddlewareInterface, not callable which you might be tempted to do if the method was __invoke and if you had some fancy plan involving accepting middleware and other callables for some weird reason."

So I think wording could be improved, since it does not clearly and robustly present its point, but I don't think it should be deleted entirely. Also, I (as noted) may be completely wrong in my understanding of the paragraph.

[schnittstabil]

@atanvarno69 Sure, someone may improve the wording. Unfortunately, @shadowhand erased the git history by copying the proposal into the REAMDE.md; and middleware-meta.md does not contain them – hence I cannot comprehend how these two sentences evolved and I can not guess, what these two sentences supposed to prove.

Therefore, I can only suggest to remove them. In the current version they only appear to be good arguments for non-__invoke, but sadly they are just a logical fallacy.

[schnittstabil]

@shadowhand Let me get this right. The current sentences contains a demonstrative (this):

In addition, classes that define __invoke can be type hinted as callable, which results in less strict typing. This is generally undesirable,….

The this have to refer to one single(!) entity (statement, sentence etc.), but to which one?

1. _classes that define __invoke can be type hinted as callable_, which generally undesirable
2. less strict typing is generally undesirable
3. ??? is generally undesirable

[shadowhand]

It means "less strict typing" is generally undesirable.

[schnittstabil]

It means "less strict typing" is generally undesirable.

That is obviously a Faulty Generalization.

For example, the PHP type-system is very limited and does not support overloading, i.e. having two append methods is not possible:

use Zend\Stratigility\MiddlewareInterface as LegacyMiddlewareInterface;
use Interop\Http\Middleware\MiddlewareInterface;

class MiddlewarePipe
{
    /**
     * @var array
     */
    private $middleware = [];

    /**
     * @param LegacyMiddlewareInterface $middleware
     */
    public function append(LegacyMiddlewareInterface $middleware)
    {
        array_push($this->middleware, $middleware);
    }

    /**
     * @param MiddlewareInterface $middleware
     */
    public function append(MiddlewareInterface $middleware)
    {
        array_push($this->middleware, $middleware);
    }
}

The (only good?) way to deal with that is less strict typing:

use Zend\Stratigility\MiddlewareInterface as LegacyMiddlewareInterface;
use Interop\Http\Middleware\MiddlewareInterface;

class MiddlewarePipe
{
    /**
     * @var array
     */
    private $middleware = [];

    /**
     * @param MiddlewareInterface|LegacyMiddlewareInterface $middleware
     */
    public function append($middleware)
    {
        …
    }
}

[shadowhand]

The other (better) way to handle it is to have two separate methods that are each strongly typed.

[jschreuder]

Going slightly off-topic, but this is what the adapter pattern is for. To quote the Wikipedia article:

It is often used to make existing classes work with others without modifying their source code.

Have 1 append method and either wrap the legacy in the new interface or wrap both in a single interface. Still strictly typed, a consistent flow through your application without weird if-else garbage to support 2 interfaces.

[schnittstabil]

Going slightly off-topic, but this is what the adapter pattern is for.

This is exactly my point! With adapter pattern:

use Zend\Stratigility\MiddlewareInterface as LegacyMiddlewareInterface;
use Interop\Http\Middleware\MiddlewareInterface;

class MiddlewarePipe
{
    /**
     * @var array
     */
    private $middleware = [];

    /**
     * @param MiddlewareInterface|LegacyMiddlewareInterface $middleware
     */
    public function append($middleware)
    {
        if ($middleware instanceof MiddlewareInterface) {
            array_push($this->middleware, $middleware);
        } else if ($middleware instanceof LegacyMiddlewareInterface) {
            array_push($this->middleware, new LegacyAdapter($middleware));
        } else {
            throw new LogicException("unsupported middleware");
        }
    }
}

Still strictly typed.

No, as we can see, the append is not necessarily strictly typed anymore.

---

@jschreuder If you've had this in mind:

 class MiddlewarePipe
 {
     …
     /**
-     * @param LegacyMiddlewareInterface $middleware
+     * @param MiddlewareInterface $middleware
      */
-    public function append(LegacyMiddlewareInterface $middleware)
+    public function append(MiddlewareInterface $middleware)
     {
         array_push($this->middleware, $middleware);
     }
}

This would be a BC and is often not desired; and may just move the less-strict typing to another place.

[schnittstabil]

It means "less strict typing" is generally undesirable.

As that statement includes the word generally, we do not need to focus on the middleware domain, any (commonly used) counter-example will prove that it is wrong, e.g. Psr\SimpleCache\CacheInterface:

interface CacheInterface
{
    /**
     * @param string                $key   The key of the item to store.
     * @param mixed                 $value The value of the item to store, must be serializable.
     * @param null|int|DateInterval $ttl   Optional. The TTL value of this item.
     * …
     */
    public function set($key, $value, $ttl = null);
}

This also shows that, separate methods are not desired in general.

[shadowhand]

Using CacheInterface to support your argument doesn't really work, since $ttl is:

1. Optional, thereby can be nullable
2. Could be a timestamp or a datetime, both of which represent the same thing

I would say this is the exception that proves the rule and it does not help your argument.

[jschreuder]

@schnittstabil wrote

This is exactly my point! With adapter pattern: ...[snip, some bad code]...

That's really not an adapter pattern. That's exactly what I mentioned was the problem which the adapter pattern solves. You might want to read up on it with the links I provided.

[schnittstabil]

That's really not an adapter pattern.

@jschreuder Well, I've used the adapter pattern above:

array_push($this->middleware, $middleware);
…
array_push($this->middleware, new LegacyAdapter($middleware));

How do you think I should use the pattern instead? – Without introducing a BC of course.

[schnittstabil]

@shadowhand Of course, it supports my argument: It proves that "less strict typing" is not generally undesirable.

Could be a timestamp or a datetime, both of which represent the same thing

That obviously does not matter. We are talking about type-hints, therefore about syntax, not semantic.

Moreover, the CacheInterface uses @param mixed $value too. If you believe "less strict typing" is generally undesirable, then we should prefer a ValueInterface version.

[jschreuder]

@schnittstabil if you do the wrapping of the legacy middleware inside the MiddlewarePipe you're doing it wrong. The pipe should just take & thus type-hint one interface, the new one. When you want to pass in a legacy interface you wrap it inside the LegacyAdapter before you pass it into the Pipe. The Pipe should have no knowledge of that, or even that it is dealing with legacy classes. That is abstracted away.

[schnittstabil]

@jschreuder While I agree that this is possible and usually the right way, it really depends on your concrete project and use-cases.

For example:

use Middleware\Composer\Package1\Foo;
use Middleware\Composer\Package2\Bar;

$pipe->append(new Foo());
$pipe->append(new LegacyAdapter(new Bar()));

I.e. if we migrate Bar to the new interface, we have to change the setup code everywhere – this is usually not desired.

[jschreuder]

Changing setup code is cheap, it's easily found in any project that practices dependency injection and even those that don't. The changes are also not error prone.

Managing the complexity arising from having non-strict type hints and dealing with different interfaces is far more difficult and error prone. It also makes changes to the project far more difficult as you can never be entirely sure what type object you're working with.

[schnittstabil]

@jschreuder While that sounds reasonable, sometimes (e.g. within frameworks) we want to provide a migration path which is as softly as possible. Hence we may prefer a "less strict typing" version to avoid losing customers (maybe this is only during one major release).

Another argument: PHP is a weak-typed (duck-typed) script language, which also allows type-hints (yes, these are only hints!). Thus, concluding that "less strict typing" is generally undesirable, is an unfounded faulty generalization in my opinion.

[schnittstabil]

Anyway, lets focus on the current version again:

Why doesn't middleware use __invoke?

In addition, classes that define __invoke can be type hinted as callable, which results in less strict typing. This is generally undesirable, especially when the __invoke method uses strict typing.

Which boils down to the following logical reasoning:

1. MiddlewareInterface::__invoke results in less strict typing
2. less strict typing is generally bad
3. (implicit consequence using 1. and 2.) MiddlewareInterface::__invoke is bad
4. (implicit consequence using 3.) we do not propose MiddlewareInterface::__invoke

Obviously, 3. is a Fallacy:

A fallacy is the use of invalid or otherwise faulty reasoning. An argument that is formally fallacious is rendered invalid due to a flaw in its logical structure. Such an argument is always considered to be wrong.

(Btw, 1. is also a fallacy in this context.)

[jschreuder]

Another argument: PHP is a weak-typed (duck-typed) script language, which also allows type-hints (yes, these are only hints!).

There's 2 misconceptions in that sentence:

1. While PHP is a weak typed language, that is something very different from it being "duck-typed". It allows for programming with duck-typing at its core, but allowing for it doesn't make the language itself duck-typed.
2. Type hints are and have always been a bad label. They're checked in run-time and will throw errors (PHP <7) or Throwable Errors (PHP >=7). Even scalar types in PHP7 without declare(strict_types = 1) aren't hints as they will coerce the type and still may cause warnings. As such they're really very much not "only hints".

[shadowhand]

@schnittstabil you haven't, by any stretch, proven that the following statement is a fallacy:

MiddlewareInterface::__invoke results in less strict typing

In fact, I offer a simple proof that proves __invoke() (and thereby callable) is less strict than a class type hint: https://3v4l.org/4Ncgj

[schnittstabil]

@jschreuder 1. was obvious to me – thanks for clarifying. About 2.: As they cause (catchable) run-time errors/exceptions they are not bad labeled.

No matter how you look at it, PHP is weak-typed and neither 1. nor 2. refutes my conclusion:

Thus, concluding that "less strict typing" is generally undesirable, is an unfounded faulty generalization in my opinion.

[schnittstabil]

@schnittstabil you haven't, by any stretch, proven that the following statement is a fallacy:

[Proposition] MiddlewareInterface::__invoke results in less strict typing.

1st EDIT: Well, that is true, I haven't yet. Stupid me, see my very first https://github.com/http-interop/http-middleware/pull/55#issue-199772936. The current reasoning, in a more explicit version:

1. MiddlewareInterface::__invoke implies middlewares can be type hinted as callable
2. which (necessarily!) results in less strict typing.

The first list item is obviously true. But even my 3 year old niece knows, can and will have distinct meanings. For example:

• Barack Obama can launch atomic bombs, but that does not necessarily imply he will!

I've shown that the current reasoning have to be a fallacy by applying it to the current proposed version at https://github.com/http-interop/http-middleware/pull/55#issue-199772936, https://github.com/http-interop/http-middleware/pull/55#issuecomment-271608974 and more explicit at https://github.com/http-interop/http-middleware/pull/55#issuecomment-271610631, which is usually and commonly an accepted technique. Well, It seems I've made the erroneous assumption of some knowledge about logical reasoning, like it is taught in undergraduate courses for computer science (e.g. linear algebra and calculus courses).

– I will try to be more explicit, if it comes again to debates about logical reasoning :pensive:

---

2nd EDIT: Independent from above, https://github.com/http-interop/http-middleware/pull/55#issuecomment-272623201 still shows:

• _MiddlewareInterface::__invoke is bad_ - is a logical fallacy; the claim is not supported by previous sentences.

---

In fact, I offer a simple proof that proves __invoke() (and thereby callable) is less strict than a class type hint: https://3v4l.org/4Ncgj

Unfortunately, that does not prove the [Proposition] you wanted to prove. – I would suggest to postpone that discussion, because of https://github.com/http-interop/http-middleware/pull/55#issuecomment-272623201.

[jschreuder]

1. was obvious to me – thanks for clarifying. About 2.: As they cause (catchable) run-time errors/exceptions they are not bad labeled.

Uhh... what? Because the whole thing doesn't segfault you consider it a hint? The entire function or method won't be executed because of that exception and you have to catch it or end up with a hard error anyway. Being able to catch errors (they're TypeErrors) doesn't make them "hints", it allows you to decently handle them.

Also: while PHP is weakly typed, it does allow for gradual typing. As such any type discussion is valid and opinions may differ. Which is obviously the case here.

No matter how you look at it, PHP is weak-typed and neither 1. nor 2. refutes my conclusion:

Actually. If you re-read your post, those 2 points were the only basis you gave for the conclusion. You've made other points in other posts, but those have been answered (in my opinion) by @shadowhand already.

[schnittstabil]

Uhh... what? Because the whole thing doesn't segfault you consider it a hint?

Aww, sorry, you are right. I've oversimplified it and haven't presented any valid arguments why the name "type-hint" makes sense.

---

The concrete (remaining) argument is:

PHP allows weak-typing, hence we cannot conclude that "less strict typing" is generally undesirable

– Simply, because the conclusion is not a well known fact, it is an opinion; and many developers disagree, not only me (e.g. Type Hinting in PHP - Good or Bad Practice?).

Therefore, if we do not want __invoke for type-safety reasons, then we have to provide a valid non-opinionated argument in the README.md.

[shadowhand]

Again, feel free to make these arguments during the PSR-15 review phase. I've already spent far too much time and energy trying to debate this with you.

[schnittstabil]

For what is worth, I've update my https://github.com/http-interop/http-middleware/pull/55#issuecomment-272632485.

[schnittstabil]

An example where less strict-typing is sometimes desirable:

class AwesomeMiddleware implements MiddlewareInterface
{
    /*
     * removed $delegate type-hinting!
     */
    function process(ServerRequestInterface $request, $delegate)
    {
        if ($delegate instanceof ResponseInterface) {
            return $delegate;
        } else if (is_callable($delegate)) {
            return $delegate($request);
        } else if ($delegate instanceof DelegateInterface {
            return $delegate->process($request);
        }
        //…
        return $unicorns;
    }
}

$am = new AwesomeMiddleware();
var_dump($am instanceof MiddlewareInterface); // => true
$am->process(new ServerRequest(), 666); // no error or warning 

[schnittstabil]

Mhm, @shadowhand do you still want to prevent use-cases like the AwesomeMiddleware above?

[schnittstabil]

The entire function or method won't be executed because of that exception and you have to catch it or end up with a hard error anyway.

@jschreuder Not necessarily, php-src/Zend/tests/objects_008.phpt shows why it is still valid to call them type-hints, they sometimes cause only warnings:

class test {
    function foo(Test $arg) {}
}

class test3 extends test {
    function foo(Test3 $arg) {} 
}

echo "Done\n";

// Warning: Declaration of test3::foo(Test3 $arg) should be compatible with test::foo(Test $arg) in %s on line %d
// Done