Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116208
MIME-Version: 1.0
References: <CAB-BujYDenMHYUTy13LEMW9MnpO-sbZhvJpaQB0D6aRvrX7hBA@mail.gmail.com>
In-Reply-To: <CAB-BujYDenMHYUTy13LEMW9MnpO-sbZhvJpaQB0D6aRvrX7hBA@mail.gmail.com>
Date: Mon, 4 Oct 2021 10:58:09 +0200
Message-ID: <CAF+90c-UzyExTFs1r66S0cn81eNSS+afQsHoObK28nXZigd2Bw@mail.gmail.com>
To: Andreas Hennings <andreas@dqxtech.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000aca42505cd831db9"
Subject: Re: [PHP-DEV] Static (factory) methods in attributes and initializers
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000aca42505cd831db9
Content-Type: text/plain; charset="UTF-8"

On Mon, Sep 27, 2021 at 5:58 PM Andreas Hennings <andreas@dqxtech.net>
wrote:

> Hello list,
>
> currently, the default mode for attributes is to create a new class.
> For general initializers, with
> https://wiki.php.net/rfc/new_in_initializers we get the option to call
> 'new C()' for parameter default values, attribute arguments, etc.
>
> Personally I find class construction to be limiting, I often like to
> be able to use static factories instead.
> This allows:
> - Alternative "constructors" for the same class.
> - A single constructor can conditionally instantiate different classes.
> - Swap out the class being returned, without changing the factory name
> and signature.
>
> In fact, static factories for initializers were already mentioned in
> "Future Scope" in https://wiki.php.net/rfc/new_in_initializers.
> However this does not mention static factories for the main attribute
> object.
>
> For general initializers this is quite straightforward.
> For attributes, we could do this?
>
> // Implicitly call new C():
> #[C()]
> # Call the static factory instead:
> #[C::create()]
>
> So the only difference here would be that in the "traditional" case we
> omit the "new " part.
>
> We probably want to guarantee that attributes are always objects.
> We can only evaluate this when somebody calls ->newInstance(), because
> before that we don't want to autoload the class with the factory. So
> we could throw an exception if the return value is something other
> than an object.
> I was also considering to require an explicit return type hint on the
> factory method, but again this can only be evaluated when somebody
> calls ->newInstance(), so the benefit of that would be limited.
>
> The #[Attribute] annotation would allow methods as target.
>
> Reflection:
>
> ::getArguments() -> same as before.
> ::getName() -> returns "$class_qcn::$method_name".
> ::getTarget() -> same as before.
> ::isRepeated() -> This is poorly documented on php.net, but it seems
> to just look for other attributes with the same ->getName(). So it
> could do the same here.
> ::newInstance() -> calls the method. Throws an error if return value
> is non-object.
>
> we could add more methods like ReflectionAttribute::isClass() or
> ReflectionAttribute::isMethod(), or a more generic ::getType(), but
> these are not absolutely required.
>
> We could also allow regular functions, but this would cause ambiguity
> if a class and a function have the same name. Also, functions cannot
> be autoloaded, so the benefit would be small. I'd rather stick to just
> methods.
>

I see where you're coming from here, and I think an argument could be made
that our attribute syntax should have been #[new C()], allowing us to
associate arbitrary values -- which would naturally allow the use of static
factory methods once/if they are supported in constant expressions.

As that particular ship has sailed, I'm not convinced that supporting
static factory methods as "attributes" would be worthwhile. It's a
significant complication to the system (that users need to be aware of, and
consumers of the reflection API need to handle) for what ultimately seems
like a personal style choice to me. Do you have any examples where using
static factories over constructors for attributes would be particularly
compelling?

Regards,
Nikita

--000000000000aca42505cd831db9--