On Tue, 30 May 2023 at 15:14, Stephen Reay <php-li...@koalephant.com> wrote:
>
> (Resending to the list without all the history because qmail complained about
> message size)
>
>
> >>
> >> Hi Andreas,
> >>
> >> I too have wondered (and I think asked in room11?) about such a concept.
> >> >From memory the general response was “just do it in userland with a
> >> wrapper” so its good to see someone else is interested in this being part
> >> of the language.
> >>
> >> While I agree that it’s most useful if the `Reflector` instance is
> >> available in the constructor, I’m not keen on the proposed magic
> >> “skipping” of arguments as you suggest. It seems way too easy to confuse
> >> someone (particularly if the attribute class itself has reason to be
> >> instantiated directly in code)
> >
> > Good point! Almost made me change my mind completely. But I already
> > changed it back :)
> >
> > When instantiating in code, the "real" signature would have to be
> > used, and the reflector argument passed explicitly.
>
>
> That’s kind of my point: it’s not super intuitive why (or the specifics of
> how) it’s being skipped when it’s an attribute, vs when it’s instantiated
> from code. What if someone specifies an argument with the same name? If they
> specify args without names, can they just use null for that? Etc.
I agree it could be confusing.
But for the named args, I think it is quite obvious:
#[Attribute]
class A {
public readonly array $moreArgs;
public function __construct(
public readonly string $x,
// Reflector parameter can be last required parameter, BUT
#[AttributeDeclaration] public readonly \ReflectionClass $class,
// Optional parameters have to be after the required reflector parameter.
public readonly ?string $y = NULL,
// Variadic parameter must be last.
string ...$moreArgs,
) {
$this->moreArgs = $moreArgs;
}
}
#[A('x', 'y', 'z')] // -> new A('x', $reflector, 'y', 'z')
#[A(x: 'x', y: 'y')] // -> new A('x', $reflector, 'y')
#[A(x: 'x', class: new ReflectionClass('C'))] // -> new A('x', new
ReflectionClass('C'))
We _could_ say that explicitly passing a value for the reflector in an
attribute declaration is forbidden.
Or we allow it, then an explicit value would simply overwrite the
implicit value.
If we use placeholder syntax, the above examples would look like this:
#[A('x', ?, 'y', 'z')] // -> new A('x', $reflector, 'y', 'z')
#[A(x: 'x', class: ?, y: 'y')] // -> new A('x', $reflector, 'y')
#[A(x: 'x', class: new ReflectionClass('C'))] // -> new A('x', new
ReflectionClass('C'))
>
> > This would be
> > useful for unit tests that want to replicate the realistic behavior.
> > Also it guarantees that the code of the attribute class can really
> > count on this value to not be null, no matter how the class is
> > instantiated.
> >
> >
>
> I would expect that whether the Reflector object is required is simply a
> matter of whether or not the parameter is nullable.
> If it’s not nullable, then yes, the explicit instantiation call will need to
> supply it at the correct location. If it’s only required when created from
> attribute usage, then it would accept null, and the constructor would have
> appropriate logic to handle that.
>
Yes.
But I would expect the common practice to be to make it required,
because then the constructor code will be simpler.
>
>
> >>
> >> I think a better approach would be to suggest authors put the parameter at
> >> the *end* of the parameter list, so that no ‘skipping' is required when
> >> passing arguments without names (or put it where you like if you’re always
> >> using named arguments)
> >
> > If I understand correctly, the proposal would technically not change,
> > we just add a recommendation.
>
> Technically, yes “my way” would work fine with the proposal you’ve suggested,
> if I choose to always put the parameter marked by #[ReflectionContext] last.
As above, the problem with this would be optional and variadic
parameters, which have to come after a required reflector parameter.
>
> I’m just concerned about confusing usage if “insert this parameter anywhere”
> is the ‘recommended’ (i.e. documented example) way to use this feature.
>
> Even with that concern, I still prefer this to most other solutions mentioned
> so far, for the same reasons: they’re all some degree of magic.
>
> The only other solution I can think of that’s less “magic” and more explicit,
> is (and I have no idea if this is even feasible technically) to introduce a
> builtin trait for attribute classes to use, providing a protected method or
> property that gives access to the Reflector (how the trait has access is not
> really important, I assume it can be assigned to the object somehow before
> the constructor is called). I guess this could also be an abstract class, but
> a trait makes it much easier to adopt so that would be my preferred approach.
>
> So something like
>
> trait AttributeReflector {
> protected function getReflector(): \Reflector {
> // do internal stuff
> }
> }
>
> #[Attribute]
> class Foo {
> Use \AttributeReflector;
>
> public readonly string $name;
>
> function __construct(?string $name = null) {
> $this->name = $name ?? $this->getReflector()->name;
> }
> }
I was also considering this, but there is a problem.
When you instantiate the attribute class outside an attribute
declaration, how do you tell it about a required reflector?
This would be relevant e.g. during unit tests.
The constructor parameter provides a clean way to pass a custom reflector.
But with the ->getReflector(), the reflector would already be
magically added to the object before the constructor is executed. This
would be impossible to replicate in custom code outside an attribute
declaration.
---
Cheers
Andreas
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
