Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:129606
Precedence: list
MIME-Version: 1.0
References: <CAH5C8xXUPKgd0wWwLC3G2Ye=kRZN58gK3DY82v5GoduBNNtb5A@mail.gmail.com>
 <83238ad3-c844-4457-dfb3-11321787e022@php.net>
In-Reply-To: <83238ad3-c844-4457-dfb3-11321787e022@php.net>
Date: Sat, 13 Dec 2025 22:48:34 +0100
Message-ID: <CAH5C8xUOEkfQ6u8AxuV8UGjWU0h7goDsvWa9QQmANhNdU=Dwhw@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Followup Improvements for ext/uri
To: Derick Rethans <derick@php.net>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000a622070645dc57a8"
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--000000000000a622070645dc57a8
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Derick,


> Would it make sense to have an interface for the set*() methods? Besides
> build(), they all seem to have the same API.
>

I've just answered this to Juris, but TLDR: IMO it is not possible if we go
with the currently suggested behavior.
The set*() method pairs would have completely different validation rules
based on the specification they implement.


> > - Query Parameter Manipulation
>
> I see this adds NoDiscard:
> #[\NoDiscard(message: "as Uri\Rfc3986\Uri::withQueryParams() does not
> modify the object itself")]
>
> But the original methods on the classes don't have these NoDiscards, and
> it doesn't seem that this RFC is suggesting to add them. It should at
> least be consistent.
>

Thanks for noticing these; I wanted to add #[NoDiscard] to Uri/Url wither
methods just before the final PHP 8.5 release,
but it was a bit controversial, so I didn't do it at last. Then I thought
that at least the newly proposed methods could
have it, that's why I included these attributes into this RFC. But now I
realized that I'm not ready to fight for them,
so I've just removed them all from the proposal.


> > - Accessing Path Segments as an Array
>
> Compare:
>
> "especially considering the fact that Uri\Rfc3986\Uri internally stores
> the path as a list of segments."
>
> And:
>
> Uri\Rfc3986\Uri::withPathSegments() =E2=80=A6 internally concatenate the =
input
> segments separated by a / character, and then trigger
> Uri\Rfc3986\Uri::withPath() =E2=80=A6
>
> Why does it need to do this concattenation, and then call withPath() for
> Rfc3986\Uri then?
>

The path setter of the uriparser library (uriSetPathMmA) takes a string
first,
then reparses it as an URI (setters for the other components work much
simpler),
and finally, does a few other checks/modifications. If everything went well=
,
the path is stored as a list of segments indeed. That's why concatenating
the
segments is needed, otherwise validation wouldn't work properly.

Cheers,
M=C3=A1t=C3=A9

--000000000000a622070645dc57a8
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Derick,</div><div class=3D"gmail_quote=
 gmail_quote_container"><div>=C2=A0</div><blockquote class=3D"gmail_quote" =
style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);pa=
dding-left:1ex">
Would it make sense to have an interface for the set*() methods? Besides <b=
r>
build(), they all seem to have the same API.<br></blockquote><div><br></div=
><div>I&#39;ve just answered this to Juris, but TLDR: IMO it is not possibl=
e if we go with the currently suggested behavior.</div><div>The set*() meth=
od pairs would have completely different validation rules based on the spec=
ification they=C2=A0implement.</div><div>=C2=A0</div><blockquote class=3D"g=
mail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204=
,204,204);padding-left:1ex">
&gt; - Query Parameter Manipulation<br>
<br>
I see this adds NoDiscard:<br>
#[\NoDiscard(message: &quot;as Uri\Rfc3986\Uri::withQueryParams() does not =
modify the object itself&quot;)]<br>
<br>
But the original methods on the classes don&#39;t have these NoDiscards, an=
d <br>
it doesn&#39;t seem that this RFC is suggesting to add them. It should at <=
br>
least be consistent.<br></blockquote><div><br></div><div>Thanks for noticin=
g these; I wanted to add #[NoDiscard] to Uri/Url wither methods just before=
 the final PHP 8.5 release,</div><div>but it was a bit controversial,=C2=A0=
so I didn&#39;t do=C2=A0it at last. Then I thought that at least the newly =
proposed methods could</div><div>have it, that&#39;s why I included these a=
ttributes into this RFC. But now I realized that I&#39;m not ready to fight=
 for them,</div><div>so I&#39;ve just removed them all from the proposal.</=
div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px =
0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
&gt; - Accessing Path Segments as an Array<br>
<br>
Compare:<br>
<br>
&quot;especially considering the fact that Uri\Rfc3986\Uri internally store=
s <br>
the path as a list of segments.&quot;<br>
<br>
And:<br>
<br>
Uri\Rfc3986\Uri::withPathSegments() =E2=80=A6 internally concatenate the in=
put <br>
segments separated by a / character, and then trigger <br>
Uri\Rfc3986\Uri::withPath() =E2=80=A6<br>
<br>
Why does it need to do this concattenation, and then call withPath() for <b=
r>
Rfc3986\Uri then?<br></blockquote><div><br></div><div>The path setter of th=
e uriparser library (uriSetPathMmA) takes a string first,</div><div>then re=
parses=C2=A0it as an URI (setters for the other components work much simple=
r),</div><div>and finally, does a=C2=A0few other checks/modifications. If e=
verything went well,</div><div>the path is stored as a list of segments ind=
eed. That&#39;s why concatenating the</div><div>segments is needed, otherwi=
se validation wouldn&#39;t work properly.</div><div><br></div><div>Cheers,<=
br>M=C3=A1t=C3=A9</div></div></div>

--000000000000a622070645dc57a8--