On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > I believe I have already stated my position on all of these concerns > in my last email. >
Sorry if I cannot keep everything in my mind. On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > There's no such thing as optional dependencies - if you depend on a > Doctrine ORM annotation, you depend on that package. > Both statements are not true. #1 A good example of an optional dependency can be found in the Symfony console package: https://github.com/symfony/console/blob/master/composer.json#L28 https://github.com/symfony/console/blob/master/Application.php#L814-L816 #2 I could have code in a third-party package that provides some default annotations which suggests me to install some annotation systems that are capable of handling exactly those annotations. However, I decide to only use one of the suggested annotation systems in my project. With your solution this is not possible because it always ends up in fatal errors; although I consciously decided to not install one suggested dependency. See #1 for such an example. The only solution for the maintainer of the third-party package would be to provide multiple versions of the same package: 1. No annotations. 2. Annotations from System X. 3. Annotations from System Y. 4. Annotations from System X and Y. 5. ... This is not an acceptable approach. On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > The code should (must) error the moment you try to instantiate those > annotations - whether you intend to use them or not is besides the > question - a missing class is an error, in annotations as it is > anywhere else in your code. > It is not (again see #1) a problem at compile time and always at runtime. You can try this easily on your console: $ php -r 'if (false) { new NonExistingClass; }' $ php -r 'if (true) { new NonExistingClass; }' Fatal error: Uncaught Error: Class 'NonExistingClass' not found in Command line code:1 Stack trace: #0 {main} thrown in Command line code on line 1 On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > Silently ignoring missing annotations solves nothing - it just leads > to errors that are even harder to find and debug. > My proposal errors as soon as you try to retrieve an annotation with the annotation reader which was not previously registered as such. However, you can use Reflection and introspect any existing annotation without any kind of error. That is how it should be! On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > You seem awfully concerned about name collisions? > No, I am not. It is just a nice side product of my proposal. On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > The one new thing you brought up here, is the issue of potentially > loading/instantiating a bunch of annotations that go unused. [...] > From a performance point of view [...] > The problem is parsing and interpreting them although they might not be used because the current path of logic will not go through them at all. This is not a micro optimization thing. You just don't want to interact with annotations at all until they are actually needed. This is true for any kind of PHP code. See console commands above. On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > The extreme case of what you're proposing, is what Go does - an > annotation is simply a string. No chance of any error, anywhere, ever, > right? Wrong. People put JSON data in those strings for example, and > parsing that data leads to run-time errors instead. That's the extreme > example, but the problem with what you're proposing is precisely the > same: you allow something like nested array data structures, those are > still going to get interpreted and mapped to objects at run-time, and > it leads to run-time errors instead. > Runtime errors for a runtime feature, well, yes. Again, see console command example above. @someAnnotation('{"json": "data"}') function f(); @someAnnotation('invalid json') function x(); $reflector = new ReflectionFunction('f'); $annotations = $reflector->getAnnotations(); /* $annotations = [ 'someAnnotation' => [ ['{"json": "data"}'], ], ]; */ $reflector = new ReflectionFunction('x'); $annotations = $reflector->getAnnotations(); /* $annotations = [ 'someAnnotation' => [ ['invalid json'], ], ]; */ annotation_register('someAnnotation', function (string $json) { $decoded_json = json_decode($json); if ($decoded_json === false) { throw new JsonParseException(json_last_error_msg()); } return $decoded_json; }); $annotations = new AnnotatedFunction('f'); $some_annotations = $annotations->getAnnotations('someAnnotations'); echo $some_annotations[0]->json; // data $annotations = new AnnotatedFunction('x'); $some_annotations = $annotations->getAnnotations('someAnnotations'); // Fatal error: Uncaught JsonParseException in ... The above is just fine. The annotation library registered the callback and it knows what kind of errors can happen. The annotation library is also the one reading the annotations and it can therefore easily handle those errors and act upon them. Of course in the above case the fatal error is just fine and it is the developers duty to write unit tests and take care of writing proper configuration. It is not PHP's duty to ensure to absolutely no error can happen in any subsystem. (This is actually ridiculous to think of in the first place.) On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > I maintain that the majority use-case is object annotations - and that > deferring the construction of those objects doesn't solve anything. > Seriously? Every construction is deferred, always, again see the console example from above. On 5/17/2016 11:06 AM, Rasmus Schultz wrote: > You're just deferring or hiding the problem, adding complexity, and > mental as well as practical debugging effort - but the problem doesn't > go away. > I am putting a runtime feature to runtime like all runtime features are at runtime in PHP. The error is hidden during reflection, that is true, but on purpose! You do not want reflection to error out while you are performing introspection on a data structure. The annotation reader however will error out in the moment you try to access something. This separation of concern makes it actually easier and is in the vain of good software development too. I understand what you want to solve. I am just telling you that this is the wrong approach and others are telling it to you too. -- Richard "Fleshgrinder" Fussenegger
signature.asc
Description: OpenPGP digital signature
