Simplify first line in constructor doc comments to "Constructor."

Created on 28 June 2024, 2 months ago
Updated 20 July 2024, about 2 months ago

Problem/Motivation

History / current situation

In #2140961: Allow constructor methods to omit docblocks it was decided that the docblock on a constructor method is now optional.
The documentation at https://www.drupal.org/docs/develop/standards/php/api-documentation-and-... was updated accordingly.
See the change record at https://www.drupal.org/node/3365111

At the same time: If a constructor does have a docblock, it needs to follow the existing conventions, which includes a first line comment.

Possible reasons to keep the doc comment:

  • There is one or more parameter that is not obvious, and that needs documentation.
  • The constructor has some special behavior that needs explanation.

Currently, the unwritten convention for constructor doc comments first line is "Constructs a new *** object.", where *** is the class name (without namespace).

This is currently not written down anywhere that I know of, but I have seen it being requested in code reviews.

Problem

As was pointed out in multiple comments in #2140961: Allow constructor methods to omit docblocks , this pattern is very suboptimal:

  • The information it conveys is redundant, because we already know the class name.
  • It is unnecessarily distracting.
  • It is easy to end up with a wrong doc comment, if the doc is copied from one class to another.
  • It is easy to end up with a wrong doc comment, if the class is renamed but the comment remains unchanged.

Proposal

I propose that the first line of constructor doc comments should simply read "Constructor." by default.
For special cases, it can point out how a specific constructor is different, e.g. "Private constructor.".
This does not follow the general verb pattern "Does xyz", but this is fine imo.

class LemonRatio {
  /**
   * Constructor.
   *
   * This constructor does nothing special really.
   * But we need at least two lines to document this.
   *
   * @param int $number
   *   The number of lemons in one glass of lemonade.
   */
  public function __construct(
    public readonly int $number,
  ) {}
}

Alternatives

We could simply omit the first line comment.
I see some problems with this:
- It would look inconsistent next to other comments.
- It lacks the recognizable visual signature.
- If we need additional multi-line doc text, this would now appear at the top.

Benefits

If we adopted this change, the Drupal Project would benefit by ...

  • Easier to write constructors.
  • Easier for an IDE to generate a constructor from a predefined pattern.
  • Reduced mental load / distraction when looking at a constructor.
  • Easy to recognize the constructor due to a fixed visual signature of the first line comment.
  • Less likely to end up with mistakes in the documentation.

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

TBD.
I think a new section could be added that specifically talks about constructors.
I am not sure where this would go.
We have a big section with many types of functions, but methods are only mentioned as part of the "classes and namespaces" section.

Remaining tasks

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

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

🌱 Plan
Status

Active

Component

Coding Standards

Created by

🇩🇪Germany donquixote

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

Comments & Activities

  • Issue created by @donquixote
  • 🇮🇹Italy mondrake 🇮🇹

    +1 on the issue, but -1 on the proposal: it states the obvious.

    I'd rather

    omit the first line comment.

    as the alternative suggestion says.

    Other projects just add extra annotations (including phpstan or psalm ones) for the parameters where it's relevant to add them.

  • 🇩🇪Germany donquixote

    omit the first line comment.

    How do you want to handle constructors that need additional doc text as explanation?

    E.g. Drupal\Component\Annotation\Plugin

    class Plugin implements AnnotationInterface {
    [..]
      /**
       * Constructs a Plugin object.
       *
       * Builds up the plugin definition and invokes the get() method for any
       * classed annotations that were used.
       */
      public function __construct($values) {
    

    This would become

    class Plugin implements AnnotationInterface {
    [..]
      /**
       * Builds up the plugin definition and invokes the get() method for any
       * classed annotations that were used.
       */
      public function __construct($values) {
    

    I used this regex to find examples:

    /\*\*(
     *\* [^@].*|
     *\*){3}(
     *\* .*)*
     *\*/
     *public function __construct
    

    In general, logic in constructors should be avoided.
    But sometimes it can't, or it is already there and we can't remove it for BC reasons.

    it states the obvious.

    It is true.
    But it also gives some visual orientation, if the actual method name is further below.
    And it is easy to generate or just copy/paste.

    Other projects just add extra annotations (including phpstan or psalm ones) for the parameters where it's relevant to add them.

    Or they omit documentation, and then are super confusing to read.
    (just to make the point that the grass is not always greener elsewhere)

    As for the "extra annotations .. for the parameters", I assume you just mean regular old `@param` docs, just not for all parameters?
    There are already issues talking about the option of an incomplete collection of param docs.

    Imo we can still do the change proposed here and then further relax in future issues.

  • 🇦🇹Austria drunken monkey Vienna, Austria

    +1

    Personally, I’ve been using “Constructs a new class instance.” as the first line, which still follows the “start with third-person verb” rule while also solving the listed problems. But I do see the appeal of making this even shorter to make it easier to spot, so would be fine changing it to “Constructor.” as well. If there was a vote, though, I’d go for “Constructs a new class instance.” – even though it’s a bit longer, it’s in line with the almost universal format for method doc comments and once you read “Constructs” you already know what’s going on.

Production build 0.71.5 2024