Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93365
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain mindplay.dk from 209.85.218.50 cause and error)
MIME-Version: 1.0
In-Reply-To: <ec6a9d04-83d0-1b70-4418-281e4b3f5a54@fleshgrinder.com>
References: <CADqTB_i5WL+rzCuFHkRmXsPj16WDc4eWmpAoFmyeuYXvuOP_NQ@mail.gmail.com>
	<CAEPJdunJ-67CqJq2hzvmnEmuOFQRGpxv8iycq3CnocXR4r20vw@mail.gmail.com>
	<CADqTB_iUDGHXUR5cYUDuzL4XrqJk-Jf0oce_uj6ZUtEYr7fTYw@mail.gmail.com>
	<CAEPJdunVsJWkOez3ERdpsKjFx8oxPmWVropZ5n9VKTfKdokpXg@mail.gmail.com>
	<CADqTB_jzRPG2CNXniyyo2XA36GGy_s29i5SGPfYe4c8gcpnK6g@mail.gmail.com>
	<61e6a083-91c2-799f-9757-02f56fe184e9@gmail.com>
	<CADqTB_jB-bXU5FjJoEqx-qTCiX6NSoo9YtxxHiX1X40FqmJ9Pg@mail.gmail.com>
	<ec6a9d04-83d0-1b70-4418-281e4b3f5a54@fleshgrinder.com>
Date: Tue, 17 May 2016 11:06:44 +0200
Message-ID: <CADqTB_jNcQ24=KC68vrpvXAEmeTJH9Xk4kB9Yr4Pi+3vRWsJiw@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC] Simple Annotations
From: rasmus@mindplay.dk (Rasmus Schultz)

Richard,

I believe I have already stated my position on all of these concerns
in my last email.

Short recap:

There's no such thing as optional dependencies - if you depend on a
Doctrine ORM annotation, you depend on that package.

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.

Silently ignoring missing annotations solves nothing - it just leads
to errors that are even harder to find and debug.

You seem awfully concerned about name collisions? Most packages use a
vendor\package namespace convention now - PHP has always been adding
things to the global namespace, and it's never been a really
substantial problem. I'm not that concerned.

The one new thing you brought up here, is the issue of potentially
loading/instantiating a bunch of annotations that go unused. From a
dependency point of view, I maintain that that's correct and it should
error out on missing classes.

From a performance point of view, you may have a valid concern, and
this had occurred to me before - but I decided to leave it alone, for
a couple of reasons. The most important reason being, look at how
annotations get used in practice: controllers get things like route
and filter annotations, all of which are usually needed at the same
time; entities get ORM annotations, all of which are usually needed at
the same time; form models get validation and input-type annotations,
all of which are usually needed at the same time. And so on.

The fact of the matter is that classes tend to have a single
responsibility, and this usually means that applicable annotations
tend to belong to a specific domain, and are needed at the same time.

In other words, this problem is pretty theoretical - in reality, it's
a very small problem, which, if somebody was very concerned about,
could absolutely be solved in userland, by adding a cache layer. Which
projects like Doctrine Annotations would likely do anyway, in order to
cache the result of mode complex annotation logic to control
inheritance, cardinality, applicability, etc. - all the complex stuff
that this RFC stays away from.

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.

I maintain that the majority use-case is object annotations - and that
deferring the construction of those objects doesn't solve anything.

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 and will always be in favor of language design that takes the
most direct route to satisfy a requirement. In this case, the
requirement is annotations as objects - and the most natural and
direct route is the "new" keyword, static method calls, or whatever
way you normally create objects; inventing a more indirect way to do
that just adds complexity.


On Mon, May 16, 2016 at 8:39 PM, Fleshgrinder <php@fleshgrinder.com> wrote:
> On 5/16/2016 3:05 PM, Rasmus Schultz wrote:
>> I'm sorry, but I think that the idea of meta-data that can't error
>> somehow, somewhere, at some point, is completely unrealistic.
>>
>> And I'd much rather have a simple facility that enables those errors
>> to surface quickly.
>>
>
> It should error and it should error exactly at the point when you want
> to use it and at no other point. Right now they will error anytime
> anyone wants to do anything with any annotation.
>
> I fully understand the urge to directly access objects and I actually
> support it and I want them to error out at the point where you try to
> instantiate them. However, not when I simply access annotations in
> introspection.
>
> According to your logic I have to load the whole dependency chain, even
> thought I just want to generate my documentation that might have some
> annotations in use. I also need to load all my dependencies even though
> I wanted to leave the security dependency out because I wanted to easily
> disable the security annotation system locally for development. I even
> have to load the whole dependency chain, even though I just want to
> introspect the single data structure at hand without invoking anything.
>
> Even worse if I am using a package that has Doctrine annotations but I
> use it without them: /fatal error/
>
> Nice? No!
>
> Solutions?
>
> Make the /simple/ annotations /simple/. ;-)
>
> Only allow scalar data to be defined and allow userland to register
> themselves for specific annotations. I mentioned this before but somehow
> everyone wants something more fancy and I have no karma (even though
> requested but ignored) to write my own RFC. That being said, you have
> the ability to write an RFC so please just take it. :-)
>
> Some PHP code to illustrate what I was thinking of since I am better
> with code than with words. :-P
>
> https://gist.github.com/Fleshgrinder/d26cd4751827b8c10a0dc33da32b48c3
>
> Reflection is the wrong tool for the job to build annotation systems.
> Reflection should not be used in production it should be used to
> introspect data structures and reason about them or other stuff during
> unit tests.
>
> However, reflection should provide the ability to read annotations, it
> just does not do anything with them by default and outputs stupid
> arrays. You will definitely understand what I mean it you follow the Gist.
>
> I am sure there is room for improvement and that I overlooked something
> in my Gist. But I think it is a starting point, some additional notes:
>
> - I do not see any benefit in annotations for parameters.
> - I do not see any benefit in annotations for Generators.
> - I do not see any benefit for annotations in php-src.
>
> My proposal would include to ban annotations from php-src and make it a
> pure userland functionality. This has many advantages:
>
> - No name collisions.
> - Clear policy for the future.
> - php-src features are never optional.
> - php-src does not provide meta-data.
>
> Let me know what you think or if you have questions in regards to the
> Gist that are unclear. (Or maybe you found mistakes.)
>
> --
> Richard "Fleshgrinder" Fussenegger
>