Shadowing of parent instance fields in derived autogenerated parser classes

[quasilyte]

A generated parser class declared instance fields that shadow the base class instance fields:

https://github.com/nikic/PHP-Parser/blob/f767b9fd9f9b7c2f411ca6f28e3bdf06e66a1754/lib/PhpParser/Parser/Php5.php#L20-L27

It looks like this pattern:

<?php

class Base {
    public static function getData1() {
        $x = new static();
        return $x->data;
    }

    public function getData2() { return $this->data; }

    protected $data = ['base'];
}

class Derived extends Base {
    protected $data = ['derived'];
}

var_dump(Base::getData1()); // ['base']
var_dump(Derived::getData1()); // ['derived']

$b = new Base();
$d = new Derived();
var_dump($b->getData2()); // ['base']
var_dump($d->getData2()); // ['derived']

Which is reported by some linters as a bad code (example: https://github.com/kalessil/phpinspectionsea/blob/master/docs/architecture.md#class-overrides-a-field-of-a-parent-class)

Since Nikita has divine knowledge of the PHP internals (:heart:), I would like to ask what differences we would get from these two approaches:

• Shadowing the parent instance field to override the default initializer (the code in question above)
• Assigning a new value inside a constructor

There might be subtle differences internally, but it looks like the derived class can't access base class instance field (parent::$field would work only for static fields), nor can the base class see overridden fields in any way.

Another question is: is it OK to adjust the parser generator code to produce a constructor that initialized fields with new values instead of doing this kind of field shadowing?

Some context: I'm trying to build a php-parser with KPHP compiler. There are a lot of things to do in order to achieve that inside the KPHP compiler, but this code pattern looks a little bit odd and it's not obvious whether supporting it is a good thing.

Thank you for the attention. :open_hands:

---

The code above could be re-written as:

<?php

class Base {
    protected $data = ['base'];
}

class Derived extends Base {
   // No more shadowing.
   //  protected $data = ['derived'];
  public function __construct() {
    $this->data = ['derived'];
  }
}

[quasilyte]

One gotcha:

• If parent::__construct is called before these values are assigned, there will be a difference
• If it's called after fields are initialized, there will be no difference

[nikic]

Moving these assignments into the constructor should be fine. initReduceCallbacks() already does that for the one part that can't be represented as a constant expression.