Re: What is the prevailing sentiment about extract() and compact() ?

> On 29 Nov 2023, at 09:58, Larry Garfield <larry@garfieldtech.com> wrote: > > On Tue, Nov 28, 2023, at 7:49 PM, Juliette Reinders Folmer wrote: >> L.S., >> >> What with all the drives towards cleaner code, how do people feel >> nowadays about extract() and >> compact() still being supported ? >> >> Both have alternatives. The alternatives may be a little more cumbersome >> to type, but also make the code more descriptive, lessens the risk of >> variable name collisions (though this can be handled via the $flags in >> extract), prevents surprises when a non-associative key would be >> included in an array and lessens security risks when used on untrusted data > > *snip* > >> I can imagine these could be candidates for deprecation ? Or limited >> deprecation - only when used in the global namespace ? >> >> For now, I'm just wondering how people feel about these functions. >> >> Smile, >> Juliette > > 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. > > 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. > > 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. > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > 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(). 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