Changing default assertion mode to throw exceptions

  110978
July 13, 2020 17:36 levim@php.net (Levi Morrison)
Hello everyone,

I'd like to change the default mode of assertion failures to throw.
The current default is to warn. In my opinion this is a bad strategy:
the engine asserted that something that is expected to be true is not,
so executing further is a bad idea. This leaves throwing or bailing
out. I think throwing an exception is better than bailing out, so
that's what I propose.
  110979
July 13, 2020 17:51 marcio.web2@gmail.com (Marcio Almada)
Hi!

> > Hello everyone, > > I'd like to change the default mode of assertion failures to throw. > The current default is to warn. In my opinion this is a bad strategy: > the engine asserted that something that is expected to be true is not, > so executing further is a bad idea. This leaves throwing or bailing > out. I think throwing an exception is better than bailing out, so > that's what I propose. > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php >
Couldn't agree more, but I always assumed this BC break could be too big before. Did you make any research on the impact already? Otherwise +1 Ty, Márcio
  111000
July 14, 2020 15:10 internals@lists.php.net ("Levi Morrison via internals")
On Mon, Jul 13, 2020 at 11:52 AM Marcio Almada web2@gmail.com> wrote:
> > I'd like to change the default mode of assertion failures to throw. > > The current default is to warn. In my opinion this is a bad strategy: > > the engine asserted that something that is expected to be true is not, > > so executing further is a bad idea. This leaves throwing or bailing > > out. I think throwing an exception is better than bailing out, so > > that's what I propose. > > Couldn't agree more, but I always assumed this BC break could be > too big before. Did you make any research on the impact already? > > Otherwise +1
Nikita put together a search of top packagist repositories to look for assert: https://gist.github.com/nikic/8311ee63c72573d514217456bf2df552 We can't generally know what their ini settings are, though. In my opinion this change is worth any backwards compatibility break as we are choosing the worst setting except for ignoring them altogether, and can be changed back to the previous value using an existing ini setting; we just need to put a note in the migrating guide.
  111003
July 14, 2020 16:19 theodorejb@outlook.com (Theodore Brown)
On Tue, July 14 2020 at 10:10 AM Levi Morrison wrote:

> On Mon, Jul 13, 2020 at 11:52 AM Marcio Almada web2@gmail.com> wrote: > > > I'd like to change the default mode of assertion failures to throw. > > > The current default is to warn. In my opinion this is a bad strategy: > > > the engine asserted that something that is expected to be true is not, > > > so executing further is a bad idea. This leaves throwing or bailing > > > out. I think throwing an exception is better than bailing out, so > > > that's what I propose. > > > > Couldn't agree more, but I always assumed this BC break could be > > too big before. Did you make any research on the impact already? > > > > Otherwise +1 > > Nikita put together a search of top packagist repositories to look for assert: > https://gist.github.com/nikic/8311ee63c72573d514217456bf2df552 > > We can't generally know what their ini settings are, though. > > In my opinion this change is worth any backwards compatibility break > as we are choosing the worst setting except for ignoring them > altogether, and can be changed back to the previous value using an > existing ini setting; we just need to put a note in the migrating > guide.
Big +1 on changing the default to throw. The current default of only producing a warning is surprising and can result in code continuing to execute in a state the developer intended to prevent. Theodore
  111006
July 14, 2020 20:43 marcio.web2@gmail.com (Marcio Almada)
Hi!

On Mon, Jul 13, 2020 at 11:52 AM Marcio Almada web2@gmail.com>
> wrote: > > > I'd like to change the default mode of assertion failures to throw. > > > The current default is to warn. In my opinion this is a bad strategy: > > > the engine asserted that something that is expected to be true is not, > > > so executing further is a bad idea. This leaves throwing or bailing > > > out. I think throwing an exception is better than bailing out, so > > > that's what I propose. > > > > Couldn't agree more, but I always assumed this BC break could be > > too big before. Did you make any research on the impact already? > > > > Otherwise +1 > > Nikita put together a search of top packagist repositories to look for > assert: > https://gist.github.com/nikic/8311ee63c72573d514217456bf2df552 > > We can't generally know what their ini settings are, though. > > In my opinion this change is worth any backwards compatibility break > as we are choosing the worst setting except for ignoring them > altogether, and can be changed back to the previous value using an > existing ini setting; we just need to put a note in the migrating > guide. >
I took a look at the top ones + random picks, most are converting warnings to exceptions at least during tests. Still a +1 from me.
  111075
July 20, 2020 13:48 internals@lists.php.net ("Levi Morrison via internals")
On Mon, Jul 13, 2020 at 11:37 AM Levi Morrison <levim@php.net> wrote:
> > Hello everyone, > > I'd like to change the default mode of assertion failures to throw. > The current default is to warn. In my opinion this is a bad strategy: > the engine asserted that something that is expected to be true is not, > so executing further is a bad idea. This leaves throwing or bailing > out. I think throwing an exception is better than bailing out, so > that's what I propose.
Hello, everyone. It's been a week and I have only heard a bit of positive feedback and no pushback. Anyone else want to reply?
  111100
July 22, 2020 08:48 brendt@stitcher.io (Brent Roose)
Hi Levi

I personally almost never use assert, and actually assumed it would throw an error, so this sounds like a good change! 

Have you done research on how this would affect the top-1000-ish popular packages like Nikita does?

Kind regards
Brent

> On 20 Jul 2020, at 15:48, Levi Morrison via internals <internals@lists.php.net> wrote: > > On Mon, Jul 13, 2020 at 11:37 AM Levi Morrison <levim@php.net> wrote: >> >> Hello everyone, >> >> I'd like to change the default mode of assertion failures to throw. >> The current default is to warn. In my opinion this is a bad strategy: >> the engine asserted that something that is expected to be true is not, >> so executing further is a bad idea. This leaves throwing or bailing >> out. I think throwing an exception is better than bailing out, so >> that's what I propose. > > Hello, everyone. It's been a week and I have only heard a bit of > positive feedback and no pushback. Anyone else want to reply? > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php >
  111125
July 22, 2020 15:05 carusogabriel34@gmail.com (Gabriel Caruso)
On Mon, 13 Jul 2020 at 19:37, Levi Morrison <levim@php.net> wrote:

> Hello everyone, > > I'd like to change the default mode of assertion failures to throw. > The current default is to warn. In my opinion this is a bad strategy: > the engine asserted that something that is expected to be true is not, > so executing further is a bad idea. This leaves throwing or bailing > out. I think throwing an exception is better than bailing out, so > that's what I propose. > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php
Hello Levi +1 on this one, I use assertions on my local development, and we also have that on `doctrine/coding-standard` ( https://github.com/doctrine/coding-standard/pull/47). I think throwing an exception is indeed a better way to make the most out of it.
  111126
July 22, 2020 15:07 nikita.ppv@gmail.com (Nikita Popov)
On Wed, Jul 22, 2020 at 5:05 PM Gabriel Caruso <carusogabriel34@gmail.com>
wrote:

> On Mon, 13 Jul 2020 at 19:37, Levi Morrison <levim@php.net> wrote: > > > Hello everyone, > > > > I'd like to change the default mode of assertion failures to throw. > > The current default is to warn. In my opinion this is a bad strategy: > > the engine asserted that something that is expected to be true is not, > > so executing further is a bad idea. This leaves throwing or bailing > > out. I think throwing an exception is better than bailing out, so > > that's what I propose. > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: https://www.php.net/unsub.php > > > Hello Levi > > +1 on this one, I use assertions on my local development, and we also have > that > on `doctrine/coding-standard` ( > https://github.com/doctrine/coding-standard/pull/47). > > I think throwing an exception is indeed a better way to make the most out > of it. >
I'm also +1 on this change. Throwing an AssertionError is clearly the better default experience, and changing back to the previous behavior is super simple. Nikita
  111127
July 22, 2020 15:53 kalle@php.net (Kalle Sommer Nielsen)
Hi

Den man. 13. jul. 2020 kl. 20.37 skrev Levi Morrison <levim@php.net>:
> > Hello everyone, > > I'd like to change the default mode of assertion failures to throw. > The current default is to warn. In my opinion this is a bad strategy: > the engine asserted that something that is expected to be true is not, > so executing further is a bad idea. This leaves throwing or bailing > out. I think throwing an exception is better than bailing out, so > that's what I propose.
+1 on this. I was recently surprised why this wasn't the default either -- regards, Kalle Sommer Nielsen kalle@php.net