PHP: rfc:propertygetsetsyntax-as-implemented:change-requests

This page is read only. You can view the source, but not change it. Ask your administrator if you think this is wrong.
====== v1.1 -> v1.2 ======
    * [[rfc/propertygetsetsyntax-as-implemented|Versions 1.1 RFC]]


===== Current Consensus Changes =====

==== Syntax Change ====
<del>[[https://github.com/cpriest/php-src/issues/6|https://github.com/cpriest/php-src/issues/6]]</del>

Allow syntax to use parenthesized syntax as well so that both a [TypeHint] and $value may be specified.

<code php>
class TimePeriod {
    private $Seconds;

    public $Hours {
        get() { return $this->Seconds / 3600; }
        set([TypeHint] $value) { $this->Seconds = $value * 3600; }
        isset() { return isset($this->Seconds); }
        unset() { unset($this->Seconds); }
    }
}
</code>

The abbreviated syntax is also allowed as per 1.1 RFC.

==== read-only / write-only keywords ====
<del>[[https://github.com/cpriest/php-src/issues/7|https://github.com/cpriest/php-src/issues/7]]</del>

read-only / write-only keywords will be eliminated, the issue will be pushed to userland developers who can enforce the same by coding something like this:

<code php>
class TimePeriod {
    private $Seconds;

    public $Hours {
        get() { return $this->Hours; }
        private final set($value) { throw new Exception("Setting of TimePeriod::$Hours is not allowed."); }
    }
}
</code>

This was previously being debated, the full details of this are in the last section under "Previously Debated"

==== Automatically Implemented Accessors ====
[[https://github.com/cpriest/php-src/issues/5|https://github.com/cpriest/php-src/issues/5]]

v1.1 properties shadow (or over-ride) accessors, so if an accessor exists and a property is defined (only possible from within the accessors setter), then the property would shadow the accessor.  This was done to keep it consistent with what %%__get()%% does and also to support lazy loading.

Nikita suggested that this should be inverted, that accessors shadow properties.  With that comes the advantage that automatically implemented get/set could use the same property name as the accessor (no more magic %%$__Hours%% property):

<code php>
class TimePeriod {
    private $Seconds;

    public $Hours {
        get();
        set([TypeHint] $value);
        isset();
        unset();
    }
}

/* Would automatically be implemented as */

class TimePeriod {
    private $Seconds;

    public $Hours {
        get() { return $this->Hours; }
        set([TypeHint] $value) { $this->Hours = $value; }
        isset() { return $this->Hours !== NULL; }
        unset() { $this->Hours = NULL; }
    }
}
</code>

==== Shadowing ====
[[https://github.com/cpriest/php-src/issues/11|https://github.com/cpriest/php-src/issues/11]]

Accessors will shadow properties such that if a property named $foo is declared and an accessor for $foo is declared, the accessor will be used instead, only the accessor will have direct access to the underlying property.

<code php>
class TimePeriod {
    private $Seconds;

    public $Seconds {
        get() { return $this->Seconds; /* Accesses the private $Seconds directly */ }
        set($x) { $this->Seconds = $x; /* Accesses the private $Seconds directly */ }
    }
}

$o = new TimePeriod();
$o->Seconds = 45;  /* Calls set accessor */
echo $o->Seconds;  /* Calls get accessor */
</code>

==== isset / unset / attempted writes when no setter / attempted reads when no getter ====
[[https://github.com/cpriest/php-src/issues/12|https://github.com/cpriest/php-src/issues/12]]

Invalid calls to isset/unset will fail silently and return false in the case of isset(), no warning/notice should be thrown.

<code php>
class TimePeriod {
    private $Seconds;

    protected $Seconds {
        get() { return $this->Seconds; /* Accesses the private $Seconds directly */ }
        set($x) { $this->Seconds = $x; /* Accesses the private $Seconds directly */ }
    }
}

$o = new TimePeriod();
isset($o->Seconds); /* Returns false, no error thrown */
unset($o->Seconds);  /* Does nothing, no error thrown */
</code>

==== Extra shorthand declaration ====

Not much further support and good arguments against it, dropped.

==== Interfaces ====
[[https://github.com/cpriest/php-src/issues/13|https://github.com/cpriest/php-src/issues/13]]

An implementing class may specify a property with an appropriate access level to satisfy an accessor declaration requirement of an interface.
<code php>
interface i {
    public $Seconds { get; }
}

class TimePeriod implements i {

    /* Satisfies interface implementation requirements of interface i */
    public $Seconds;
}

</code>

===== Being Debated =====

==== internal accessor method visibility / callability ====

Some people are in favor of the internal functions being generated by an accessor declaration should be invisible and non-callable directly.  Others are in favor of leaving them visible and callable.

  - v1.1 currently has them callable but not visible via reflection.
  - v1.1 also alters any error which would reference an internal accessor and changes the error message to make sense from the perspective of the userland php developer.
    - Examples:
      - Fatal error: Cannot override final property getter TimePeriod::$Hours
      - Fatal error: Cannot set read-only property TimePeriod::$Hours.

=== Type 1 ( Userland Programmer ) ===

As a userland programmer, someone who cares nothing for "how" php works, only how their own code works.  If they define an accessor they expect to see an accessor, reflection should reflect that there are accessors and no other "methods" they did not explicitly define.  If they were to reflect on all of the methods of their class and see a number of %%__getHours()%% they may be confused as to why or where this function came from.  From their perspective, they have defined an accessor and "how" that accessor works on the inside is of no importance to them and only seeks to complicate or confuse matters when they are exposed to these "implementation details" of the php language its-self.  If you tried to set a value such as $obj->abc = 1 through an accessor which could not be set, you would probably want to see an error like: Warning, cannot set Class->abc, no setter defined.

=== Type 2 ( Internals Programmer ) ===

As an internals programmer, you want nothing hidden from you.  If an accessor implements special %%__getHours()%% methods to work its magic, then you want to see them, you want to call them directly if you so choose.  In effect you want nothing hidden from you.  In this case you probably don't even want Reflection to reflect accessors as anything different than specially formatted and called methods on the class.  This can be understandable because you want all information available to you.  You would probably not be confused if you wrote $obj->abc = 1 and got back an error like "Fatal Error: %%Class->__setAbc()%% function does not exist.

**Unfortunately 80 to 95% of all people who use PHP are of the first type.**  

Revealing these internal matters to them would only leave them confused, possibly frustrated //and likely asking about it to the internals mailing list to answer (repeatedly)//.

===== Issues =====

==== Static Accessors ====
Stas pointed out a number of critical mistakes with the current implementation of static accessors.  They would not currently work under these circumstances:
  - Class defined after a static accessor reference was made
  - Dynamic variable use of an accessor --> $class::$variable = 5;

The current implementation "back-patches" the op_array at the time of a static variable declaration by patching the op array into a method call.  zend_do_assign() further detects these calls to %%ex: __getStaticHours()%% into %%__setStaticHours()%%, and a change to pass_two() detects invalid static accessor gets (those which were not properly backpatched with zend_do_assign())

Stas suggested that he could either help me to implement this more appropriately (such as changing the engine to make calls into zend_object_handler.c) so that these calls can be resolved dynamically, or that static accessors be shelved for a future version of accessors.

**Update 12/30/2012**: Too much engine work is necessary to implement static accessors, this is being pushed to a future RFC change.
===== TODO =====
  - <del>Exceptions thrown from an accessor reveal underlying implementation details</del> [[https://github.com/cpriest/php-src/issues/14|github#14]]
  - <del>get_class_methods() - hide accessors or re-write to call into Reflection* code?</del> [[https://github.com/cpriest/php-src/issues/15|github#15]]
  - <del>Check debug_backtrace() output as well</del> [[https://github.com/cpriest/php-src/issues/16|github#16]]
  - <del>Add notes to RFC on shadowing behavior of properties and acessors [[https://github.com/cpriest/php-src/issues/17|github#17]]</del>
  - <del>Fix automatically implemented isset to use !== rather than != (Thanks Stas!)</del>
  - <del>Check that the following should work and write tests for them:</del> [[https://github.com/cpriest/php-src/issues/18|github#18]]
    - <del>$foo->bar[] = 1</del>
    - <del>$foo->bar[123] = 4</del>
    - <del>$foobar = &$foo->bar</del>
    - <del>$foo->bar->baz = $x with $bar being object or empty or non-object value (proper error and no leaks -- Stas)</del>
    - <del>$foo->bar{$x} = "foo" (string offsets), also checks that non-string vars work fine</del>
    - <del>$foo->bar = &bar->baz, etc. with modifying both sides and checking everything works (including modifying via accessors -- Stas)</del>
  - <del>Expand on the "Error Messaging" with examples of what has been translated. [[https://github.com/cpriest/php-src/issues/17|github#17]]</del>
  - <del>Move all accessor tests into Zend/tests/accessors</del>

===== Previously Debated =====

==== read-only / write-only keywords ====
read-only / write-only keywords replacement.  Just about everyone is against these two new keywords, no equivalent replacement has been suggested (see below).

**For Clarity**: read-only enforces upon the defining class and all sub-classes that no setter may be defined, a type of final that restricts the defining of a setter.  The inverse is true for write-only.

Possible alternatives:

This would prevent sub-classes from changing the setter, however this would produce an automatically implemented setter which would be callable from within the defining class.  It's close, but not identical.
<code php>
class TimePeriod {
    private $Seconds;

    public $Hours {
        get() { return $this->Hours; }
        private final set([TypeHint] $value);
    }
}
</code>

This could be an alternative that could be implemented internally exactly the same as a read-only or write-only:
<code php>
class TimePeriod {
    private $Seconds;

    public $Hours {
        get() { return $this->Hours; }
        final set NULL;
    }
}
</code>

I have also suggested that perhaps the read-only and write-only functionality (type of final) is not even needed or desired.

**Possible Consensus:** Eliminate read-only/write-only keywords and let userland programmers enforce it with something like:
<code php>
class TimePeriod {
    private $Seconds;

    public $Hours {
        get() { return $this->Hours; }
        private final set($value) { throw new Exception("Setting of TimePeriod::$Hours is not allowed."); }
    }
}
</code>

==== Shadowing ====
v1.1 has properties shadow accessors, the suggestion is that this be inverted.

Specifically this means that in v1.1, if an accessor is defined and no property is defined, then the accessor will be used.  But when a property is assigned/declared then the property will "shadow" the accessor and the accessor will no longer be used until the property is unset().

v1.2 Proposes that this be inverted such that if there is an accessor defined for a given property name, the accessor will always be used.  The accessor would be able to get/set/isset/unset the property with the same name as the accessor.  No direct access to the property would be allowed except from within the accessor.  

v1.2 proposal seems to make the most sense however it would incur a slight (possibly *very* slight) performance penalty.

==== isset / unset / attempted writes when no setter / attempted reads when no getter ====
Stas suggested that since there is presently no cases where these can fail that with accessors these should never "fail."

Three possible ways to go (maybe others):
  - If all cases can be tested for during compilation, prefer compile failures.
  - Let the compilation occur and at runtime when a disallowed action is attempted, emit a warning and move on.
  - As is currently, either at compilation or at runtime we issue a fatal error and stop execution (probably least preferable if at runtime)

==== Extra shorthand declaration ====
Allow extra short declaration of an accessor:
<code php>
class TimePeriod {
    public DateTime $date;
}

/* Would be equivalent to this */

class TimePeriod {
    public $date {
        get() { return $this->date; }
        set(DateTime $value) { $this->date = $value;}
    }
}
</code>

==== Interfaces ====
v1.1 fully allows accessors to be defined in interfaces and requires that those accessors be defined in implementing classes.  Some people have suggested that a property defined by the class should also satisfy the requirement.

=== Arguments For ===
  - From the outside observer of an interface, a class which defines a property has satisfied the requirement, because an interface only defines what must be allowed, not what cannot be done.

=== Arguments Against ===
  - Additional overhead on interface checking, would only be allowed for a non-asymmetrical accessor
  - Would give userland developers the ability to write poor code as properties are non-observable (object would not know its property was changed)
Page Tools
