Problem/Motivation
In #3129184: Remove description from @return $this pointed that core should not provide description for @return $this but that's not pointed in coding standards https://www.drupal.org/docs/develop/standards/api-documentation-and-comm...
Update coding standards about that case and introduce sniff #3326850: Warn if @return $this has description
The current sniff allows for no description of "@return $this" and "@return static".
Benefits
If we adopted this change, the Drupal Project would benefit by being more consistent.
Three supporters required
- https://www.drupal.org/u/borisson_ (2025-02-11)
- https://www.drupal.org/u/drunken-monkey (2025-02-15)
- https://www.drupal.org/u/joachim (2025-02-18)
Proposed changes
Change documentation to explicitly allow no description for "@return $this", "@return self", and "@return static".
Change the relevant sniff to allow not no description for "@return self".
1. https://www.drupal.org/docs/develop/standards/php/api-documentation-and-comment-standards#types
Current text
Syntax notes:
- ...
- When you are returning the main class object ($this), use
@return $this.- When creating a new instance of the same class, use
@return static.Drupal standards notes:
- ...
- You may omit the description if using
@return $thisor@return static.Proposed text
Syntax notes:
- ...
- When you are returning the main class object ($this), use
@return $this.- When creating a new instance of the same class, use
@return static.- When creating a new instance of the same class, and that class is final, use
@return self.Drupal standards notes:
- ...
- You may omit the description if using
@return self,@return static, or@return $this.2. Repeat the above for each page or sub-page that needs to be changed.
Remaining tasks
Create this issue in the Coding Standards queue, using the defined templateAdd supporters- Create a Change Record
- Review by the Coding Standards Committee
- Coding Standards Committee takes action as required
- Discussed by the Core Committer Committee, if it impacts Drupal Core
- Final review by Coding Standards Committee
- Documentation updates
- Edit all pages
- Publish change record
- Remove 'Needs documentation edits' tag
- If applicable, create follow-up issues for PHPCS rules/sniffs changes
For a full explanation of these steps see the Coding Standards project page
Issue fork coding_standards-3326851
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
quietone commentedWe should check the history to see if "return $this" was discussed earlier.
Also, from longwave, we should do discuss, "@return self" and "@return static". #3129184-6: Remove description from @return $this
Comment #3
drunken monkeyIt’s documented a bit further down, under Indicating data types in documentation. But yes, I also often struggle to find things in our documentation.
Comment #4
quietone commentedUpdate to new issue template
Comment #5
borisson_This is already in the docs. So that should be fine already? What do we need to do for this issue?
Comment #6
quietone commented@borisson_, thanks for finding that sentence! I surely missed it.
So, the sentence is in the section "Indicating data types in documentation" which is the wrong place. It should be with the '@return'. I have updated the issue summary with the proposal. The difference is also to add
@return self.Comment #7
quietone commentedTesting with Drupal.Commenting.FunctionComment.MissingReturnComment enabled and "@return $this", and "@return static" pass without a comment but "@return self" fails with "Description for the @return value is missing".
Comment #8
quietone commentedImproving the issue summary
Comment #9
borisson_Comment #10
quietone commentedUpdated title. I also changed the current and proposed text since the information is in a different section.
There is a TBA in the proposed text which needs to completed.
Comment #11
borisson_Filled in the when TBA,
return static is only useful because it can do late binding, which is really helpful when that class is extended by another one. However final classes cannot be extended, so they can use self instead.
Comment #12
drunken monkeyAdded a comma in the proposed text, but looks good otherwise.
Comment #13
joachim commented+1
Comment #14
quietone commentedSetting to NW and tagging for a change record.
Comment #15
borisson_Added a change record.
We need to update the text change to a merge request still
Comment #16
quietone commentedComment #17
quietone commentedCreated an MR. The syntax notes in the issue summary were removed in #3527545: Formatting fixes, whitespace, links, rename files