[RFC] [VOTE] Typed properties v2 View as Markdown

I agree that this is a hard problem, but I don't agree that this decision
is being made "for now". If we allow "non-nullable but uninitialized"
properties now, it will be extremely hard to change their behaviour in
future.

I'm with Rowan on this one.

This concept of "uninitialized" frankly seems like an allowance for
people who insist on writing poor code.

Nulls are bad, and "unintialized" is just another kind of "null" with
a built-in run-time type-check that executes on read - too late.

The first example given is just bad:

class Point {
public float $x, $y;

private function __construct() {}

public static function fromEuclidean(float $x, float $y) {
    $point = new Point;
    $point->x = $x;
    $point->y = $y;
    return $point;
}

}

You define two invariants: $x and $y must be floats - and then proceed
to break those constraints in the constructor?

Wrong. The RFC itself accurately states that "this code can be
rewritten to indirect through __construct() instead" - as shown in the
previous example.

Now why would you deliberately open the fences and knowingly invite
people to write poor code like this?

As for the second example:

class Point {
public float $x, $y;

public function __construct(float $x, float $y) {
    $this->doSomething();
    $this->x = $x;
    $this->y = $y;
}

}

If doSomething() attempts to read an uninitialized property while the
constructor is still executing, throwing a helpful "uninitialized"
error is fine.

But, in my opinion, once the constructor has executed, the invariants
as declared by the class itself must be satisfied.

If there's one meaningful use-case for allowing objects in a
partially-initialized state, it's during
hydration/unserialization/reflection scenarios, maybe - but in those
cases, you're willfully bypassing the constructor; it's not the
everyday 95% use-case and some risk is acceptable here, you'll get
around it with tests. But nobody wants to write tests all day to see
if any classes contain "unininitialized" properties - that misses half
the whole point of being able to declare those types in the first
place, e.g. makes type-hinted private/protected properties totally
unreliable.

Once this is in a release, it'll be unfixable, and in my opinion will
likely go down in history as another one of those little things we
wish we could go back in time and fix :-/

--

PHP permits skipping constructors. The code may not work if you do so,
but it's the state of how things are. Validating after a constructor
call will not catch all issues while requiring a constructor, whereas
I think this code should be allowed:

class User {
public int $id;
public string $preferred_name;
public string $username;
}

I doubt we will come to a resolution -- these points were already
pointed out in the discussion phase.

I think the distinction here is that one group is arguing for "state of
the
data assertions" while the RFC as implemented is "setter assertion
shorthand".
That is, it doesn't assert that a value IS a given type, but that it can
only
be SET TO a given type.

I don't think a complete IS enforcement is possible given PHP's nature.
...
That hasn't brought about the end of the world, so I'm not super worried
about
imperfect property checks destroying everything.

That said, I totally appreciate the desire as a consumer of an object to
know
that it's read-type-safe.

This is a useful distinction, thank you for putting it so clearly.

Perhaps it depends whether you are looking at the feature as a library
author, or a library consumer: as an author, you want to protect "your"
objects from invalid assignments, inside and outside the library; as a
consumer, you want assurance that you can use the object in a particular
way. The current implementation provides a good tool for the author, but
falls short for the consumer.

To wit, could we add an engine check to scan an object and make sure its
objects are all type valid right-now (viz, nothing is unitialized), and
then
call it on selected actions automatically and allow users to call it at
arbitrary times if they are doing more esoteric things?

I'm thinking:

• on __construct() exit.
• on __wakeup() exit.
• Possibly other similar checkpoints.
• When a user calls is_fully_initialized($obj); (or something)

is_fully_initialized() would return bool. The other checkpoints would
throw a
TypeError.

That would layer on top of the current RFC cleanly, would give us the read-
assurance that we'd like in the 95% case

I think this is a really sensible approach: as you say, it may never be
possible to assert the invariant in every case, so checking the most common
scenarios may be the pragmatic thing to do.

Conveniently, treating it as an occasional assertion, rather than a strict
invariant, means we can keep the unset() lazy-initialization hack; it's
still an odd feature IMO, but probably not all that likely to be triggered
by mistake.

Regards,

Rowan Collins
[IMSoP]

I apologize for the long posts, but Larry asked me to comment on this.

I think the distinction here is that one group is arguing for "state of the
data assertions" while the RFC as implemented is "setter assertion shorthand".

The point of the setter assertions is to provide guarantees about the state of
the data - there is literally no difference.

That is, it doesn't assert that a value IS a given type, but that it can only
be SET TO a given type.

That is literally the same thing - if it can only be set to a given
type, is can only
BE a given type.

The problem here is you want to make exemptions for null as type - as though
nulls aren't types (which they are) and as though nullable types aren't distinct
from the types they're derived from. (again, they are.)

I don't think a complete IS enforcement is possible given PHP's nature.

I don't think that, and I don't expect that - I'm not suggesting we enforce
anything statically, I'm merely suggesting that a constructor needs to satisfy
the constraints specified by the class.

If you've type-hinted a property as int, you don't allow strings - that would be
pointless, right?

But that is precisely the problem we're talking about - if you've type-hinted a
property as non-nullable int, it shouldn't allow null... but that is
literally the
exemption you want to allow for - you just want to annotate the property
with an additional meta-data property "unintialized", which is literally the
same as adding a boolean "is set" for every property in your model.

This isn't a compile time check
either:

$b = 'foo';
test($b);

It's a runtime TypeError, not compile time.

This actually illustrates my point nicely - the variable $b in this
case has nothing
to do with the state of the parameter var inside the function body,
until you pass
it via a function call. That's perfectly fine. You'll get an error at
the point where
the parameter type-constraint was violated - you can look at a stack-trace and
debug that easily.

Now take your example and call a constructor instead of a function:

class Test {
public string $b;
}

$b = 'foo';
$test = new Test();

Your constructor call generates no run-time error here, and the program
continues to execute with $test in a state that, according to specification of
the class itself, isn't valid.

Since the constructor is how a valid instance of Test gets generated in the
first place, you've missed your only chance to enforce those constraints -
meaning, you've missed the only opportunity you had to verify that the
constructor does in deed generate an instance of Test that lives up to
Test's own specification of what a Test instance is.

To wit, could we add an engine check to scan an object and make sure its
objects are all type valid right-now (viz, nothing is unitialized), and then
call it on selected actions automatically and allow users to call it at
arbitrary times if they are doing more esoteric things?

In my opinion, this is a solution to the problem we created when we decided
every property should internally be annotated with an "initialized" boolean
property - you have all this extra state that wasn't part of the specification
of the class itself, and now you have to deal with that state.

In my opinion, tracking this extra state during the constructor call is
acceptable and necessary in a scripting language like PHP - I never
asked for static type-checking, I'm merely asking for this check to be
built-into the language and performed at run-time when you exit the
constructor, e.g. at the last moment where you can feasibly perform
this check.

If you don't perform that check, you have no guarantees at all.

Consider this example:

function login(int $user_id) {
// ...
}

Inside the body of this function, you can safely assume you have an integer,
right? A basic guarantee provided by the language.

Now consider this alternative:

function login(User $user) {
// ...
}

Inside the body of this function, you'd assume you have a valid instance
of User, right? The same basic guarantee provided by the language.

Wrong. Something might be "unintialized". This might in fact not be a valid
instance of User, it might be only partially initialized.

This is a problem, because I want my type User to be a reliable type - as
reliable as any other type.

Integers for example is nice, because I know I'm going to be able to add,
subtract, multiple, divide, and so on - if you've type-hinted a parameter as
int, you know it has those abilities, you know the + operator isn't going to
fail because there's a special kind of int that doesn't work with the

• operator.

We want those same guarantees from our own types, that's all.

In the case where a type doesn't provide such a guarantee, it can explicitly
state that something is nullable - not guaranteed to be set. You have that
option already, you don't need unintialized properties for that.

If there really is a use-case for "optional values that must not
be removed again once they've been set" - and I can't think of one - but
if there is, you can handle that rare case with logic in getters/setters.

The normal everyday use-case is you want to know if a property is going
to be set or not - hence nullable property-types, which are part of the RFC.

Userland work-arounds for basic guarantees that the language should be
providing, in my opinion, are totally unacceptable - in a nutshell, because
basic guarantees provided for other types won't apply to objects.

It's incoherent with the workings of type-hints in the language, and will
make those substantially less useful.

Thank you all!

Is the Perfomance section of the RFC updated?
https://wiki.php.net/rfc/typed_properties_v2#performance would be great to
know now that the patch is final and merged

Hi Nikita,

On Mon, Oct 22, 2018 at 1:58 PM Sebastian Bergmann sebastian@php.net
wrote:

Am 26.09.2018 um 15:46 schrieb Nikita Popov:

I'm pleased to announce that the typed properties RFC has been
accepted
with 70 votes in favor and one vote against. We will work to finalize
and
merge the implementation in the next few days.

Any update on when this will be merged? Thanks!

Sorry for the long delay here. I've spent the last week fixing the
remaining issues and cleaning up the patch, and...

...Typed properties are now merged into master! [1]

It would be great if people could start experimenting with typed
properties
(e.g. by migrating over existing @var annotations and seeing what
happens).
The earlier we find issues in the implementation or semantics, the
better.

Thanks to everyone who worked on this RFC and participated in
discussions.
Special thanks to Bob Weinand who did most of the initial implementation
work on this RFC.

Regards,
Nikita

[1]:

https://github.com/php/php-src/commit/e219ec144ef6682b71e135fd18654ee1bb4676b4

Great news! Just in time to ruin the weekend with some OSS work :D

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

Heya,

Reflection is not slow: it is only slow if you instantiate it continuously.

Hi Nikita,

While playing with typed properties, something that annoys me a lot, in the
context of a data mapper, is not to be able to use isset() to check whether
an object's property is initialized. The issue was already there with
properties that have been explicitly unset(), but will be exacerbated by
typed properties, which will be unset by default.

Sure, we have ReflectionProperty::isInitialized(), but reflection is slow
compared to direct property access, when reading a lot of objects.

For example, check these 3 ways of reading initialized public/protected
object properties by name, along with their timings for 100,000 iterations
(benchmark here
<
https://gist.github.com/BenMorel/9a920538862e4df0d7041f8812f069e5#file-reflection-vs-array-cast-benchmark-php

):

Using reflection: (890 ms, down to 384 ms using pre-instantiated
ReflectionProperty objects)

$r = new \ReflectionClass($class);

foreach ($props as $prop) {
    $p = $r->getProperty($prop);
    $p->setAccessible(true);
    if ($p->isInitialized($object)) {
        $values[$p->getName()] = $p->getValue($object);
    }
}

Using array cast: (193 ms)

foreach ((array) $object as $key => $value) {

    // Remove the "\0*\0" in front of protected properties
    $pos = strrpos($key, "\0");
    if ($pos !== false) {
        $key = substr($key, $pos + 1);
    }
    $values[$key] = $value;
}

Using a bound closure and isset(): (145 ms)

(function() use ($props, & $values) {
    foreach ($props as $prop) {
        if (isset($this->{$prop})) { // skips `NULL` values as well :-(
            $values[$prop] = $this->{$prop};
        }
    }
})->bindTo($object, $object)();

Unfortunately, while the last approach is the fastest, and IMO the cleanest
one, it is currently unusable because there is no way to differentiate
between an uninitialized property and a NULL property.

Would it be possible to introduce another isset() operator, that would
return true for NULL values?
Note that this would also be useful when checking if an array key exists,
instead of having to switch from isset() to array_key_exists() when the
array may contain NULL values.

Ben

On Mon, Oct 22, 2018 at 1:58 PM Sebastian Bergmann sebastian@php.net
wrote:

Am 26.09.2018 um 15:46 schrieb Nikita Popov:

I'm pleased to announce that the typed properties RFC has been
accepted
with 70 votes in favor and one vote against. We will work to finalize
and
merge the implementation in the next few days.

Any update on when this will be merged? Thanks!

Sorry for the long delay here. I've spent the last week fixing the
remaining issues and cleaning up the patch, and...

...Typed properties are now merged into master! [1]

It would be great if people could start experimenting with typed
properties
(e.g. by migrating over existing @var annotations and seeing what
happens).
The earlier we find issues in the implementation or semantics, the
better.

Thanks to everyone who worked on this RFC and participated in
discussions.
Special thanks to Bob Weinand who did most of the initial implementation
work on this RFC.

Regards,
Nikita

[1]:

https://github.com/php/php-src/commit/e219ec144ef6682b71e135fd18654ee1bb4676b4

On Wed, Feb 6, 2019 at 1:38 PM Benjamin Morel benjamin.morel@gmail.com
wrote:

Hi Nikita,

While playing with typed properties, something that annoys me a lot, in
the context of a data mapper, is not to be able to use isset() to check
whether an object's property is initialized. The issue was already there
with properties that have been explicitly unset(), but will be
exacerbated by typed properties, which will be unset by default.

Sure, we have ReflectionProperty::isInitialized(), but reflection is slow
compared to direct property access, when reading a lot of objects.

For example, check these 3 ways of reading initialized public/protected
object properties by name, along with their timings for 100,000 iterations
(benchmark here
https://gist.github.com/BenMorel/9a920538862e4df0d7041f8812f069e5#file-reflection-vs-array-cast-benchmark-php
):

Using reflection: (890 ms, down to 384 ms using pre-instantiated
ReflectionProperty objects)

$r = new \ReflectionClass($class);

foreach ($props as $prop) {
    $p = $r->getProperty($prop);
    $p->setAccessible(true);
    if ($p->isInitialized($object)) {
        $values[$p->getName()] = $p->getValue($object);
    }
}

Using array cast: (193 ms)

foreach ((array) $object as $key => $value) {

    // Remove the "\0*\0" in front of protected properties
    $pos = strrpos($key, "\0");
    if ($pos !== false) {
        $key = substr($key, $pos + 1);
    }
    $values[$key] = $value;
}

Using a bound closure and isset(): (145 ms)

(function() use ($props, & $values) {
    foreach ($props as $prop) {
        if (isset($this->{$prop})) { // skips `NULL` values as well :-(
            $values[$prop] = $this->{$prop};
        }
    }
})->bindTo($object, $object)();

Unfortunately, while the last approach is the fastest, and IMO the
cleanest one, it is currently unusable because there is no way to
differentiate between an uninitialized property and a NULL property.

Would it be possible to introduce another isset() operator, that would
return true for NULL values?
Note that this would also be useful when checking if an array key exists,
instead of having to switch from isset() to array_key_exists() when the
array may contain NULL values.

It's possible and someone even worked on an implementation at some point (
https://github.com/php/php-src/pull/1530), but I don't believe this ever
proceeded to the RFC stage. (I'm rather doubtful about introducing
something like this, due to the quite significant increase in conceptual
language complexity for little benefit. I definitely have no plans to
pursue such a feature myself.)

Nikita

On Mon, Oct 22, 2018 at 1:58 PM Sebastian Bergmann sebastian@php.net
wrote:

Am 26.09.2018 um 15:46 schrieb Nikita Popov:

I'm pleased to announce that the typed properties RFC has been
accepted
with 70 votes in favor and one vote against. We will work to finalize
and
merge the implementation in the next few days.

Any update on when this will be merged? Thanks!

Sorry for the long delay here. I've spent the last week fixing the
remaining issues and cleaning up the patch, and...

...Typed properties are now merged into master! [1]

It would be great if people could start experimenting with typed
properties
(e.g. by migrating over existing @var annotations and seeing what
happens).
The earlier we find issues in the implementation or semantics, the better.

Thanks to everyone who worked on this RFC and participated in discussions.
Special thanks to Bob Weinand who did most of the initial implementation
work on this RFC.

Regards,
Nikita

[1]:

https://github.com/php/php-src/commit/e219ec144ef6682b71e135fd18654ee1bb4676b4

On Wed, 6 Feb 2019 at 12:38, Benjamin Morel benjamin.morel@gmail.com
wrote:

While playing with typed properties, something that annoys me a lot, in the
context of a data mapper, is not to be able to use isset() to check whether
an object's property is initialized.

This sounds like a reasonable problem to raise...

Would it be possible to introduce another isset() operator, that would
return true for NULL values?

...but this sounds like the wrong solution.

The distinction you need has nothing to do with nulls, it has to do with
the very specific case of "this property currently violates its contract
and I want to treat that as something other than an error". I personally
still think that unset() should raise an error for typed properties, but
apparently there are cunning hacks that it makes possible; as such, 99% of
PHP developers should never need to detect this state.

It therefore makes more sense to me to have this as a specific API relating
to typed properties; if it can be done faster outside the reflection class,
it could be in the form of a new function like
property_is_initialized($object, $propertyName); this would be an extension
of property_exists, that handles the edge case of "declared with a type and
then explicitly unset".

Regards,

Rowan Collins
[IMSoP]