Use of @internal in RenderElementBase property attributes yields phpstan errors

Created on 3 July 2025, 5 days ago

Problem/Motivation

πŸ“Œ Slowly, very slowly start OOPifying the render system Needs review added @property docs to render element plugins.
On RenderElementBase it marked some of these as @internal

This confuses phpstan which things the whole class is internal

Steps to reproduce

Install 11.x-dev
Have a contrib project with phpstan and @mglaman/phpstan-drupal on level 8
See errors like this

 18     Class Drupal\experience_builder\Element\RenderSafeComponentContainer
         extends @internal class Drupal\Core\Render\Element\RenderElementBase.
         πŸ’‘ Read the Drupal core backwards compatibility and internal API
            policy:
            https://www.drupal.org/about/core/policies/core-change-policies/drupal-8-and-9-backwards-compatibility-and-internal-api#internal

Proposed resolution

Remove @internal from the base class properties and use some other syntax to signify that they're internal

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

11.0 πŸ”₯

Component

forms system

Created by

πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • 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.

Production build 0.71.5 2024