# RFC: Locked Classes

- Thread: https://externals.io/message/104620
- Original archive: https://news-web.php.net/php.internals/104620
- Started by Rowan Collins on 2019-03-10T18:35:03+00:00
- Messages: 22
- RFC: Locked Classes (Withdrawn) — https://externals.io/rfc/locked-classes.md
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #104620 — Rowan Collins — 2019-03-10T18:35:03+00:00

URL: https://externals.io/message/104620#104620

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

Regards,

-- 
Rowan Collins
[IMSoP]

---

## #104622 — Marco Pivetta — 2019-03-10T18:51:57+00:00

URL: https://externals.io/message/104620#104622

Hi Rowan,

Overall good idea, except that I disagree with the `unset()` being
disabled: that bit is strictly required for lazy-loading purposes, and
mostly harmless for userland ("normal" people don't do it, libraries do it).

Besides that (blocker for me), if this RFC would be enforced in my coding
standards of choice 👍

Does anybody know if this has potential for engine optimizations?




> 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
>
> Regards,
>
> --
> Rowan Collins
> [IMSoP]

---

## #104623 — Gabriel O — 2019-03-10T18:56:27+00:00

URL: https://externals.io/message/104620#104623

Isn’t unset() currently only way to remove reference? This part is hence very problematic.

> 
> 
> 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
> 
> Regards,
> 
> -- 
> Rowan Collins
> [IMSoP]

---

## #104624 — Rowan Collins — 2019-03-10T19:23:00+00:00

URL: https://externals.io/message/104620#104624

Hi Marco,


> Overall good idea, except that I disagree with the `unset()` being 
> disabled: that bit is strictly required for lazy-loading purposes, and 
> mostly harmless for userland ("normal" people don't do it, libraries 
> do it).


To me, the unset() restriction fits naturally into the definition; I 
think most people would consider "removing a property" in the same 
category as "adding a property". Notably, it's included in ECMAScript's 
definition of sealed objects [1], which is essentially the same feature.

I understand that unset() currently enables a neat trick for 
lazy-loading, but it feels like just that to me: a trick, not a core 
part of the language. Similarly, there may be tricks that rely on adding 
dynamic properties to objects, but that's fine, just don't mark those 
classes as "locked".


[1] 
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/seal

Regards,

-- 
Rowan Collins
[IMSoP]

---

## #104626 — Larry Garfield — 2019-03-10T19:32:30+00:00

URL: https://externals.io/message/104620#104626

> Hi Rowan,
> 
> Overall good idea, except that I disagree with the `unset()` being
> disabled: that bit is strictly required for lazy-loading purposes, and
> mostly harmless for userland ("normal" people don't do it, libraries do it).
> 
> Besides that (blocker for me), if this RFC would be enforced in my coding
> standards of choice 👍
> 
> Does anybody know if this has potential for engine optimizations?

Marco, have you an example of such lazy-loading tricks?  I'm not familiar with them but would like to be...

I'm overall +1 here as well, as I am for most things that allow me to make my code stricter and pickier.  (I don't have a strong feeling on unset() either way.)

--Larry Garfield

---

## #104627 — Rowan Collins — 2019-03-10T19:36:16+00:00

URL: https://externals.io/message/104620#104627

> Isn’t unset() currently only way to remove reference? This part is hence very problematic.


Hm, that's an interesting case to consider, thanks!

At the moment, the following code breaks the reference between $a->foo 
and $bar, but also deletes the entire property from the instance:

class A { public $foo; }
$a = new A;
$a->foo =& $bar;
unset($a->foo);
var_dump($a->foo); # Notice: Undefined property: A::$foo

This feels very peculiar to me: $foo clearly *is* a defined property, 
and I didn't want to over-ride the class definition, I just wanted to 
"reset" that property. We can chalk it up to "references are bad", but 
it is indeed problematic that there isn't an alternative way to say 
"breakReference($a->foo);" that doesn't have this side effect.

Regards,

-- 
Rowan Collins
[IMSoP]

---

## #104628 — Marco Pivetta — 2019-03-10T19:46:08+00:00

URL: https://externals.io/message/104620#104628

Typical scenario (works for either of the properties - note that typed
properties that aren't nullable and have no default value also behave the
same way):

class Foo {
    private int $a;
    private $b;
    public function __construct() {
        unset($this->b);
    }
    public function __get(string $name) {
        $this->a = 123
        $this->b = 'abc';
        return $this->$name;
    }
    public function saySomething() : string {
        return $this->a . $this->b;
    }
}

echo (new Foo())->saySomething();

https://3v4l.org/Rn39Q (note: 3v4l doesn't have 7.4, so I simplified it a
bit)

@rowan: I've stated it multiple times in the past, but if the mechanism is
to be removed, then it should be replaced with a language-level concept of
laziness then.



> 
> > […]
> it).
> > […]
>
> Marco, have you an example of such lazy-loading tricks?  I'm not familiar
> with them but would like to be...
>
> I'm overall +1 here as well, as I am for most things that allow me to make
> my code stricter and pickier.  (I don't have a strong feeling on unset()
> either way.)
>
> --Larry Garfield

---

## #104631 — Paul Jones — 2019-03-10T20:16:08+00:00

URL: https://externals.io/message/104620#104631

> 
> 
> Hi all,
> 
> I'd like to present a new RFC for "locked classes": classes which restrict dynamically adding or removing properties from their instances.

Nice. This would help enforce at least one element of immutability in userland; namely, not having to define `final public function __set()` and `final public function __unset()` to prevent adding and mutating undefined properties.



-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php

---

## #104632 — Dan Ackroyd — 2019-03-10T20:19:17+00:00

URL: https://externals.io/message/104620#104632

>
> Hi all,
>
> - 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.

I believe those two parts of the RFC are completely possible already in
user-land with a trait similar to the one below. What would be the
compelling case for making it be a keyword, rather than the user-land
implementation that is already achievable?

Also.....this seems to be a special case of an idea that has come up
repeatedly, of allowing the concept of 'packages' with rules applied to all
of the classes in a package e.g.

* class Friendship which only allows access to certain methods to other
classes in the same package.
* strict enabled by default for a classes in a package.

and now this RFC (or at least 2/3 of it) could be implemented by adding
some traits by default to all classes in a package.

Rather than adding a special case, I'd much rather we looked at providing
the general case.

cheers
Dan
Ack



trait LockedClass
{
public function __set($name, $value)
{
throw new \Exception("Property [$name] doesn't exist for class [" .
get_class($this) . "] so can't set it");
}
public function __get($name)
{
throw new \Exception("Property [$name] doesn't exist for class [" .
get_class($this) . "] so can't get it");
}
}

---

## #104633 — Rowan Collins — 2019-03-10T20:19:45+00:00

URL: https://externals.io/message/104620#104633

> @rowan: I've stated it multiple times in the past, but if the mechanism is
> to be removed, then it should be replaced with a language-level concept of
> laziness then.


1) I'm all for adding a language-level concept of laziness. It feels 
like it could fit with a syntax for getters and setters, which has been 
discussed a few times, but never quite made it.

2) I'm not saying that the current behaviour should be removed, I'm 
saying that dynamically unsetting properties is inherently incompatible 
with declaring a class "locked".

I've actually had more than one suggestion that __get(), __set(), and 
__unset() should not be allowed at all on locked classes; in which case, 
it wouldn't matter how unset() behaved, because the lazy-loading trick 
relies on __get() as well.

Regards,

-- 
Rowan Collins
[IMSoP]

---

## #104634 — Rowan Collins — 2019-03-10T20:48:26+00:00

URL: https://externals.io/message/104620#104634

> I believe those two parts of the RFC are completely possible already 
> in user-land with a trait similar to the one below. What would be the 
> compelling case for making it be a keyword, rather than the user-land 
> implementation that is already achievable?


Hi Dan,

Yes, such a trait had occurred to me before. My main motivation for 
proposing it as part of the language is to make it as easy as possible 
to opt into, and standardising the mechanics - I can see "all classes 
must be strict" making it into far more coding standards than "all 
projects must include this Trait and add it to all classes". It occurred 
to me that language support might also allow optimisation around it, 
whereas implementing __get()/__set() would likely do the opposite.

Apart from that, I was partly just experimenting with how hard it would 
be to add; the answer, so far at least, is surprisingly easy.

The alternative would be to simply deprecate dynamic property getting 
and setting altogether. I suspect that would affect a lot of code, 
though, so would be wary of actually removing it as soon as PHP 8, given 
that the set part doesn't even raise a notice right now.


> and now this RFC (or at least 2/3 of it) could be implemented by 
> adding some traits by default to all classes in a package.

This wasn't really relevant to my motivation - you'd still have to add 
it to each class under the proposed RFC - but it is an interesting idea.

In general, I think such packages would be a great addition to the 
language, but they'd also be a big change in how things work, and take a 
lot of working out, so I'm not going to hold my breath.


Regards,

-- 
Rowan Collins
[IMSoP]

---

## #104635 — Rowan Collins — 2019-03-10T20:56:43+00:00

URL: https://externals.io/message/104620#104635

> trait LockedClass
> {
> public function __set($name, $value)
> {
> throw new \Exception("Property [$name] doesn't exist for class [" . 
> get_class($this) . "] so can't set it");
> }
> public function __get($name)
> {
> throw new \Exception("Property [$name] doesn't exist for class [" . 
> get_class($this) . "] so can't get it");
> }
> }


Incidentally, this isn't quite equivalent to the current RFC, because it 
will also intercept access to private or protected properties from out 
of scope. It doesn't make much difference other than making the error 
messages less helpful, but it's an example of how a nice userland 
implementation isn't quite as trivial as it first looks.

Regards,

-- 
Rowan Collins
[IMSoP]

---

## #104636 — Marco Pivetta — 2019-03-10T21:09:09+00:00

URL: https://externals.io/message/104620#104636

Still feasible though: PRs to
https://github.com/Roave/Dont/tree/master/src/Dont welcome



> 
> > […]
>
>
> Incidentally, this isn't quite equivalent to the current RFC, because it
> will also intercept access to private or protected properties from out
> of scope. It doesn't make much difference other than making the error
> messages less helpful, but it's an example of how a nice userland
> implementation isn't quite as trivial as it first looks.
>
> Regards,
>
> --
> Rowan Collins
> [IMSoP]

---

## #104639 — Rowan Collins — 2019-03-10T22:37:21+00:00

URL: https://externals.io/message/104620#104639

> Still feasible though: PRs to 
> https://github.com/Roave/Dont/tree/master/src/Dont welcome


Indeed. Many things in a language are sugar for something that could be 
done in a more complex way; it's a judgement call which ones are worth 
including, and maybe this one is, maybe it's not.

Note that that trait would definitely break lazy-loading, though, 
because it declares a final __get()

Regards,

-- 
Rowan Collins
[IMSoP]

---

## #104648 — Nikita Popov — 2019-03-11T11:27:15+00:00

URL: https://externals.io/message/104620#104648

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

---

## #104649 — Rowan Collins — 2019-03-11T12:14:23+00:00

URL: https://externals.io/message/104620#104649

Hi Nikita,



> 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.
>


That's an interesting approach. I think it's conceptually a lot more
complicated (and probably more complicated to implement, too) - the same
object would behave in different ways in different contexts. There are also
edge cases to define which the current proposal doesn't need to worry
about, like stdClass, and objects which were created in one scope and
manipulated in another:

declare(strict_properties=0) {
    class Foo {}
    $foo = new Foo;
    $foo->bar = 42;
}
declare(strict_properties=1) {
    echo $foo; # Is $bar now 'defined', or do we only count the properties
in the original class definition?
}

More importantly, you would have to check that third-party code was
declaring the properties you needed before enabling it. For instance, a
library might document that you should set `$handler->debug = true`, read
that property internally, but never declare it; you'd then have to patch
the library before you could run in strict_properties mode.

It feels like at that point, we might as well just deprecate dynamic
properties altogether - which I'm beginning to think is the best approach.

To put it explicitly, what are the use cases where you'd turn such a flag
off?




> 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").
>


Oops, I'll hold my hands up to not actually testing that. I think I took
that wording from a comment in the parser, but probably didn't read it
properly.



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().
>


Huh, cunning. :)

Regards,
-- 
Rowan Collins
[IMSoP]

---

## #104651 — Rowan Collins — 2019-03-11T12:32:45+00:00

URL: https://externals.io/message/104620#104651

>
> declare(strict_properties=0) {
>     class Foo {}
>     $foo = new Foo;
>     $foo->bar = 42;
> }
> declare(strict_properties=1) {
>     echo $foo; # Is $bar now 'defined', or do we only count the properties
> in the original class definition?
> }
>


Oops, that should read echo $foo->bar;


-- 
Rowan Collins
[IMSoP]

---

## #104652 — Nikita Popov — 2019-03-11T12:35:26+00:00

URL: https://externals.io/message/104620#104652

On Mon, Mar 11, 2019 at 1:14 PM Rowan Collins <rowan.collins@gmail.com>
wrote:

> Hi Nikita,
>
> 
>
> > […]
> This
> > […]
> from
> > […]
> your
> > […]
>
>
> That's an interesting approach. I think it's conceptually a lot more
> complicated (and probably more complicated to implement, too) - the same
> object would behave in different ways in different contexts. There are also
> edge cases to define which the current proposal doesn't need to worry
> about, like stdClass, and objects which were created in one scope and
> manipulated in another:
>
> declare(strict_properties=0) {
>     class Foo {}
>     $foo = new Foo;
>     $foo->bar = 42;
> }
> declare(strict_properties=1) {
>     echo $foo; # Is $bar now 'defined', or do we only count the properties
> in the original class definition?
> }
>

I'd expect it to be defined, especially as it allows interoperating with
libraries that are missing property declarations and set dynamic properties
(intentionally).

More importantly, you would have to check that third-party code was
> declaring the properties you needed before enabling it. For instance, a
> library might document that you should set `$handler->debug = true`, read
> that property internally, but never declare it; you'd then have to patch
> the library before you could run in strict_properties mode.
>
> It feels like at that point, we might as well just deprecate dynamic
> properties altogether - which I'm beginning to think is the best approach.
>
> To put it explicitly, what are the use cases where you'd turn such a flag
> off?
>

You'd turn it off in the case you mentioned: A 3rd-party library that
relies on dynamic properties. The idea is that you disable dynamic
properties globally for your own code, but can opt back in for cases where
they are necessary for some reason (most likely interoperability, but
possibly also code doing weird Marco things).

This allows you to single out some piece of code as "here be dragons",
rather than keeping dragons as the default state :)

Removing the ability to use dynamic properties (even excluding stdClass) is
probably not feasible on a short timetime. However, I think that having a
directory-scoped declare would also be useful for that, because it allows a
more gradual migration. You can start with having strict_properties default
to false and have interested parties opt-in. Then you can flip the default
to true, while still giving people who rely heavily on it the ability to
opt-out. Removal would be the final stage where the ability to opt-out goes
away.

Regarding optimizations that your RFC allows: In theory we could drop the
properties_table member (8 bytes) for objects of locked classes. In
practice this is probably not beneficial as access to the embedded property
table would no longer be at a fixed offset.

Nikita

> 1. The BC break section is not terribly clear due to the use of
> > […]
>
>
> Oops, I'll hold my hands up to not actually testing that. I think I took
> that wording from a comment in the parser, but probably didn't read it
> properly.
>
>
>
> 2. It is possible to break a reference by reassigning it to another
> > […]
> unset().
> > […]
>
>
> Huh, cunning. :)
>
> Regards,
> --
> Rowan Collins
> [IMSoP]

---

## #104655 — Rowan Collins — 2019-03-11T14:18:46+00:00

URL: https://externals.io/message/104620#104655

>
> Removing the ability to use dynamic properties (even excluding stdClass)
> is probably not feasible on a short timetime. However, I think that having
> a directory-scoped declare would also be useful for that, because it allows
> a more gradual migration. You can start with having strict_properties
> default to false and have interested parties opt-in. Then you can flip the
> default to true, while still giving people who rely heavily on it the
> ability to opt-out. Removal would be the final stage where the ability to
> opt-out goes away.
>
>

I think that's actually the best argument I've seen so far. The current
proposal is basically very conservative: a simple, self-contained change,
that even a noob like me was able to implement. The downside of that is
that it doesn't have a long-term vision: there's no clear path to making
all classes "locked" by default.

I agree that the combination of package- or directory-scoped declares, and
a switch that can be both opt-in and opt-out, gives a better future
direction. I just hope we get those things, rather than just having neither
- as happened with "static class", which was rejected partly because "we'll
have function autoloading real soon".

Regards,
-- 
Rowan Collins
[IMSoP]

---

## #104684 — Stanislav Malyshev — 2019-03-12T21:40:22+00:00

URL: https://externals.io/message/104620#104684

Hi!

> 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".

Isn't it just:

trait Locked {
  public function __set($name, $value) {
	throw new RuntimeException("Property $name not declared!");
  }
  public function __get($name) {
	throw new RuntimeException("Property $name not declared!");
  }
  public function __unset($name) {
	throw new RuntimeException("Property $name not declared!");
  }
}

The RFC says:

While this can be achieved through strategic use of the __set, __get,
and __unset magic methods, this is long-winded, hard to optimise, and
interferes with other uses of those methods.

I don't see how it's "long-winded" - it's just three one-liners, I don't
see how "interferes with other uses" of these methods (if you want other
uses, just add code to the method), and as for optimization, I think we
already have optimization for pre-declared properties, and if you don't
use any others, it would work just as well.


-- 
Stas Malyshev
smalyshev@gmail.com

---

## #104685 — Gabriel O — 2019-03-12T21:45:55+00:00

URL: https://externals.io/message/104620#104685

It was already addressed how using such trivial solution is insufficient, please read whole thread

> 
> 
> Hi!
> 
> > […]
> 
> Isn't it just:
> 
> trait Locked {
>  public function __set($name, $value) {
> 	throw new RuntimeException("Property $name not declared!");
>  }
>  public function __get($name) {
> 	throw new RuntimeException("Property $name not declared!");
>  }
>  public function __unset($name) {
> 	throw new RuntimeException("Property $name not declared!");
>  }
> }
> 
> The RFC says:
> 
> While this can be achieved through strategic use of the __set, __get,
> and __unset magic methods, this is long-winded, hard to optimise, and
> interferes with other uses of those methods.
> 
> I don't see how it's "long-winded" - it's just three one-liners, I don't
> see how "interferes with other uses" of these methods (if you want other
> uses, just add code to the method), and as for optimization, I think we
> already have optimization for pre-declared properties, and if you don't
> use any others, it would work just as well.
> 
> 
> -- 
> Stas Malyshev
> smalyshev@gmail.com

---

## #104688 — Rowan Collins — 2019-03-12T23:33:16+00:00

URL: https://externals.io/message/104620#104688

> Isn't it just:
> trait Locked {
>    public function __set($name, $value) {
> 	throw new RuntimeException("Property $name not declared!");
>    }
>    public function __get($name) {
> 	throw new RuntimeException("Property $name not declared!");
>    }
>    public function __unset($name) {
> 	throw new RuntimeException("Property $name not declared!");
>    }
> }


Almost...

Magic methods are run both for undefined properties and out-of-scope 
ones, so at the very least this leads to unhelpful or vague error messages.

It also doesn't handle unset in the same way as the current RFC, because 
it won't affect unsetting a property that was previously defined 
(although that part of the RFC is slightly controversial, it seems).

There may be other edge cases it doesn't handle the same way an internal 
implementation would, as well, I'm not sure.

It's certainly not a major saving over hacking it together in userland, 
but nor is it a major weight to add it to the language. I think having a 
well-thought-out implementation built in (either the one in the current 
RFC, or a declare directive as has been suggested elsewhere) would 
encourage people to use it, and generally be more user-friendly.


Regards,

-- 
Rowan Collins
[IMSoP]