Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:111963 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 81228 invoked from network); 30 Sep 2020 13:01:39 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 30 Sep 2020 13:01:39 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 229081804D8 for ; Wed, 30 Sep 2020 05:13:31 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,FREEMAIL_REPLY, HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 30 Sep 2020 05:13:30 -0700 (PDT) Received: by mail-ej1-f46.google.com with SMTP id qp15so1530537ejb.3 for ; Wed, 30 Sep 2020 05:13:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=x+SJUHpe7wVxffX/gG1HcxERWhSJGSBRE54o+NhitlY=; b=NFApPX6R7EKTMAGsc+hcXSx9ZUgFwfru1EG7S93EJOjSGV6ulcRdiEySKzr30Ft7EV eF2RtPigqeD4K/I3bNn/1Mcrw/rbTff3mQQa4XVf+lu1hPLWawspLBpTf12q2G1tIQ0T 2bzY1ztsLJmwmAO8155SleTcic5DtCt7PSTLpO35FalT/B7co8slCdM/2s4rOLPtRQjS JYuPJ/VOHaNXHPsEvV4ss9eDoKROfJIKJS+x1p6Qas02Ie2LFYFuIe6JQzddLpgryKpO s+JgV7kTI1z7OuHUeLSZzjYmGkGxJsSdxspDTR0boJb1qs99ESsumFY84UrwjXod48qf /EMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=x+SJUHpe7wVxffX/gG1HcxERWhSJGSBRE54o+NhitlY=; b=Ap++S4CTTy0SKMS8RR+fSZZ/YLfhukdxryhNTnDuiymhg1M9FHIEjm5mC+CjjI0/AA hn3qTbIag2Yb5e3mLXiqAtzRQb6vo/NmQ7oU2sJqubRtAZJF6x8njpEze2NByFCbQhdH ML6DR71EV+dPq0QELh8s9x4bb27fxyUUJd5dhbgOwebYSk5hKn8t7wtYwGWIvD+9ahd3 t2cnFgxsHFIBbLc62nrWPJQ2MY6+2UICjuwa4x6iZ0BRO2hfZ0BvVmbUo490mejIpX78 GU8yoTxFR9Dp4bua4/sbf4qyAdKa6Awm/x74mZXCLjaSnqBCU6cVY7BdzQhRRpRUn2lI LTwg== X-Gm-Message-State: AOAM533yQ0T0scmvMy8xnjpgW5tzNj+bdY+AddMvi6+g+2tCA0XQ1aHQ anZvhqi45qtwVbWG6BZSSwnTMVDMjriRVFZWixE= X-Google-Smtp-Source: ABdhPJzQJf3VhMOKoQKpDw8yx0aKW8t2epko7Fbinub8WDcZbzdzVQkL7TOHJXp8s5idBJMgKKQRBne7aG3dc/dfixg= X-Received: by 2002:a17:907:b0c:: with SMTP id h12mr2393901ejl.115.1601468006755; Wed, 30 Sep 2020 05:13:26 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: Date: Wed, 30 Sep 2020 14:13:14 +0200 Message-ID: To: Nikita Popov Cc: Benjamin Eberlei , Larry Garfield , php internals Content-Type: multipart/alternative; boundary="000000000000a9764805b086d3ef" Subject: Re: [PHP-DEV] Re: Attributes and constructor property promotion From: nicolas.grekas+php@gmail.com (Nicolas Grekas) --000000000000a9764805b086d3ef Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Le mer. 30 sept. 2020 =C3=A0 12:45, Nikita Popov a = =C3=A9crit : > On Tue, Sep 29, 2020 at 3:57 PM Benjamin Eberlei > wrote: > > > On Tue, Sep 29, 2020 at 3:45 PM Larry Garfield > > wrote: > > > > > On Mon, Sep 28, 2020, at 12:06 PM, Benjamin Eberlei wrote: > > > > On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel < > > benjamin.morel@gmail.com > > > > > > > > wrote: > > > > > > > > > On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas < > > > nicolas.grekas+php@gmail.com> > > > > > wrote: > > > > > > > > > >> I assume the 80% case is properties, because attributes did not > have > > > > >>> docblock annotations yet, that means this use-case isn't even > > > possible at > > > > >>> the moment. Yet annotations on properties are widespread > (Doctrine > > > ORM, > > > > >>> symfony validator, ...). > > > > >>> > > > > >> > > > > >> I'm 100% with Benjamin here, this is what will be the most usefu= l > to > > > me > > > > >> also. > > > > >> > > > > > > > > > > To be clear, I don't have a strong opinion against yours, I'm jus= t > > > > > pointing out the fact that even though it might be useful, it mig= ht > > > also be > > > > > confusing and create yet another WTF moment in PHP for developers= . > > > Sure, it > > > > > might make more sense to apply to the property. Sure, so far > > > annotations > > > > > weren't possible on parameters. But is that obvious to the averag= e > > > > > developer writing the attribute? A few years down the road, DI > > > containers > > > > > may have broad support for annotating parameters for injection. > Will > > it > > > > > still be obvious then that an attribute on a promoted property > > applies > > > to > > > > > the property only? > > > > > > > > > > I do agree that applying the attribute to both the property and t= he > > > > > parameter will probably never be useful, though. So, throwing an > > > exception > > > > > and forcing the de-sugaring feels like the most sensible thing to > do > > > for me > > > > > in this case! > > > > > > > > > > > > > I haven't checked if this is possible in the code doing the > > "desugering", > > > > but what if we had an attribute on the constructor that could speci= fy > > > where > > > > the attributes should apply to? > > > > > > > > #[AttributePromotion(Attribute::TARGET_PROPERTY)] > > > > public function __construct(#[Foo] public string $bar) {} > > > > > > > > Then we could apply it to both by default, which is what is probabl= y > > the > > > > expected approach, and users could change it to apply only to > > properties, > > > > which is what is the use-case that makes most sense. > > > > > > > > > > > > > > =E2=80=94 Benjamin > > > > > > From a user experience POV, I'd almost expect the opposite. > > > > > > If I mark the attribute as being for properties, it gets applied to t= he > > > property. > > > > > > If I mark the attribute as being for parameters, it gets applied to t= he > > > parameter. > > > > > > If I mark it for both, or don't restrict it at all, it applies to bot= h. > > > > > > That may not be how the logic is currently implemented but that's wha= t > as > > > a user I'd find least-surprising. > > > > > > > The problem with this approach is that it would require autoloading the > > attributes when they are assigned to either the internal property or > > parameter struct, but we have the design goal *not* to trigger > autoloading > > unless newInstance() or getArguments() is called. What you could do in > > userland code is handle this case yourself and never newInstance() > > attributes that don't apply to the right "thing" (parameter vs property= ). > > but that would defer the problem to userland with some annoying piece o= f > > code. > > > > So, as there seems to be resistance to applying the attribute to properti= es > only I only see a couple of options: > > 1. Forbid combining attributes and promotion. > That'd be quite deceptive to me. > 2. Relax attribute validation for this case, as implemented in > https://github.com/php/php-src/pull/6244. I think if we otherwise stick t= o > the current behavior, this is what we should do to avoid false-positive > errors. > This makes a lot of sense actually! Just to be sure, "ignore the error" means that attributes that target properties would be applied only to the property, and attributes that target params only to the param, isn't it? Or does it mean that they would still be applied to both, but not error would be raised? > 3. Disambiguate what is desired somehow. __construct(#[PropAttr] public i= nt > $x) vs __construct(public #[ParamAttr] int $x). This is ... a bit too muc= h > magic :) > a bit too much magic I agree :) Nicolas --000000000000a9764805b086d3ef--