Re: RFC: Marking return values as important (#[\NoDiscard])

From: Date: Wed, 12 Feb 2025 14:07:51 +0000
Subject: Re: RFC: Marking return values as important (#[\NoDiscard])
References: 1  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
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


Thread (48 messages)

« previous php.internals (#126379) next »