Re: [PHP-DEV] Revisiting Userland Operator Overloads

2021-08-06 Thread Scott Arciszewski
My recommendation is to have a base interface for every operator that can
be overloaded, so that these can be composed together.

AddOverloadInterface { __overloadAdd(): self; }
SubOverloadInterface { __overloadSub(): self; }
MulOverloadInterface { __overloadMul(): self; }
DivOverloadInterface { __overloadDiv(): self; }
ModOverloadInterface { __overloadMod(): self; }
LeftShiftOverloadInterface { __overloadLeftShift(): self; }
RightShiftOverloadInterface { __overloadRightShift(): self; }
XorOverloadInterface { __overloadXor(): self; }
etc.

Then if you were implementing matrix math, you could do this:

MatrixMath implements AddOverloadInterface, SubOverloadInterface,
MulOverloadInterface {

}

This prevents you from having to define methods for operations you don't
support.

It's probably worth exploring whether common combinations are worth
defining for convenience.

On Fri, Aug 6, 2021 at 1:49 PM Larry Garfield 
wrote:

> On Fri, Aug 6, 2021, at 11:34 AM, Jordan LeDoux wrote:
> > Hey all,
> >
> > I contacted Jan a few days ago to ask if they were going to try again for
> > their RFC, but I wanted to get a quick temperature check on this.
> >
> > I would like to work on this RFC for 8.2, and after going through
> previous
> > discussions on the topic, I feel like the right place to start is to
> limit
> > the RFC to only the mathematical operators at first.
> >
> > Based on previous comments, I was considering working from Jan's previous
> > implementation with these basic changes:
> >
> > 1. The only supported operators in the RFC will be the mathematical
> > operators: (+, -, /, *, **, %)
> > 2. The RFC would also provide an interface, (something like
> > MathObjectInterface), that has all the magic methods in it.
> > 3. The do_operation would be changed to check for the interface being
> > implemented instead of the specific method.
> >
> > All of these operators belong to the same "group" of changes, and
> (broadly)
> > any object that is valid to override for one of them should be valid to
> > override for any of them. NOTE: There are edge cases of this statement
> > certainly.
> >
> > This would help address some of the concerns that were expressed about
> > misuse of things like the + operator to add things to a cache. It would
> > more explicitly take a position on how these operators should be used in
> > userland code to keep them more consistent with the behavior of these
> > operators in normal PHP code.
> >
> > This would be my first contribution and my first RFC, so I wanted to get
> > some very broad feedback on this before diving in.
> >
> > It would suggest that future sets of operators could come with their own
> > interfaces to also attempt to guarantee an all-or-nothing approach in
> > userland code, (BitwiseObjectInterface, ComparableObjectInterface,
> > LogicalObjectInterface, etc.). Though decisions on any of those operators
> > or their implementations would be left as future scope.
> >
> > Jordan
>
> If you wanted to cluster them, I would strongly recommend much smaller
> groupings.  IMO, adding two collections to get another collection -- like
> we add/merge arrays -- is a completely reasonable thing to do, but would
> only use + and potentially - (for removing items from a collection and
> returning the smaller collection). But * and / make no sense in that
> context.
>
> Even if you confine yourself to mathematical concepts, matrix
> multiplication is a well-established thing that would make sense for
> math-centric applications to do.  But matrix division is... somewhat more
> fiddly, and has multiple possible definitions.  (Note: I've not done matrix
> math in over 20 years, so I'm basing that statement on some quick googling.)
>
> So I don't think force-grouping the overloads makes sense, in practice.
> I'm still open to operator overloading in general, but I don't think the
> clustering is going to help.  The weird and complicated type interactions,
> particularly when it comes to commutablity, are a much larger issue, IMO.
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Kamil Tekiela
Hi Internals,

Thanks for all the feedback. I have changed the name to http_parse_query as
it looks like more people prefer that name. I have also updated
https://wiki.php.net/rfc/parse_str_alternative for 8.2 (sorry for the
confusion) and I added open points.
I hear two concerns currently about this RFC. Please note that my intention
was to provide a simple drop-in replacement. However, I agree that there is
a lot of room for improvement in this area, so I highly appreciate your
comments on how we can best tackle these open points.
I will not open this RFC for voting any time soon, as I want to let the
conversation run through to see what other options/concerns people have.
Please feel free to share your comments on what you think is the right path
to improve this functionality in PHP.

Regards,
Kamil


Re: [PHP-DEV] Revisiting Userland Operator Overloads

2021-08-06 Thread Larry Garfield
On Fri, Aug 6, 2021, at 11:34 AM, Jordan LeDoux wrote:
> Hey all,
> 
> I contacted Jan a few days ago to ask if they were going to try again for
> their RFC, but I wanted to get a quick temperature check on this.
> 
> I would like to work on this RFC for 8.2, and after going through previous
> discussions on the topic, I feel like the right place to start is to limit
> the RFC to only the mathematical operators at first.
> 
> Based on previous comments, I was considering working from Jan's previous
> implementation with these basic changes:
> 
> 1. The only supported operators in the RFC will be the mathematical
> operators: (+, -, /, *, **, %)
> 2. The RFC would also provide an interface, (something like
> MathObjectInterface), that has all the magic methods in it.
> 3. The do_operation would be changed to check for the interface being
> implemented instead of the specific method.
> 
> All of these operators belong to the same "group" of changes, and (broadly)
> any object that is valid to override for one of them should be valid to
> override for any of them. NOTE: There are edge cases of this statement
> certainly.
> 
> This would help address some of the concerns that were expressed about
> misuse of things like the + operator to add things to a cache. It would
> more explicitly take a position on how these operators should be used in
> userland code to keep them more consistent with the behavior of these
> operators in normal PHP code.
> 
> This would be my first contribution and my first RFC, so I wanted to get
> some very broad feedback on this before diving in.
> 
> It would suggest that future sets of operators could come with their own
> interfaces to also attempt to guarantee an all-or-nothing approach in
> userland code, (BitwiseObjectInterface, ComparableObjectInterface,
> LogicalObjectInterface, etc.). Though decisions on any of those operators
> or their implementations would be left as future scope.
> 
> Jordan

If you wanted to cluster them, I would strongly recommend much smaller 
groupings.  IMO, adding two collections to get another collection -- like we 
add/merge arrays -- is a completely reasonable thing to do, but would only use 
+ and potentially - (for removing items from a collection and returning the 
smaller collection). But * and / make no sense in that context.

Even if you confine yourself to mathematical concepts, matrix multiplication is 
a well-established thing that would make sense for math-centric applications to 
do.  But matrix division is... somewhat more fiddly, and has multiple possible 
definitions.  (Note: I've not done matrix math in over 20 years, so I'm basing 
that statement on some quick googling.)

So I don't think force-grouping the overloads makes sense, in practice.  I'm 
still open to operator overloading in general, but I don't think the clustering 
is going to help.  The weird and complicated type interactions, particularly 
when it comes to commutablity, are a much larger issue, IMO.

--Larry Garfield

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



[PHP-DEV] Revisiting Userland Operator Overloads

2021-08-06 Thread Jordan LeDoux
Hey all,

I contacted Jan a few days ago to ask if they were going to try again for
their RFC, but I wanted to get a quick temperature check on this.

I would like to work on this RFC for 8.2, and after going through previous
discussions on the topic, I feel like the right place to start is to limit
the RFC to only the mathematical operators at first.

Based on previous comments, I was considering working from Jan's previous
implementation with these basic changes:

1. The only supported operators in the RFC will be the mathematical
operators: (+, -, /, *, **, %)
2. The RFC would also provide an interface, (something like
MathObjectInterface), that has all the magic methods in it.
3. The do_operation would be changed to check for the interface being
implemented instead of the specific method.

All of these operators belong to the same "group" of changes, and (broadly)
any object that is valid to override for one of them should be valid to
override for any of them. NOTE: There are edge cases of this statement
certainly.

This would help address some of the concerns that were expressed about
misuse of things like the + operator to add things to a cache. It would
more explicitly take a position on how these operators should be used in
userland code to keep them more consistent with the behavior of these
operators in normal PHP code.

This would be my first contribution and my first RFC, so I wanted to get
some very broad feedback on this before diving in.

It would suggest that future sets of operators could come with their own
interfaces to also attempt to guarantee an all-or-nothing approach in
userland code, (BitwiseObjectInterface, ComparableObjectInterface,
LogicalObjectInterface, etc.). Though decisions on any of those operators
or their implementations would be left as future scope.

Jordan


Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Rowan Tommins

On 06/08/2021 16:06, Larry Garfield wrote:

Give me a properly defined HttpQuery object with named, type-enforced 
properties and meaningful methods.  Give it a parse(string) static method and a 
__toString() method to convert back to a query string.  Anything less is, 
frankly, not worth the effort.



While I understand your general principle, I'm not sure what such an 
object would look like. There is no "standard list" of query string keys 
which could be named as properties on a built-in HttpQuery object; every 
request is different, so the user would have to provide the object to be 
"hydrated" in some form.


Maybe it would have to be a trait, so you could write something like this?

class GetUserQueryParams {
    use HttpQueryParser;
    private ?int $id;
    private ?string $name;
}
$params = GetUserQueryParam::fromQueryString('?id=42');

Or a utility method that took a class name, and converted the query 
string parameters to named constructor arguments?


class GetUserQueryParams {
    public function __construct(
       private ?int $id,
       private ?string $name
   ) {}
}
$params = parse_query_string('?id=42', GetUserQueryParams::class);


Once you've started down that route, people will want a way to alias 
property names (e.g. with attributes), have parallel handling for other 
formats like JSON ... and before you know it you have the kind of 
complexity which is much easier to manage as a userland library than 
something tied to core.


On the other hand, if core provides a sane implementation of the parsing 
into a generic "bag of key-value pairs" (which PHP calls an "array"), 
that would provide a very useful building block for any userland library 
to build on.



Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative toparse_str

2021-08-06 Thread Christoph M. Becker
On 06.08.2021 at 15:20, Rowan Tommins wrote:

> On 06/08/2021 12:12, Kamil Tekiela wrote:
>
>> Perhaps, instead of adjusting this behaviour only for the new
>> function, we
>> could remove this behaviour as a whole, given that it is a remainder
>> of the
>> long-forgotten register globals? I don't see any use for it anymore
>
> Changing behaviour like this is tricky, because even if it's not that
> useful, it is consistent, so code to process a query string of
> "?foo.bar=42" will be looking at $_GET['foo_bar'] not $_GET['foo.bar'].
> If we "fix" the name mangling, that code will break, and not necessarily
> in a very obvious way.
>
> Note that there are other parts of the request which get mangled in
> similar ways, such as when HTTP headers are translated into CGI
> environment variables. The solution in that case is to ignore the CGI
> vars and use a function like getallheaders(), so having a non-mangling
> http_parse_query() doesn't feel out of place.

Right.  *If* we want to change this name mangling, we should change it
everywhere, and that certainly needs an own RFC and ideally some kind of
deprecation/change notice.

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Christoph M. Becker
On 06.08.2021 at 16:09, Scott Arciszewski wrote:

> http_parse_query() would get my vote (if I had vote karma :P)

You have a php.net account[1], so you are supposed to have voting karma. :)

[1] 

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Larry Garfield
On Thu, Aug 5, 2021, at 5:21 PM, Kamil Tekiela wrote:
> Hi Internals,
> 
> I have added implementation for
> https://wiki.php.net/rfc/parse_str_alternative. If there are no other
> comments, I would like to start voting on Saturday.
> 
> Regards,
> Kamil

I will be voting No on this as is.

Not because I'm against improving the query parsing tools; I'm in favor of 
that.  But as noted, this RFC doesn't do enough to improve it.  Returning a 
value and changing the name isn't enough to justify another global method.  As 
others noted, the parsing rules themselves are fugly and need to be improved.

More importantly to me, though, it is the year of our lord 2021, and PHP APIs 
have no business pretending that arrays with possibly-missing values are a data 
structure.  They're a satire of a data structure.

If you're parsing a string with known structure into something, that something 
should be a properly defined object.  The new PhpToken is a good example of 
that shift done well.  Don't give me an array where I have to mess around with 
isset() and crap like that.  

Give me a properly defined HttpQuery object with named, type-enforced 
properties and meaningful methods.  Give it a parse(string) static method and a 
__toString() method to convert back to a query string.  Anything less is, 
frankly, not worth the effort.

--Larry Garfield

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Scott Arciszewski
http_parse_query() would get my vote (if I had vote karma :P)

On Fri, Aug 6, 2021 at 4:05 AM Mike Schinkel  wrote:

> > On Aug 6, 2021, at 3:51 AM, Aleksander Machniak  wrote:
> >
> > I agree about the _string suffix removal. However, I know we have
> > parse_url() already, but parse_query() might be too generic. I would
> > suggest adding "http" to the name. And as we already have
> > http_build_query() I would rather see http_parse_query().
>
> +1.  http_parse_query() is even better.
>
> -Mike
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Rowan Tommins

On 06/08/2021 12:12, Kamil Tekiela wrote:

Perhaps, instead of adjusting this behaviour only for the new function, we
could remove this behaviour as a whole, given that it is a remainder of the
long-forgotten register globals? I don't see any use for it anymore



Changing behaviour like this is tricky, because even if it's not that 
useful, it is consistent, so code to process a query string of 
"?foo.bar=42" will be looking at $_GET['foo_bar'] not $_GET['foo.bar']. 
If we "fix" the name mangling, that code will break, and not necessarily 
in a very obvious way.


Note that there are other parts of the request which get mangled in 
similar ways, such as when HTTP headers are translated into CGI 
environment variables. The solution in that case is to ignore the CGI 
vars and use a function like getallheaders(), so having a non-mangling 
http_parse_query() doesn't feel out of place.


Having the function mirror http_build_query() as closely as possible 
seems like a good idea, e.g. http_parse_query('foo_0=a;foo_1=b', 'foo_', 
';') should return [0=>'a', 1=>'b'].


Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Kamil Tekiela
btw why isn't foo.bar=123 decoded to array("foo.bar"=>123);  ?
> this looks pretty bad to me https://3v4l.org/6Wa23


Hi Hans,

This is because variables in PHP that contain the concatenation operator or
space are much more difficult to access. See https://3v4l.org/vUBWK
As the primary purpose of parse_str was to register globals from a query
string, it follows the same logic as your normal POST/GET/COOKIE parsing.
See https://stackoverflow.com/a/68742/1839439
Perhaps, instead of adjusting this behaviour only for the new function, we
could remove this behaviour as a whole, given that it is a remainder of the
long-forgotten register globals? I don't see any use for it anymore, as all
parsed input is only available in the form of an associative array. People
who extract $_POST can suffer the consequences of their own actions...

Regards,
Kamil


Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Hans Henrik Bergan
btw why isn't foo.bar=123 decoded to array("foo.bar"=>123);  ?
this looks pretty bad to me https://3v4l.org/6Wa23

On Fri, 6 Aug 2021 at 10:29, Peter Bowyer  wrote:

> On Fri, 6 Aug 2021 at 08:18, ignace nyamagana butera 
> wrote:
>
> > I read your RFC and I understand the intent in improving the current
> > parse_str function behaviour by introducing a new function to
> > avoid possible breakage,
> > However I feel that we are missing a chance to also improve how parse_str
> > algorithm is currently used, we could or should (?) use this opportunity
> > to fix some long shortcomings around parse_str.
> >
> > In no particular order:
> >
>
> I agree that the first 2 shortcomings ought to be addressed in a
> HTTP-focused parse_str alternative. The first has been an annoyance, as
> some querystrings use '.' to denote an array, in the way PHP chooses []. So
> foo.bar would mean foo[bar] in PHP-speak.
>
> The third I won't comment on as I don't know the Url algorithm.
>
> Peter
>


Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Ayesh Karunaratne
> I agree about the _string suffix removal. However, I know we have
> parse_url() already, but parse_query() might be too generic. I would
> suggest adding "http" to the name. And as we already have
> http_build_query() I would rather see http_parse_query().
>

+1 for http_parse_query() as it sounds a lot more natural and consistent.

> parse_str assumes that the query separator is always a "&" which reduces
> its usage to only parsing query using that particular character. Again this
> might be seen as an edge case but no RFC prevents using any other
> character. If you where to use another character you are bound to use some
> userland code workaround to then feed the "normalized" string to parse_str

Both `http_build_query` and `parse_str` respect the
`arg_separator.input` [INI
setting](https://www.php.net/manual/en/ini.core.php#ini.arg-separator.input).
If we were to go for a `http_parse_query`, I think it makes sense to
support the same behavior and parameters.

Thank you,
Ayesh.

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Peter Bowyer
On Fri, 6 Aug 2021 at 08:18, ignace nyamagana butera 
wrote:

> I read your RFC and I understand the intent in improving the current
> parse_str function behaviour by introducing a new function to
> avoid possible breakage,
> However I feel that we are missing a chance to also improve how parse_str
> algorithm is currently used, we could or should (?) use this opportunity
> to fix some long shortcomings around parse_str.
>
> In no particular order:
>

I agree that the first 2 shortcomings ought to be addressed in a
HTTP-focused parse_str alternative. The first has been an annoyance, as
some querystrings use '.' to denote an array, in the way PHP chooses []. So
foo.bar would mean foo[bar] in PHP-speak.

The third I won't comment on as I don't know the Url algorithm.

Peter


Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Mike Schinkel
> On Aug 6, 2021, at 3:51 AM, Aleksander Machniak  wrote:
> 
> I agree about the _string suffix removal. However, I know we have
> parse_url() already, but parse_query() might be too generic. I would
> suggest adding "http" to the name. And as we already have
> http_build_query() I would rather see http_parse_query().

+1.  http_parse_query() is even better. 

-Mike

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Aleksander Machniak
On 06.08.2021 09:45, Mike Schinkel wrote:
> I too would appreciate having a function in the PHP library that returns an 
> array and that is named more intuitively than parse_str().
> 
> However, I would suggest naming it `parse_query()` instead of 
> `parse_query_string()` as `_string()` is redundant and I see shorter function 
> names being preferable when the intent of the function is clear.

I agree about the _string suffix removal. However, I know we have
parse_url() already, but parse_query() might be too generic. I would
suggest adding "http" to the name. And as we already have
http_build_query() I would rather see http_parse_query().

The RFC should target 8.2.

-- 
Aleksander Machniak
Kolab Groupware Developer[https://kolab.org]
Roundcube Webmail Developer  [https://roundcube.net]

PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread Mike Schinkel
> On Aug 5, 2021, at 6:21 PM, Kamil Tekiela  wrote:
> 
> I have added implementation for
> https://wiki.php.net/rfc/parse_str_alternative. If there are no other
> comments, I would like to start voting on Saturday.


I too would appreciate having a function in the PHP library that returns an 
array and that is named more intuitively than parse_str().

However, I would suggest naming it `parse_query()` instead of 
`parse_query_string()` as `_string()` is redundant and I see shorter function 
names being preferable when the intent of the function is clear.

I searched for prior art and it appears that Guzzle's PSR7 helper library v1.x 
had a namespaced parse_query() function:

https://github.com/guzzle/psr7/blob/1.x/src/functions.php#L299 


I found Psr7\parse_query() being called on GitHub in 840 places using 
SourceGraph code search:

https://sourcegraph.com/search?q=context:global+lang:php+Psr7%5Cparse_query%28+count:all=literal
 


There was only one (1) place with SourceGraph code search where someone named a 
function parse_query_string(), and that example did not use 
parse_query_string() in the same way as your RFC:

https://sourcegraph.com/github.com/k0a1a/hotglue2/-/blob/controller.inc.php?L344:1
 


My takeaway is that PHP developers will easily be able to understand the intent 
if named `parse_query()`, especially those who might be familiar with it from 
Guzzle, and thus there is no need for the redundant `_string()`.  

And, of course, a parse_query() in the global namespace won't conflict with 
Guzzle's use because their function is namespaced with Guzzle\Psr7.

> On Aug 6, 2021, at 3:17 AM, ignace nyamagana butera  
> wrote:
> 
> I feel that we are missing a chance to also improve how parse_str
> algorithm is currently used, we could or should (?) use this opportunity
> to fix some long shortcomings around parse_str.

Also, +1 to this.

-Mike

P.S. WordPress has a parse_query() *method* on their WP_Query() class, but as 
someone who has worked with WordPress for 10+ years even I don't see these two 
in conflict. One is a method scoped to the solution domain of its class and the 
other would be a function in PHP's global namespace.

[PHP-DEV] [RFC] Add parse_query_string as an alternative to parse_str

2021-08-06 Thread ignace nyamagana butera
Hi Kamil Tekiela,

I read your RFC and I understand the intent in improving the current
parse_str function behaviour by introducing a new function to
avoid possible breakage,
However I feel that we are missing a chance to also improve how parse_str
algorithm is currently used, we could or should (?) use this opportunity
to fix some long shortcomings around parse_str.

In no particular order:

parse_str mangled data while it was acceptable before when it could
directly inject PHP variables into the current scope, this
feature is no longer needed and prevents interoperability with other query
parsing algorithms used in other languages. Also since this mangled data
is not well known it comes as a surprise to average PHP developer as the
behaviour feels unexpected see https://3v4l.org/KIJ9V

parse_str assumes that the query separator is always a "&" which reduces
its usage to only parsing query using that particular character. Again this
might be seen as an edge case but no RFC prevents using any other
character. If you where to use another character you are bound to use some
userland code workaround to then feed the "normalized" string to parse_str

parse_str parse the query string using PHP algorithm while there are now
some established parsing algorithm that are languages independant like
https://url.spec.whatwg.org/#interface-urlsearchparams that defines how a
query string should be parsed.

If we were to introduce your function as is, I feel that we will have to
submit yet another RFC and have a language with at least two ways to parse
a string. So maybe instead of a new function what we need is an object with
a better public API ?  thoughts