- Issue created by @larowlan
- πΊπΈUnited States nicxvan
Personally I think it is fine as is for a couple of reasons.
1. That is a warning that can be ignored by the code extending it, that is precisely the point of @internal as far as I see it. (Also why I advocate against final so strongly, if this had been final you wouldn't be able to extend it. At least here you have a choice and get a warning)
2. Level 8 is significantly higher than most people are running php stan, what level do you start getting earnings about @internal. - πΊπΈUnited States tim.plunkett Philadelphia
#2.1 That's not the point, RenderElementBase is NOT internal, only the property
has_garbage_value
is, but because of how the PHPDocs were written it is being parsed as the whole class being internal.#2.2 Project Browser runs on Level 8, and also provides a Render Element, and will start failing on this.
- πΊπΈUnited States nicxvan
I was thinking of:
/** * The storage. * * @internal */ protected array $storage = [];
But this seems to be about the properties documented on the class itself.
PHPStan interpreting the property being set to internal as the whole class seems like a bug in phpstan, probably worth an upstream issue to resolve.
In the short term I think we can do one of two things.
1. Change it back to how we defined it as internal before with just the comment
Internal
2. Move that code block somewhere else in the class until they fix the upstream issue.I would strongly prefer 2 since that property is internal.
Regarding your second point (separately from the bug that the whole class is being interpreted as internal), my point wasn't that other modules don't use level 8, but when it comes to @internal and @final phpstan is more about notifying you rather than something you have to fix.
In my mind if phpstan throws an error for something like
a variable is not defined
, that is something you need to address because it will throw errors.On the other hand when phpstan says something like you're extending @internal code, you have to look and evaluate if you're willing to take on the risk that entails, but it's not a bug in your code, phpstan is just warning you here. The solution downstream is to put an ignore rule acknowledging that risk.
One is about potential bugs in your code, the other is a warning to ensure you did something intentionally.