Re: [PHP-DEV] [RFC][Discussion] Add #[NoSerialize] attribute for excluding properties or classes from serialization

Hi Nicolas,

Thank you for the review, and sorry for the delay.

> I checked code bases I know well, Symfony in particular, and I had a
> hard time finding any implementation of __serialize()/__sleep() that
> the property-level attribute would replace

You are right, and the RFC was wrong to imply otherwise. There is no
such __sleep() in Symfony.

The class-level case is there, six times, written identically in
Lock\Lock, Semaphore\Semaphore, Process\Pipes\UnixPipes,
ErrorHandler\BufferingLogger, HttpClient\Chunk\ErrorChunk and
Form\Util\OrderedHashMapIterator:

    public function __serialize(): array
    {
        throw new \BadMethodCallException('Cannot serialize '.__CLASS__);
    }

For the property-level case, Magento 2 has 31 classes with __sleep(),
chained so that each level strips its own injected services. Each
class below extends the one above it:

    Framework\Model\AbstractModel: -8 properties
    Framework\Model\AbstractExtensibleModel: -2
    Eav\Model\Entity\Attribute\AbstractAttribute: -12
    Eav\Model\Entity\Attribute: -4
    Catalog\Model\ResourceModel\Eav\Attribute: -4

Every level is array_diff(parent::__sleep(), [...]) with a mirrored
__wakeup() re-injecting the same services, so each property is named
twice. Same shape in their collection hierarchy, in Store and in User
(13 properties). Smaller whitelist form: Laravel's RateLimited and
Doctrine's PersistentCollection.

> having objects that embed both value-state and a PDO connection looks like a design smell

Agreed as a principle, and PDO was a bad headline example. But this is
the normal shape of an ORM model in Magento and Laravel, and it will
not be redesigned away. __serialize() is not a substitute: it requires
describing the whole format by hand, which is what produces those
array_diff() chains.

The introduction has been rewritten around these examples.

> contracts propagate naturally to child classes. Attributes don't, and
> making this one an exception raises the question: why break attribute
> rules

It is not an exception. #[AllowDynamicProperties] already behaves this
way, since 8.2:

    #[AllowDynamicProperties]
    class P {}
    class C extends P {}

    $c = new C;
    $c->dyn = 1;                 // no deprecation: flag is inherited
    $r = new ReflectionClass('C');
    $r->getAttributes(AllowDynamicProperties::class);   // array(0) {}

zend_inheritance.c copies ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES from the
parent, alongside ZEND_ACC_NOT_SERIALIZABLE.

The interface alternative was argued at length in the 2024 thread and
the list did not converge: Eberlei, Banyard and Dusk for the
attribute, you and Larry for the interface.

One cost that was not raised there: this RFC migrates 107 internal
classes across 64 stub files. As an interface, all of them would gain
a new entry in class_implements() — observable userland state, and a
larger BC surface than an attribute.

If the list prefers an interface, that is the list's call, but it
should be made deliberately rather than as a side effect here.

> we would need reflection-based checks on every property during both
> serialization and unserialization

I checked var-exporter. The cost is per-class on 7.x, and on 8.1 it is
not reflection at all.

Class-level is free: it sets the existing ZEND_ACC_NOT_SERIALIZABLE.
On 7.x Registry::unserialize() calls native unserialize(), which
already guards that flag, and getClassReflector() probes with
serialize($proto) once per class. ext-deepclone tests the flag
directly (deepclone.c:519, :5234).

Property-level on 8.1: VarExporter::export() delegates to
deepclone_to_array(), whose fast path already iterates
ce->properties_info_table and tests prop_info->flags —
ZEND_ACC_STATIC, ZEND_ACC_PUBLIC, ZEND_ACC_PROTECTED_SET,
ZEND_ACC_PRIVATE_SET (deepclone.c:2453-2489). ZEND_ACC_NO_SERIALIZE is
one more condition in a loop that already runs. The polyfill does use
reflection, but per class on both sides: $scopeMaps for export
(DeepClone.php:785-795), $propertyScopes for hydration (:453).

For the engine the comparison runs the other way: a ce_flags bit test
is cheaper than instanceof, which is why the engine uses a flag.

Three changes to the RFC since your message:

* The introduction cites the codebases above instead of asserting that
the use case is common.
* Internal classes are migrated explicitly, as @deprecated was
migrated to #[\Deprecated] in 8.4, rather than having the attribute
injected during code generation. @not-serializable stays supported for
third-party extensions. This follows Tim's suggestion.
* unserialize() is now symmetric: a #[NoSerialize] property present in
the payload is parsed and discarded rather than restored. The
attribute now means "not part of the serialized form" in both
directions, matching transient in Java and [NonSerialized] in C#, and
removing an injection surface. The old behaviour is recorded under
Rejected Features.

Best regards,
Dmytro Kulyk