Shadowing of parent instance fields in derived autogenerated parser classes
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 17.5k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
A generated parser class declared instance fields that shadow the base class instance fields:
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 (❤️), 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. 👐
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'];
}
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with lib/PhpParser/Parser/Php5.php lines 20-27 and trace the parser generator that produces these generated classes. Compare parent-field shadowing with constructor assignment in the relevant PHP behavior, then determine whether changing the generator is compatible; done means a documented decision and, if warranted, updated generated output and generator behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- compilers, devtools
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100