Contrib.social

Stop using FQCN in PHPDoc annotations

Open on Drupal.org →
Created on 13 May 2023, almost 2 years ago
Updated 11 September 2024, 7 months ago

Problem/Motivation

When an object variable or a parameter or a returned value is typehinted, the current policy requires the FQCN of the object to be used.

For example, 

   * @param array|\Drupal\Component\HtmlAttribute\HtmlAttributeCollection $attributes
   *   An optional array or HtmlAttributeCollection object of link attributes.

However, since the introduction of union types in PHP 8, readibility of such annotations got worse when multiple classes are allowed for a single object.

For example,

   * @param \Drupal\Component\Render\MarkupInterface|\Drupal\Component\HtmlAttribute\HtmlAttributeValueBase|scalar|array<scalar>|null $value
   *   The value to set.

Other projects just annotate the unqualified class name, and rely on the use imports at the top of each file to fully qualify the classes when needed. PHPStan supports this model natively (and IDEs too??).

Allow using the class name only in the annotations. If the class name is not present in the same namespace of the class where the annotation is present, a matching use import should be present to allow matching the referred class existance.

Given the second example above, this will become


namespace Drupal\Component\HtmlAttribute;

use Drupal\Component\Render\MarkupInterface;

   ...

   * @param MarkupInterface|HtmlAttributeValueBase|scalar|array<scalar>|null $value
   *   The value to set.

Benefits

If we adopted this change, the Drupal Project would benefit by having shorter and more readable PHPDoc annotations, which also makes it easier to adopt more complex PHPStan capabilities.
It also will make the PHPDoc annotations equal to the actual inlined type hints (as long as iterables are not involved, which is a different case). This will help removing unnecessary @var annotations from the code base.

Three supporters required

  1. https://www.drupal.org/u/Kingdutch → (2024-04-10)
  2. https://www.drupal.org/u/borisson_ → (2024-04-10)
  3. https://www.drupal.org/u/bbrala → (2024-04-10)
  4. https://www.drupal.org/u/dww → (2024-04-10)

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. API documentation and comment standards →

Section: Drupal API documentation standards for classes and namespaces →

  • If you use a namespace on a class anywhere in documentation, always make sure it is a fully-qualified namespace (beginning with a backslash).
  • Immediately after an @tag (@param, @return, @var, etc.), class and interface names must always include the fully-qualified namespace.

@todo

2. Indicating data types in documentation → 

int
string|bool
\Drupal\Core\Database\StatementInterface
int[]
\Drupal\node\NodeInterface[]
$this
static

...

  • Always prefix types with the fully-qualified namespace for classes and interfaces (beginning with a backslash). If the class/interface is in the global namespace, prefix by a backslash. 
int
string|bool
StatementInterface
int[]
NodeInterface[]
$this
static

...
@todo (remove entirely?)

Remaining tasks

  3. Finish fleshing out the proposed standards changes
  4. Create a Change Record
  5. Review by the Coding Standards Committee
  6. Coding Standards Committee takes action as required
  7. Discussed by the Core Committer Committee, if it impacts Drupal Core
  8. Final review by Coding Standards Committee
  9. Documentation updates
    1. Edit all pages
    2. Publish change record
    3. Remove 'Needs documentation edits' tag
  10. 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

🇮🇹Italy mondrake 🇮🇹

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

  • Issue created by @mondrake
  • Comment almost 2 years ago →
    🇮🇹Italy mondrake 🇮🇹

    Opened ✨ Stop using FQCN in @param @var @return annotations Active in Coder's issue queue to match this.

  • Comment almost 2 years ago →
    🇦🇹Austria klausi 🇦🇹 Vienna

    I think I agree with this proposal. We could simply drop the check for FQN names in doc comments from Coder. Tools like PHPStan are better in verifying that the class names used there are valid references.

  • Comment about 1 year ago →
    🇳🇱Netherlands kingdutch

    I used to be against this because it led to codebases having `use` import statements for types that weren't used in code but only in comments. However:

    1. That was often specifically for annotations which are already being replaced by Attributes
    2. That was often because PHP did not yet support the type information within the language where now most PHPDoc comments will have a matching parameter or return type hint for which the use statement is already needed
    3. A reference in a comment is still a "usage' because it indicates some kind of contract, so for being able to find "Where something is used" with static analysis tools it's still important in case of deprecations and such; PHPStan already does these things for us which further proves that this is no longer a problem

    Given those developments within PHP, I'm also happy to +1 this proposal :D

  • Comment 12 months ago →
    🇦🇺Australia acbramley

    Big +1 on this, we've got some wildly long union types, could this also apply to @property annotations?

  • Comment 12 months ago →
    🇳🇱Netherlands kingdutch

    Moved into new coding standards template and added myself as supporter.

    Generalised the title to PHPDoc annotations so we don't need to list all @ annotations that it may support.

  • Comment 12 months ago →
    🇧🇪Belgium borisson_ Mechelen, 🇧🇪
  • Comment 12 months ago →
    🇳🇱Netherlands bbrala Netherlands
  • Comment 12 months ago →
    🇮🇹Italy mondrake 🇮🇹

    BTW Rector here can help a lot, https://getrector.com/documentation/import-names

    Working with the configuration of withImportNames one can get (or not) existing FQCNs in PHPDoc be split between the use import and the remaining class name in the docs, can remove (or not) unused use imports.

    Once this is finalized, a single rector run could just clean the entire codebase.

  • Comment 12 months ago →
    🇮🇹Italy mondrake 🇮🇹
  • Comment 12 months ago →
    🇺🇸United States dww
    1. Adding myself as another (enthusiastic!) supporter. I never understood why we required FQCNs for these.
    2. Fixed the dates for supporters (h/t https://xkcd.com/1179 😆)
    3. Started identifying the docs that need updating.

    Tagging for a summary update since I'm out of time to start fleshing out the proposed changes. Also worth reviewing that I found all the spots we need to fix.

    Thanks!
    -Derek

  • Comment 12 months ago →
    🇳🇿New Zealand quietone
  • Comment 7 months ago →
    🇫🇷France andypost
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024