Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121850
Content-Type: multipart/alternative;
	boundary="Apple-Mail=_7DBEECCA-0A23-44AA-8B60-F204C9000A3F"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.200.91.1.1\))
Date: Wed, 29 Nov 2023 14:19:24 +0700
References: <6566989F.7010305@adviesenzo.nl>
 <34dada8e-7f2a-4d94-b7df-d9d3c7b2f3ce@app.fastmail.com>
To: php internals <internals@lists.php.net>
In-Reply-To: <34dada8e-7f2a-4d94-b7df-d9d3c7b2f3ce@app.fastmail.com>
Message-ID: <F200E6CF-5116-4361-AB5C-CDD67F2E2021@koalephant.com>
Subject: Re: [PHP-DEV] What is the prevailing sentiment about extract() and
 compact() ?
From: php-lists@koalephant.com (Stephen Reay)

--Apple-Mail=_7DBEECCA-0A23-44AA-8B60-F204C9000A3F
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii



> On 29 Nov 2023, at 09:58, Larry Garfield <larry@garfieldtech.com> =
wrote:
>=20
> On Tue, Nov 28, 2023, at 7:49 PM, Juliette Reinders Folmer wrote:
>> L.S.,
>>=20
>> What with all the drives towards cleaner code, how do people feel=20
>> nowadays about `extract()` and `compact()` still being supported ?
>>=20
>> Both have alternatives. The alternatives may be a little more =
cumbersome=20
>> to type, but also make the code more descriptive, lessens the risk of=20=

>> variable name collisions (though this can be handled via the $flags =
in=20
>> extract), prevents surprises when a non-associative key would be=20
>> included in an array and lessens security risks when used on =
untrusted data
>=20
> *snip*
>=20
>> I can imagine these could be candidates for deprecation ? Or limited=20=

>> deprecation - only when used in the global namespace ?
>>=20
>> For now, I'm just wondering how people feel about these functions.
>>=20
>> Smile,
>> Juliette
>=20
> extract() has very limited use in some kinds of template engine, which =
use PHP require() as a template mechanism.  I don't think compact() has =
any uses.
>=20
> I very recently was just reminded that these even exist, as i had to =
tell one of my developers to not use them.  I think it was compact() he =
was trying to use.  I vetoed it.
>=20
> I would not mind if they were removed, but I don't know how large the =
BC impact would be.  They'd probably need a long deprecation period, =
just to be safe.
>=20
> --Larry Garfield
>=20
> --=20
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>=20

Hi,

While I think I understand the goal behind this, I think you're missing =
some factors here.

Regarding use-cases for compact: the most common one I can think of from =
my work, is for passing multiple local variables as context to a logging =
function, but I'd be surprised if its not also used to build faux hash =
structures too.

If your goal is to achieve an associative array (i.e a poor mans hash) =
of known variable names, using compact in php8+ has *less* risk of =
uncaught/unexpected errors than building it manually. Passing an =
undefined name (i.e. due a typo, or it just not being defined) produces =
a warning regardless of whether you build the array manually or pass the =
name(s) to compact(). Providing an array key name that doesn't match the =
variable name (e.g. due to a typo, or a variable being renamed) will =
produce no error when building the array manually, but will produce a =
warning with compact().=20

IDEs (e.g. PHPStorm/IDEA+PHP plugin) can already understand that the =
names passed to compact are a variable name, and make changes when a =
variable is renamed via the IDE. They simply cannot do the same for =
plain array keys.

Due to how variable scope works, the only way to re-implement compact() =
with the same key-typo-catching behaviour as a function in userland =
would be something that requires the user to pass the result of =
get_defined_vars() to every call.

So no, I don't think compact() should be deprecated, what I think =
*should* happen, is to promote the current warning on undefined =
variables, to an error, as per =
https://wiki.php.net/rfc/undefined_variable_error_promotion. Whether =
this is a foregone conclusion or not, I don't know because that RFC =
doesn't mention compact() specifically.


extract(), as Larry points out has historically been used by 'pure php' =
style template systems, in a manner that's generally "safe". Personally =
I'm less inclined to use this behaviour now (i.e. I'd prefer to access =
named & typed properties from a template than arbitrary local variable =
names) but I don't think that's enough of a case to remove it, because =
just like with compact, by nature of how variable scope works, it's very =
difficult/impossible to re-implement this in userland, in a way that's =
reusable and doesn't involve using worse constructs (e.g. eval'ing the =
result of a function)

I think there's possibly an argument to be made for improvements, such =
as changing the default mode of extract to something besides =
EXTR_OVERWRITE, or to have checks in place preventing the overwrite of =
superglobals.


Cheers


Stephen=20=

--Apple-Mail=_7DBEECCA-0A23-44AA-8B60-F204C9000A3F--