Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:124787 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by qa.php.net (Postfix) with ESMTPS id 700D01A00B7 for ; Mon, 5 Aug 2024 19:37:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1722886780; bh=jsD79hh4IUY/t42T9U5fR78nCMUNDMc3JJnfWHeUm74=; h=Date:From:To:In-Reply-To:References:Subject:From; b=WjrRzIV8pVwj9K5xIsRolKHCnCBFWVJpv6juSLVkgcd+z/Cdan6xlULizzaWslYv2 69ODik6Rarpssf17frWDiaPXXVOjA4ORqNYrhcS9kjBWa81nVmPk9TiBA11qK0HKqU +SQhtE4PV/NXi8YuXbT7ju6nfb/QYZ3kIfRExiMh0WXbdmCy/MEDVLPVHDrk8k1rpu Cjh1AutcEsJzHpJiPnbGBOq6EsUoC2LvRih1Qlq/YmHKiTUaozZ4gxR+rOi6115/rH w9l/Juw35gP8jNzS5SICNIq46WJVaJWjv5pjLaigXy0PXKZNnoLXvYpeXHin6A6iGd 62n3goEZ8xm9A== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 335561801EF for ; Mon, 5 Aug 2024 19:39:37 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-0.1 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_MISSING,HTML_MESSAGE, RCVD_IN_DNSWL_LOW,SPF_HELO_PASS,SPF_PASS autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from fout6-smtp.messagingengine.com (fout6-smtp.messagingengine.com [103.168.172.149]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Mon, 5 Aug 2024 19:39:34 +0000 (UTC) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailfout.nyi.internal (Postfix) with ESMTP id 617CC13880A7 for ; Mon, 5 Aug 2024 15:37:53 -0400 (EDT) Received: from imap49 ([10.202.2.99]) by compute3.internal (MEProxy); Mon, 05 Aug 2024 15:37:53 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bottled.codes; h=cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm1; t=1722886673; x=1722973073; bh=jsD79hh4IU Y/t42T9U5fR78nCMUNDMc3JJnfWHeUm74=; b=rVG59avt3d3A3qYYmPItWVzBPv JWroDtj/vmeiMQ+pQr4KUmL6zW/veg/br18/cxftKpBR8zryx+O6RbEvP6xTlvfO BuuzOO5h5R8OaPieQBj9/wagGANFihEvSGng49Yp6cAh6hB81jL7ykep7lfeepMC WpE6DM7lPDojdHkd60KYpbqN8mSzxyBjO7EwV4fOr9lqgXyGkgYcIWPhcqrFaPXc 6TPFmilHKTfWyrNBoq9i0zJLLCU8Tu8LRLjNSuxZjR1akUmy5ABq8IumIDG2yX6t A+qyWSR82QJ7qT5xCyoTbr/RTs9cDPzHkU03MQ5IK3kGRUKWAGOgzrXA9dgw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722886673; x=1722973073; bh=jsD79hh4IUY/t42T9U5fR78nCMUN DMc3JJnfWHeUm74=; b=HB9LHsbDuI5yKmm7sQBaqPC8pf2sSG2DBcIzS+xngSE8 E54ZHHL/xyFBA4v5lpR7Kl4Pd8TAUSgqT+nNjmUqrKK/i1fSna3JyCyxMCgsgji2 XnDH9Bkrm50aqmJ5MG9EjcjVgcnK0obKBKdS8ME5iWoDxd/5pe70mudYCWz9sO6H q2pEbd8EbarNKkX2zmmGd30knhDsnsmSAJKhp6bjVUzbURvuZz9ppb0aVTPPHtcr tpORdgmLiStTDIaBovYUJbR/9lQG5vDKjg/pMpRmjaQKQiUxqIbGNM8E95qPuWzC xac4dbrIpX/BmV9AaoeLbWxjQ4YHbGOH5VFIsx3DwQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigddugedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucgoufhushhpvggtthffohhmrghinhculdegledmne cujfgurhepofggfffhvffkjghfufgtsegrtderreertdejnecuhfhrohhmpedftfhosgcu nfgrnhguvghrshdfuceorhhosgessghothhtlhgvugdrtghouggvsheqnecuggftrfgrth htvghrnheptdeitddvvdevhfdufffhgeelffetgeffveekheekfeeluedutdeiveekvdet jedvnecuffhomhgrihhnpeefvheglhdrohhrghenucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehrohgssegsohhtthhlvggurdgtohguvghspdhn sggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: ifab94697:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 15BC415A0092; Mon, 5 Aug 2024 15:37:52 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net x-ms-reactions: disallow MIME-Version: 1.0 Date: Mon, 05 Aug 2024 21:37:32 +0200 To: internals@lists.php.net Message-ID: In-Reply-To: References: <1a88918e-e808-d778-45e1-53797660e093@php.net> <3563cf9b-8eab-4c82-b525-a5d2f9a767bb@varteg.nz> <38920A4B-790D-48C7-B2F6-C49D3F506232@rwec.co.uk> <0824789d-0e36-4628-85c1-4b8d9b7f86af@varteg.nz> <2244a37f-8c51-448d-8a56-329ff32e6470@bastelstu.be> <0e1a21ddef3c7da17a3539b92d5f442763f4b1f8.camel@ageofdream.com> <5f1da5195e2aea7ec41e95ff4354f50b1717d3cf.camel@ageofdream.com> <599a445530357d080cb59c9443cacd5cfd5da82f.camel@ageofdream.com> <896ef99559e4e71d8f1c3abc47415b37ae325e55.camel@ageofdream.com> Subject: Re: [PHP-DEV] [RFC] Add Directive to Make All Namespaced Function Calls Global Content-Type: multipart/alternative; boundary=af236a6c014348419060d1093b694707 From: rob@bottled.codes ("Rob Landers") --af236a6c014348419060d1093b694707 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Mon, Aug 5, 2024, at 20:49, Ilija Tovilo wrote: > On Mon, Aug 5, 2024 at 3:33=E2=80=AFPM Nick Lockheart wrote: > > > > This is a different problem that could be solved by a sandbox API. >=20 > Not sure which case we were talking about then. ClockMock is what I've > been referencing all along. >=20 > > > Well, ok. But then we're back to prefixing global calls, which > > > defeats the purpose of the proposal. > > > > Global functions would only need a prefix `\` in the *very rare* cas= es > > where local functions are set as the default. For most people, the \ > > would be omitted, as globals would be set as default for unqualified > > function names. >=20 > Right. But apart from mocking, what are these cases? If performance > were no longer an issue, "using global functions" just makes the > language harder to use, removing the local fallback. "using local > functions" may be useful in namespaced code making many calls to > functions within the same namespace. In that case, it would probably > be more useful to switch the lookup order back instead. If you want to > pay zero performance penalty, you can prefix global calls with \. > You'd need to do that with "using local functions" anyway. So. Fun story. I=E2=80=99ve seen this technique used to patch out fgetcs= v due to a memory leak, with a pure php polyfill, in at least four unrel= ated codebases. I believe the leak is still there too and now that I kno= w so much more about zend strings, I can probably guess what the issue i= s as well. I digress. The point is, there are code bases that use this technique to= get around php issues or even =E2=80=9Cimplement older versions=E2=80=9D= of core functions to retain backwards compatibility until the code can = be updated to deal with the new core version. >=20 > As for mocking: If the code needs to change either way, why not make > it testable in the first place, e.g. through dependency injection for > time()? At least this only requires changing the calls that are > mocked, instead of all the calls that aren't. Have you ever worked on some legacy code where you aren=E2=80=99t really= sure how it is working in the first place? Even something as simple as = shimming out time() could cause race conditions in the overall system. R= efactoring these systems is an art form all by itself and you attempt to= add tests to understand the system end-to-end, long before you ever cha= nge a line of production code.=20 >=20 > The main benefit of the approach from ClockMock is that your code > (probably) doesn't need to change. I do think that the entire approach > is hacky, and probably worth solving on a language-level, at least if > possible without adding limitations to the engine. A good first start > would be to know what functions are commonly mocked with this > approach. >=20 > Ilija >=20 Time functions are the most obvious ones, and then any function that cha= nges between versions and breaks something would be the non-obvious ones= . (Ex: counting null: https://3v4l.org/hmNiL) This allows you to upgrade= php / support higher versions while slowly upgrading core function call= s. =E2=80=94 Rob --af236a6c014348419060d1093b694707 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable

=
On Mon, Aug 5, 2024, at 20:49, Ilija Tovilo wrote:
On Mon, Aug 5, 20= 24 at 3:33=E2=80=AFPM Nick Lockheart <lists@ageofdream.com> wrote:
>
> This is a different problem that could be solved by a sandbox= API.

Not sure which case we were talking a= bout then. ClockMock is what I've
been referencing all alo= ng.

> > Well, ok. But then we're back= to prefixing global calls, which
> > defeats the pu= rpose of the proposal.
>
> Global func= tions would only need a prefix `\` in the *very rare* cases
> where local functions are set as the default. For most people, th= e \
> would be omitted, as globals would be set as defa= ult for unqualified
> function names.
Right. But apart from mocking, what are these cases? If perf= ormance
were no longer an issue, "using global functions" = just makes the
language harder to use, removing the local = fallback. "using local
functions" may be useful in namespa= ced code making many calls to
functions within the same na= mespace. In that case, it would probably
be more useful to= switch the lookup order back instead. If you want to
pay = zero performance penalty, you can prefix global calls with \.
<= div>You'd need to do that with "using local functions" anyway.
=

So. Fun story. I=E2=80=99ve seen this t= echnique used to patch out fgetcsv due to a memory leak, with a pure php= polyfill, in at least four unrelated codebases. I believe the leak is s= till there too and now that I know so much more about zend strings, I ca= n probably guess what the issue is as well.

I digress. The point is, there are code bases that use this technique t= o get around php issues or even =E2=80=9Cimplement older versions=E2=80=9D= of core functions to retain backwards compatibility until the code can = be updated to deal with the new core version.


As for mock= ing: If the code needs to change either way, why not make
= it testable in the first place, e.g. through dependency injection for
time()? At least this only requires changing the calls that = are
mocked, instead of all the calls that aren't.

Have you ever worked on some legacy co= de where you aren=E2=80=99t really sure how it is working in the first p= lace? Even something as simple as shimming out time() could cause race c= onditions in the overall system. Refactoring these systems is an art for= m all by itself and you attempt to add tests to understand the system en= d-to-end, long before you ever change a line of production code. 

<= br>
The main benefit of the approach from ClockMock is that yo= ur code
(probably) doesn't need to change. I do think that= the entire approach
is hacky, and probably worth solving = on a language-level, at least if
possible without adding l= imitations to the engine. A good first start
would be to k= now what functions are commonly mocked with this
approach.=

Ilija


Time functions are the most obvious ones, and then = any function that changes between versions and breaks something would be= the non-obvious ones. (Ex: counting null: https://3v4l.org/hmNiL) This allows you to upgrade php / = support higher versions while slowly upgrading core function calls.
<= /div>

=E2=80=94 Rob
--af236a6c014348419060d1093b694707--