Double ampersand short-circuit not working correctly

[joelmellon]

Psy Shell v0.10.9 (PHP 8.0.5 — cli) by Justin Hileman
>>> function_exists('apcu_enabled')
=> false
>>> function_exists('apcu_enabled') && apcu_enabled()
PHP Fatal error:  Call to undefined function apcu_enabled() in Psy Shell code on line 1
>>> if (function_exists('apcu_enabled') && apcu_enabled()) {echo 'true';}
PHP Fatal error:  Call to undefined function apcu_enabled() in Psy Shell code on line 1
>>> if (function_exists('apcu_enabled') && print('this works as expected')) {echo 'true';} else {echo 'false';}
false
>>>

The apcu_enabled() function should not be called after function_exists('apcu_enabled') returns false. && should short-circuit as soon as the condition preceding it is falsy.

Notice that when the && is followed by print() the print function is not called.

From "Logical Operators" in the PHP manual:

Example # 1 Logical operators illustrated

<?php

// --------------------
// foo() will never get called as those operators are short-circuit

$a = (false && foo());
$b = (true  || foo());
$c = (false and foo());
$d = (true  or  foo());

Notice when run on the command line, there's no error (including the exit code):

# php -r 'var_dump(function_exists("apcu_enabled") && apcu_enabled());'
Command line code:1:
bool(false)
# echo $?
0

But if I try to call it directly, it fatals, which is expected:

# php -r 'var_dump(apcu_enabled());'

Fatal error: Uncaught Error: Call to undefined function apcu_enabled() in Command line code on line 1

Error: Call to undefined function apcu_enabled() in Command line code on line 1

Call Stack:
    0.0001     389728   1. {main}() Command line code:0

Psy Shell's short-circuit behavior is not consistent with PHP-CLI.

Also, when served with php-fpm:

<?php

var_dump(function_exists("apcu_enabled") && apcu_enabled()); // dumps 'false' - doesn't fatal
var_dump(apcu_enabled()); // fatals as expected

Outputs:

This demonstrates that PHP's && short-circuiting prevents undefined methods from being called making this particular use-case safe. Psy Shell doesn't respect this.

Thanks for the amazing tool, by the way. It's been life changing!

[bobthecow]

This isn't a real PHP error, it's PsySH trying to save you from yourself :)

The "code cleaner" in that trace does a lot of things to smooth over the experience of typing code line-by-line in a repl-type environment vs a file. One of the things it does is try to keep you from hitting fatal errors and booting you out of your repl session. In this case, it's (a little too aggressively) trying to keep you from calling a function that doesn't exist.

The awesome thing about it is that this is now a recoverable fatal error, so we can catch it at runtime rather than preventing it!

(Ignore the error handling the error. Nothing to see here 😛)

[joelmellon]

So you're saying it's a feature not a bug? 🤣

a little too aggressively

Right, you've summarized the bug perfectly.

this is now a recoverable fatal error

If short circuit worked, there wouldn't be an error.

It's your code, so it's your call obviously, but it's my job to save myself from me. I've written PHP for ~20 years so as you might expect, I don't need a nanny (most of the time!) I would expect an interpreter consistent with PHP. My code is safe, I did protect myself, and PHP trusts me, why doesn't Psy Shell?

It's obviously not a hugely critical issue, but I wanted to point it out.

Thanks again for all your work on this project, and for taking it over and continuing development on it. No sarcasm or condescension: you're a PHP community hero. 🤘

[bobthecow]

So you're saying it's a feature not a bug?

I'm saying it's a bug in a feature 😛

The "save you from yourself" bit is actually really important. PHP isn't great at recovering from failure, and a typo can easily drop you out of your shell and back to the command line. Not bad if you're just poking around, but could be kind of bad if you've got a lot of state in variables and you have to start over?

It's not that I think you write bad code, it's that I think PHP is bad at dealing with what should be minor issues.

For example, if what you typed isn't valid PHP, PsySH doesn't naively try to eval() it and let PHP blow up. It tells you it didn't parse and lets you try again. In addition to that, there are a handful of less straightforward cases we watch out for. All in the name of keeping a silly mistake from ruining your interactive session.

As you can imagine, it's not possible to know from static analysis whether any arbitrary code is good. PsySH's goal is to always let valid PHP through, but keep guardrails in place when something is unambiguously wrong. In the case of function names, we're already giving conditionally defined or called functions a pass, but missed the && case 😢

Luckily, PHP is getting better at not making things like this fatal errors! In the next (major) release, we're dropping support for any PHP version that can't deal with referencing function, class and constant names that don't exist. They're now runtime errors rather than fatal errors, so now I'm just … dropping that validation entirely.

[bobthecow]

But! If you're a fan of biking without helmets and driving without seatbelts because you never ever make a mistake, you'll be happy to note that PsySH has a --yolo mode as of v0.10.7, and you can run it without input validation 😛

[joelmellon]

lol, this is getting very pedantic, but function_exists('apcu_enabled') IS my seatbelt.

we're dropping support for any PHP version that can't deal with referencing function, class and constant names that don't exist

So once that's the case, theoretically this use case could be possible since a call to an undefined function could be caught at runtime?

[bobthecow]

So once that's the case, theoretically this use case could be possible since a call to an undefined function could be caught at runtime?

Right. That was literally the point of the next sentence, which included a link to a commit doing just that 😉