Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104648
MIME-Version: 1.0
References: <2c497732-96f8-3ef0-bc18-912220fbff4d@gmail.com>
In-Reply-To: <2c497732-96f8-3ef0-bc18-912220fbff4d@gmail.com>
Date: Mon, 11 Mar 2019 12:27:15 +0100
Message-ID: <CAF+90c_JaC7xmfVcRZKZD6kqjkverOGPRNc2mEFSpbDtDHGaVA@mail.gmail.com>
To: Rowan Collins <rowan.collins@gmail.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000cbca030583cfdb20"
Subject: Re: [PHP-DEV] RFC: Locked Classes
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000cbca030583cfdb20
Content-Type: text/plain; charset="UTF-8"

On Sun, Mar 10, 2019 at 7:35 PM Rowan Collins <rowan.collins@gmail.com>
wrote:

> Hi all,
>
> I'd like to present a new RFC for "locked classes": classes which
> restrict dynamically adding or removing properties from their instances.
>
> While it can be useful, the ability to set an object property which is
> not part of the class definition can also lead to subtle bugs. Banning
> this for all objects would be a significant and painful breaking change,
> so I propose instead the option to mark a particular class with a new
> keyword, "locked".
>
> An instance of a locked class behaves like any other object, except that:
>
> - Attempting to set a property on the instance which was not declared in
> the class (or inherited from one of its parent classes) will throw an
> error, and the instance will not be modified.
> - Attempting to read a property on the instance which was not declared
> (or inherited) will throw an error, rather than raising a Notice and
> evaluating to null.
> - Attempting to call unset() on any property of the instance will throw
> an error, and the instance will not be modified.
>
> Note that ECMAScript  / JavaScript includes a similar feature, called
> "sealed objects". However, the proposed modifier applies to classes, and
> "sealed class" has a different meaning elsewhere (e.g. C#, Kotlin), so
> I've chosen "locked class" to avoid confusion.
>
> For further details and examples, please check the RFC at
> https://wiki.php.net/rfc/locked-classes and the tests in the draft
> implementation at https://github.com/php/php-src/pull/3931


I like the general idea of this, though I think it's not quite the right
way to go about solving the problem. The issue I see with this proposal is
that it requires annotating each class with an additional keyword --
however, chances are good that people will want all *classes* to be locked,
including those they do not have direct control over (coming from
libraries).

The solution I would prefer is the ability to declare that within a
project, all interactions with objects (whether they are my own or come
from 3rd parties) should disallow the creation of dynamic properties. This
differs from your proposal in two important points:
 1. You cannot create dynamic properties on objects even if they come from
3rd-party code you do not control.
 2. 3rd-party code may interact with your objects (and it's own) however it
likes. It is not affected by the disabling of dynamic properties in your
project code.

If this sounds familiar, this is basically strict_types, but for dynamic
properties. Of course, doing this with declares as they are is rather
cumbersome, because you'd have to specify them in every single file. Things
would be different if you could specify a declare for a whole project...

This is what https://wiki.php.net/rfc/namespace_scoped_declares was about,
and it actually happens to use exactly this example as one of the things
the mechanism could be used for. I still think that this would be the more
principled approach to this issue, especially as it easily extends to other
problem areas, without requiring a new keyword for each. (Based on previous
feedback, it should probably be directory-scope declares rather than
namespace-scope.)

Now, to get back to your RFC, two notes:

1. The BC break section is not terribly clear due to the use of
"semi-reserved". I initially assumed that this was a contextual keyword
(i.e. only has special meaning immediately before "class") but apparently
this is a normal reserved keyword in the implementation (i.e. no classes,
functions, namespaces etc named "locked").

2. It is possible to break a reference by reassigning it to another
reference, like so:

    $null = null;
    $this->prop =& $null;
    unset($null);

This is not quite as good as unsetting because it leaves behind an rc=1
reference, but that would be the way to remove a reference without unset().

Regards,
Nikita

--000000000000cbca030583cfdb20--