Remove a test that was out of scope

[mnapoli]

The definition created in this test is invalid, i.e. what the standard method ObjectDefinition::getConstructorArguments() returns isn't compliant with the interface (what's documented in the phpdoc). So this test was validating a behavior that shouldn't happen in the first place.

[mnapoli]

Btw once that is fixed Assembly and PHP-DI will have green tests \o/

[moufmouf]

I really think we need to discuss this.

Yes, the definition created in this test is invalid. The return isn't compliant with the interface.
BUT, this is the whole point. PHP (even PHP 7) cannot ensure us that we have an array of DefinitionInterface passed in parameter or returned (lack of generics).

I very well understand this is out of scope, and yet, I believe we should at least give a guidance to implementers on what to do if they are given invalid values. I'm a big fan of defensive programming (http://bestpractices.thecodingmachine.com/php/defensive_programming.html) As part of this, I try to make sure that unexpected values are handled by my code (as errors).

If I was developing Assembly, I would probably add a safety check here: https://github.com/mnapoli/assembly/blob/master/src/Container/DefinitionResolver.php#L97-L103
I would make sure that if an object is passed to resolveSubDefinition and if that object is not a DefinitionInterface, then an exception is triggered.

definition-interop is all about interoperability and we want to make sure that code running on a container will run on any other container out there. What I mean is that we should somehow restrict the possibilities to "embrace and extend" the standard. If you are old enough, you maybe remember the trial between Microsoft and Sun about Microsoft extending Java (by allowing importing DLLs into Java code). Microsoft JVM was able to run Java code from other JVMs but if you were developing on a Microsoft JVM, you were almost guaranteed that your code would not work on Sun JVM (because at some point, you would use a MS JVM specific feature).

We have a similar problem here. If people start using PHP-DI or Assembly container, at some point, they will write something like:

\Assembly\object('MyClass')->setConstructorArguments(new DateTime());

If it works in Assembly or PHP-DI, they will expect it to work anywhere. If it does not work in Yaco, or Symfony-definition-interop bridge, they will assume the compiler/container is broken, not their code. We cannot trust our users into reading the spec and understanding that setConstructorArguments should contain ONLY scalars or DefinitionInterfaces. On the contrary, if all implementing containers/compilers are just paranoid enough to check that the user actually provides parameters that are coherent, we will ensure a greater level of compatibility. So I definitely think this test is useful, as an advice to compiler/container implementers.

So here is a proposal. What about moving this test into an optional test class (AbstractDefinitionCompatibilityOptionalTest) ?

This test could contain all the defensive programming tests that would be recommended to implement. We are not making those compulsory tests, but a recommended behaviour. Would you be ok with this approach?

[mnapoli]

Cleaning up my pending PRs 😄