(TL;DR; Down the thread a bit I put together a concrete example of why I'm opposed to this RFC)
On Aug 26 2024, at 12:44 am, Mike Schinkel <[email protected]> wrote:
> Speaking only to #1 for the moment, there are many different places in PHP where certain
> expressions make no sense either, yet I do not see those objecting calling the question for those
> other scenarios, and I ask myself "Why not?"
>
> One obvious answer is "because those are status quo and this is not" but I also maybe
> because the many non-sensical things that can be done elsewhere are, in practice, not a problem as
> nobody ever does them. And why not? Because...they are nonsensical. Given this the arguments against
> feel to me to be rather bikesheddy. But I will come back to #1.
The proposal in the RFC creates a new dependency and backward compatibility issue for API developers
that currently does not exist. It is not just because it allows for non-sensical expressions, but
that it allows perfectly sensical expressions that would create dependencies between libraries that
I don't think are a worthwhile tradeoff. See my code example below.
> Moving on to #2 I respect the argument in theory. But as we are constantly told the RFC
> author's burden is to prove the necessity, it seems only reasonable and fair to expect that —
> when many people see the utility of and support an RFC — that those who object to it should
> provide some concrete examples of how their concerns would manifest problems rather than just say
> "it might allow something bad."
Consider this metaphor -- If I have a object with private properties, PHP doesn't allow me to
reach into that object and extract those values without a lot of work (e.g. Reflection) by design.
Right now, there are lots of libraries out there defining default values and right now today they
are in the same sense "private" to the function/method they are defined for. This PR would
change the visibility of those default values, pulling them higher up into the call stack where IMO
they don't belong -- essentially making them a brand new dependency API devs need to worry
about.
> I have looked for but yet not seen any examples of how effectively publishing a default value
> could actually cause a significant problem — and this is the important part — in a real-world
> scenario and not just a contrived one.
> Sure if I have foo(int $index=1), a developer calls with
> foo(default*3), and then the author of foo changes the signature to foo(int
> $index=0) that might cause problems, but what is a real world scenario where a developer
> would actually do that, the author then change it, and then is causes a non-trivial problem?
How is that not a real-world contrived scenario? That is the problem. It's that some library
developer decided to change the default value for their own purposes and now any code that was
relying on that default value has broken. I don't think anyone needs to go hunting around
GitHub or something to prove that people change default values between library releases, nor that
this RFC would introduce a new dependency between upstream APIs and the code that uses them to
manage. See below for a fairly reasonable example I whipped up.
> 3. "How likely are people to accidentally do that stupid thing?"
Experience has shown us time and time again that people will use the features that get implemented,
and once they exist we are stuck with the consequences. Developers who consume APIs are rarely
worrying about the developers who wrote those APIs. This is making it not only very easy to do a
stupid thing, it's making the fact people are doing a stupid thing the problem of an entirely
different project/developer who now has to deal with the reality any time they touch a default value
they have to worry about the downstream impacts, being granted zero control to prevent it from
happening. It's also something that would be hard for downstream developers to even realize was
a problem...
Library dev: "BC break, I changed this default value"
Downstream dev: "Okay.... so how do I fix this code someone else wrote 2 years ago so I can
upgrade?"
Library dev: "I dunno, depends on how you used default so you're on your own. I guess you
better grep and hope you catch them all"
> But if you disallow default!=5, then you (likely?) also disallow default!=5
> ? default : 0 and any other *sensical* expression with that sub-expression. Do we really want
> to flesh out all the potential nonsensical expressions for a given use-case and build a custom
> parser for this one RFC?
As I previously said, I don't think this would be addressed in the parser -- I think it'd
have to be a runtime check (if such a check were to exist).
That aside, no I don't really want to flesh out all the potential expressions to build those
rules into the engine. I believe if we are going to allow expressions for default values as proposed
we _need_ to build those rules, but given the circumstances I think it's more likely the RFC
needs to be reconsidered, updated to reflect feedback, or withdrawn.
> Let me propose this example and see if you still hold firm to your option that the following
> expression would not be valid and that it still would not be a good idea for the language:
> class MyDependency {...}
> function doSomething(MyDependency $dep= new MyDependency) {...}
> doSomething((default)->WithLogger(new Logger));
Let's make that a little more complicated so you'll see the problem -- Consider this
rather lengthy example building off the concept of your example:
<?php
enum LoggerType: string
{
case DB = 'db';
case FILE = 'file';
case CLOUD = 'cloud';
public function getLogger(): LoggerInterface
{
return match($this)
{
static::DB => new DatabaseLogger(),
static::FILE => new FileLogger(),
static::CLOUD => new CloudLogger()
};
}
}
interface LoggerInterface
{
public function log(string $x);
}
// Assume DatabaseLogger, etc. exist and implement LoggerInterface
class A
{
protected ?LoggerInterface $log;
public function withLogger(LoggerInterface|LoggerType $a = new DatabaseLogger): static
{
if($a instanceof LoggerInterface)
{
$this->log = $a;
return $this;
}
$this->log = $a->getLogger();
return $this;
}
}
(new A)->withLogger((default)->log('B'));
This would be valid under this RFC, right? But now as the author of class A I later want to change
the default of my withLogger method. Instead of just passing in new DatabaseLogger, I now want to
change my API default to just a LoggerType::DB enum for reasons (What the reasons aren't
relevant).
Today I don't have to think too hard about that change from an API BC perspective because the
consumer has either passed in a LoggerInterface or a LoggerType -- or left it empty and used
it's default value. I can just change the default to LoggerType::DB and be on my way. The
downstream developer will never know or care that I did it because if they wanted to call log() they
had to first create their own instance of LoggerInterface and have a reference to that object in
their local context like so:
$logger = LoggerType::DB->getLogger();
(new A)->withLogger($logger);
$logger->log('B');
With this RFC, now I can't change this API call without introducing a BC break in my library
because I have no idea at this point if some downstream caller decided to use my default value
directly or not.
You can argue if this is a good API design or not, but it was only written to provide a real example
of how pulling the default value higher up the call chain and allowing it to be used in expressions
is problematic for library authors all to save a couple of lines of code on the consumer side.
> > On Aug 25, 2024, at 12:23 PM, John Coggeshall <[email protected]> wrote:
> > I won't vote for this RFC if the above code is valid, FWIW. Unlike include , default
> > is a special-case with a very specific purpose -- one that is reaching into someone else's API
> > in a way the developer of that library doesn't explicitly permit.
> Ok, so if a developer of the API currently wants to indicate that other developers CAN
> explicitly reach in then how would they do that, currently?
I'm honestly not sure what you're asking here. PHP currently doesn't allow you access
to the "default value" of a function you are calling (maybe Reflection? I don't know
offhand).
> Your argument seems to be it should implicitly be disallowed, but I do not see any way in which
> you are proposing a developer could explicitly empower a developer to allow it. At least not without
> a ton of boilerplate which, for each default value, turns into a lot of extra code to create and
> then maintain.
> It seems one-sided to argue against allowing something that "has not been explicitly
> allowed" if there is no way to explicitly allow it. And I get you may say "well
> that's not my job" to which —if you did — I would ask "Do you really only want to
> be the one who brings the problem but not the solution?"
Right now there isn't a problem. There is nothing lacking in the current language that prevents
me from providing my own LoggerInterface or LoggerType to the withLogger above. This is a suggestion
to expand the syntax of the language to simplify something, not enable it to exist where before it
didn't. The RFC would introduce something that makes it marginally easier for a downstream
consumer of the API, at the cost of making life painful for library authors and worse not providing
any way to prevent it from happening. So yes, I am pointing out a problem but not providing a
solution because I don't currently agree a solution is even needed.
Not all ideas make the cut the first time, and that's okay. Those of us who have stood up
against this idea in its current form have tried to offer our best thoughts as to how you might make
it work by outlining that we think there need to be rules around the types of expressions that would
be allowed... these opinions don't come out of nowhere or meant to be obstructionist -- still,
they have largely been dismissed with strawman counters about how include statements let you do
silly things, or responses like "The truth always lies in the code, which is why RFC authors
are strongly encouraged to pursue patches where there is doubt, and similarly, I think
counter-proposals on the mailing list should follow suit, otherwise we can find ourselves arguing
over nothing." -- I don't agree with that, or feel obligated to attempt to write patches
for an idea I don't think is necessary in the first place.
John