dependencySerializationTraitProperty.unsupportedPrivateProperty false positive for non-service properties

[travis-bradbury]

Bug report

dependencySerializationTraitProperty.unsupportedPrivateProperty reports an issue for properties that aren't relevant to DependencySerializationTrait. See #1024 for an example.

DependencySerializationTrait only looks at properties that are objects and services (by looking up the service ID from the value) https://git.drupalcode.org/project/drupal/-/blob/main/core/lib/Drupal/Core/DependencyInjection/DependencySerializationTrait.php#L39. A property that's a scalar value (eg, bool) is trivially serializable and is ignored by the trait, but it reported by dependencySerializationTraitProperty.unsupportedPrivateProperty.

Code snippet that reproduces the problem

Example 7c0932e.

[travis-bradbury]

In my code, there's a handful of places this happens. In one simple case I've rewritten it to avoid this, but another matches a pattern we have in core: entity forms with a property for the entity.

It looks like this:

final class FooConfirmForm extends FormBase implements ConfirmFormInterface {

    private ?Foo $foo;
}

FormBase uses DependencySerializationTrait, so we get the error. The same pattern is in core's ContentEntityForm:

class ContentEntityForm extends EntityForm implements ContentEntityFormInterface {

  /**
   * The entity being used by this form.
   *
   * @var \Drupal\Core\Entity\ContentEntityInterface|\Drupal\Core\Entity\RevisionLogInterface
   */
  protected $entity;

That won't trigger dependencySerializationTraitProperty.unsupportedPrivateProperty since it's protected, but it's essentially the same as what I'm doing in my forms. In both cases, the class uses DependencySerializationTrait and has properties that are not services.

[mglaman]

The service lookup is not where private properties break. __sleep() returns array_keys(get_object_vars($this)), and get_object_vars() runs in the scope of the class that composes the trait. Private properties declared on a subclass are not visible from there, so they are left out of the serialized data regardless of type. On unserialize they fall back to their defaults, or stay uninitialized if there is none.

A round trip with the flag set to a non-default value (PHP 8.5):

trait T { public function __sleep(): array { return array_keys(get_object_vars($this)); } }

final class FinalDirect { use T; private bool $flag = false; /* constructor sets true */ }
class Direct { use T; private bool $flag = false; /* constructor sets true */ }
class DirectSub extends Direct {}
class Base { use T; }
class Child extends Base { private bool $flag = false; /* constructor sets true */ }

FinalDirect  flag after round trip: true
Direct       flag after round trip: true
DirectSub    flag after round trip: false
Child        flag after round trip: false

The FooConfirmForm extends FormBase case with private ?Foo $foo is Child: $foo is lost when the form is serialized, for example when the form state is cached. The rule is correct there, and the fix is protected.

The one false positive is a final class that composes the trait itself, like the fixture in #1024 if it were final. A non-final class that composes the trait directly still breaks as soon as it is subclassed (DirectSub). I'm open to exempting that narrow case, but the report as a whole is working as intended.