Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126379
Precedence: bulk
MIME-Version: 1.0
References: <a995ac24905030e168e7f6006b996cd5@bastelstu.be>
In-Reply-To: <a995ac24905030e168e7f6006b996cd5@bastelstu.be>
Date: Wed, 12 Feb 2025 14:07:51 +0000
Message-ID: <CAGBsUrfFvWBcncpNorb3dmXJ_=cZLhmdaVkFHR6L5SkrhtPLcw@mail.gmail.com>
Subject: Re: [PHP-DEV] RFC: Marking return values as important (#[\NoDiscard])
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Cc: Volker Dusch <volker@tideways-gmbh.com>, php internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
From: tekiela246@gmail.com (Kamil Tekiela)

Hi Tim and Volker,

I'd be honest, I have a very negative attitude towards this proposal
and I'd be voting against it. It seems to me like it's creating a
problem and then trying to find a solution for it.

A return value is always supposed to be used. If some API is returning
a value that can be safely ignored, it's a badly designed API. If a
developer forgets to use a return value, a static analysis tool should
inform the developer of the mistake. I understand that not all
functions are pure like in your example, but adding an attribute
doesn't feel like the solution. In fact, it's creating a problem for
users who want to ignore the value, which you then propose to solve
with (void) cast.

My second gripe is the use of E_WARNING and E_USER_WARNING. I am of
the opinion that warnings in PHP are a really bad idea. PHP isn't a
language like C or C++ where the warnings would show up only during
the compilation phase. In PHP warnings are runtime errors. The code
should emit an exception instead of a warning. It would also make it
much easier to handle and you wouldn't need any special construct to
allow users to ignore the new attribute. And I am really not a fan of
the PHP engine generating E_USER_WARNING which should be reserved only
for warnings triggered by trigger_error.

The examples you used don't support the need for the new attribute.
Regarding the DateTimeImmutable methods, you said yourself: "The
methods are badly named, because they do not actually set the updated
value". So your proposal suggests adding a brand new thing to PHP to
deal with bad method names?

This problem should be solved using static analysers, IDE, and proper
code refactoring.

Regards,
Kamil