Contrib.social

Disallow PHPDoc comments within a functions parameter declaration

Open on Drupal.org →
Created on 8 April 2025, 5 days ago

Problem/Motivation

A fun new way to document promoted properties has popped up in the wild.

 /**
   * The constructor for dependency injection.
   */
  public function __construct(
    /**
     * The logger.
     *
     * @var \Drupal\Core\Logger\LoggerChannelFactoryInterface
     */
    private readonly LoggerChannelFactoryInterface $loggerChannelFactory,
    /**
     * The Entity Manager.
     *
     * @var \Drupal\Core\Entity\EntityTypeManagerInterface
     */
    private readonly EntityTypeManagerInterface $entityTypeManager,
  ) {
  }

However, this is not how we want to document promoted properties because those should have @param annotations for the function instead (or be omitted altogether)

Comments within parameters are not currently linted because they may be needed for /* string $futureParam */ (not that's not a docblock which starts with /**, two asterisks). Additionally, there's no warning for the lack of @param annotations because this is now allowed for constructors to ease up on documentation that only duplicates the service container.

It was discussed on Slack that specifically linting on PHPDoc blocks would avoid this pattern and can guide the user to fully omitting or a PHPDoc for the function itself, while still allowing future parameters to be documented.

Benefits

If we adopted this change, the Drupal Project would benefit by ensuring constructors are still consistently documented.

Three supporters required

  1. https://www.drupal.org/u/ → {userid} (yyyy-mm-dd they added support)
  2. https://www.drupal.org/u/ → {userid} (yyyy-mm-dd they added support)
  3. https://www.drupal.org/u/ → {userid} (yyyy-mm-dd they added support)

Proposed changes

Provide all proposed changes to the Drupal Coding standards → . Give a link to each section that will be changed, and show the current text and proposed text as in the following layout:

1. {link to the documentation heading that is to change}

Add current text in blockquotes

Add proposed text in blockquotes

2. Repeat the above for each page or sub-page that needs to be changed.

Remaining tasks

  2. Add supporters
  3. Create a Change Record
  4. Review by the Coding Standards Committee
  5. Coding Standards Committee takes action as required
  6. Discussed by the Core Committer Committee, if it impacts Drupal Core
  7. Final review by Coding Standards Committee
  8. Documentation updates
    1. Edit all pages
    2. Publish change record
    3. Remove 'Needs documentation edits' tag
  9. If applicable, create follow-up issues for PHPCS rules/sniffs changes

For a full explanation of these steps see the Coding Standards project page →

✨ Feature request
Status

Active

Component

Coding Standards

Created by

🇳🇱Netherlands kingdutch

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

Comments & Activities

  • Issue created by @kingdutch
  • Comment 5 days ago →
    🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    I would like to get this in as quickly as possible, so we don't see this style be used more often.
    There's no phpcs rule for this yet, but if we agree on this, I will create an issue there.

  • Comment 4 days ago →
    🇦🇺Australia acbramley

    +1, this is completely redundant documentation and just makes it harder to read. If parameters need documentation it should be in the constructor's doc block.

  • Comment 3 days ago →
    🇳🇿New Zealand quietone

    This needs the section "1. {link to the documentation heading that is to change}" completed so we are all clear on what the change is.

  • Comment 3 days ago →
    🇬🇧United Kingdom catch

    I think it's already covered by https://www.drupal.org/docs/develop/standards/php/api-documentation-and-... → and I'm not sure we actually need to change anything if so. The standards don't allow for this format, it's just that coder doesn't explicitly disallow it. However we probably need to explicitly agree that no change is needed before opening a coder issue? We could add something to the coding standards to point out this is bad, but I think that would potentially just be confusing - better to have positive examples.

  • Comment 3 days ago →
    🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    I agree with catch, we don't document all the negative examples and I don't think we should start doing that. We already have a lot of documentation.

  • Comment about 17 hours ago →
    🇦🇹Austria drunken monkey Vienna, Austria

    Agreed that it’s important to keep this from becoming common, but probably already covered by the linked documentation. Let’s just adapt our tools to spot this.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024