Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113973
MIME-Version: 1.0
References: <CAES=RMtF9FbRgRrigbm8mew38z8Yba9aZobibJCuquOt8a_uWA@mail.gmail.com>
 <0c5e70d7-5b11-5251-4ec9-608db63105c9@gmail.com>
In-Reply-To: <0c5e70d7-5b11-5251-4ec9-608db63105c9@gmail.com>
Date: Mon, 5 Apr 2021 21:00:31 -0400
Message-ID: <CAES=RMs5rQq4U2br9mpFH4WWRTCYeJY-N-s_8HmVLdTK=EaETQ@mail.gmail.com>
To: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000f8753005bf43572a"
Subject: Re: [PHP-DEV] Path forward for class_exists with different targeted
 PHP version implementations
From: jr@rushlow.dev (Jesse Rushlow)

--000000000000f8753005bf43572a
Content-Type: text/plain; charset="UTF-8"

Sure thing -

In Symfony's MakeBundle we have a command "php bin/console make:command".
This ultimately generates a "App\Console\XyzUserlandCommand::class" that
allows the developer to customize in order to perform some task within
their app. e.g. run persistence operations, generate a backup, etc..

What we do internally to generate that userland class is determine which
features to include by checking the PHP version, dependencies available,
etc.. As an example, when the user runs "make:command"; we check if they
have PHP 8 && if "class_exists(Symfony\.....\Attributes::class)" - if both
of those === true, we import the attribute class and generate the needed
attributes for the command. Otherwise we would fall back to using
annotations if the user is running PHP 7. Where I ran into trouble was the
Attributes::class (defines the attributes to be used in userland) utilized
constructor property promotion (which is a pretty sweet addition to 8 imo).

Up to this point if it's possible, we generally pick a
class/interface/method that introduces new functionality (Symfony
feature/bugfix) and call one of the \*_exists(class/method/interface)
functions in a conditional to determine if we can indeed use that feature.
But because in this particular case the Attributes::class also uses
constructor property promotion, when calling
class_exists(Attributes::class) in PHP 7 we get:


ParseError: syntax error, unexpected 'public' (T_PUBLIC), expecting
variable (T_VARIABLE)

The work around was to "$canUseAttributes = 80000 <= \PHP_VERSION_ID && \
class_exists(AsCommand::class)"

All is fine and well, but this in my opinion, this feels like bad mojo from
a developers standpoint. I must now need to be aware of the exact
implementation details of the object I'm checking before I can even check
if it exists or not. I would expect any of the \*_exists() to tell me if
the class/method/interface exists regardless of the implementation. My
first thought would be if the implementation is not compatible with the PHP
version some sort of \RuntimeException::class would be thrown by PHP. But
that is where my knowledge of PHP internals is disconnected from developing
software in PHP.

I created a simple reproducer repo
https://github.com/rushlow-development/attribute-test (forgive the repo
name, it really has nothing to do with attributes.) and for an even simpler
example https://3v4l.org/4cMW4

I apologize in advance for the wall of text - but I'm curious if there is a
way to either improve our native functions, improve the docs, and/or create
new functions for use cases like this.

Thanks!
Jesse Rushlow

On Mon, Apr 5, 2021 at 3:30 PM Rowan Tommins <rowan.collins@gmail.com>
wrote:

> On 05/04/2021 18:14, Jesse Rushlow wrote:
> > I was attempting to use class_exists(UsesPropertyPromotion::class) to
> > determine if an attribute implementation existed in order to generate a
> PHP
> > 8 appropriate class - else fall back to generating a PHP 7 appropriate
> > class. For context, this check was being written for Symfony's
> MakerBundle
> > which generates classes for Symfony projects.
>
>
> Hi Jesse,
>
> Can you explain the use case in a bit more detail, perhaps giving some
> example code you were hoping to write with class_exists()?
>
> I suspect there are other ways to achieve what you were trying to do,
> but don't want to confuse things if I've misunderstood the requirements.
>
> Regards,
>
> --
> Rowan Tommins
> [IMSoP]
>
>

--000000000000f8753005bf43572a--