Part of #2571965: [meta] Fix PHP coding standards in core, stage 1.

sub sniffs

Complete

Approach

We are testing coding standards with PHP CodeSniffer, using the Drupal coding standards from the Coder module. We need to do a couple of steps in order to download and configure them so we can run a coding standards check.

Step 1: Add the coding standard

Every coding standard is identified by a "sniff". For example, an imaginary coding standard that would require all llamas to be placed inside a square bracket fence would be called the "Drupal.AnimalControlStructure.BracketedFence sniff".

Open the file core/phpcs.xml.dist and add a line for the sniff of this ticket. The sniff name is in the issue title. Make sure your patch will include the addition of this line.

Step 2: Install PHP CodeSniffer and the ruleset from the Coder module

$ composer install

Once you have installed the phpcs package, you can list all the sniffs available to you like this:

$ ./vendor/bin/phpcs --standard=Drupal -e

This will give you a big list of sniffs, and the Drupal-based ones should be present.

Step 3: Prepare the phpcs.xml file

To speed up the testing you should make a copy of the file phpcs.xml.dist (in the core/ folder) and save it as phpcs.xml. This is the configuration file for PHP CodeSniffer.

We only want this phpcs.xml file to specify the sniff we're interested in. So we need to remove all the rule items, and add only our own sniff's rule. Rule items look like this:

<rule ref="Drupal.Classes.UnusedUseStatement"/>

Remove all of them, and add only the sniff from this issue title. This will make sure that our tests run quickly, and are not going to contain any output from unrelated sniffs.

Step 4: Run the test

Now you are ready to run the test! From within the core/ folder, run the following command to launch the test:

$ composer phpcs

Step 5: Fix the failures

When the test is complete it will present you a list of all the files that contain violations of your sniff, and the line numbers where the violations occur. You could fix all of these manually, but thankfully phpcbf can fix many of them. You can call phpcbf like this:

$ composer phpcbf

This will fix the errors in place. You can then make a diff of the changes using git. You can also re-run the test with phpcs and determine if that fixed all of them.

CommentFileSizeAuthor
#10 Drupal.Commenting.FunctionComment-2572645-10.patch719.63 KBvprocessor
#10 Drupal.Commenting.FunctionComment-2572645-10.interdiff.txt490 bytesvprocessor
#9 Drupal.Commenting.FunctionComment-2572645-9.patch719.15 KBvprocessor
#2 Drupal.Commenting.FunctionComment.patch22.56 KBattiks

Issue fork drupal-2572645

Command icon 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:

About issue forks

Comments

attiks created an issue. See original summary.

attiks’s picture

attiks commented
StatusFileSize
new Drupal.Commenting.FunctionComment.patch22.56 KB
duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Issue tags: -Novice

As agreed between the mentors at Drupalcon, according to issues to avoid for novices, I am untagging this issue as "Beginner". This issue contains changes across a very wide range of files and might create too many other patches to need to be rerolled at this particular time. This patch has an automated way to be rerolled later so better to implement it after Drupalcon.

tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
commented
Status: Needs review » Needs work

Unfortunately it needs more manual work to be ready to commit it, there are a lot of empty @file docs what need to fill up with valid header documentations.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Issue summary: View changes

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

anoopjohn’s picture

anoopjohn commented
Version: 8.1.x-dev » 8.2.x-dev

I am going to try to work through the sub-sniffs in the FunctionComment sniff. I am thinking of tackling the sub-sniffs one by one and grouping them into logical grouping. Here is what I have come up with. The number prefix is the number of failures for the sub-sniff in a run of coder on 8.2.x branch.

Missing Function Comment - I think we may have to reduce the severity level for this / exclude this for a while as it would take quite a bit of effort to understand the respective functions and write meaningful and useful comments

1479 Drupal.Commenting.FunctionComment.Missing

Missing Param Comment

119 Drupal.Commenting.FunctionComment.MissingParamComment

Missing Return Comment

398 Drupal.Commenting.FunctionComment.MissingReturnComment

Missing Return Type

259 Drupal.Commenting.FunctionComment.MissingReturnType

Missing Param Type

1010 Drupal.Commenting.FunctionComment.MissingParamType

Other Issues with Return Comment

8 Drupal.Commenting.FunctionComment.$InReturnType
24 Drupal.Commenting.FunctionComment.InvalidNoReturn
2 Drupal.Commenting.FunctionComment.InvalidReturn
11 Drupal.Commenting.FunctionComment.InvalidReturnNotVoid
42 Drupal.Commenting.FunctionComment.ReturnCommentIndentation
1 Drupal.Commenting.FunctionComment.VoidReturn

Issues with Type Hint

31 Drupal.Commenting.FunctionComment.IncorrectTypeHint
2 Drupal.Commenting.FunctionComment.InvalidTypeHint
75 Drupal.Commenting.FunctionComment.TypeHintMissing

Other minor issues with Param Comment

8 Drupal.Commenting.FunctionComment.IncorrectParamVarName
39 Drupal.Commenting.FunctionComment.MissingParamName
152 Drupal.Commenting.FunctionComment.ParamCommentFullStop
4 Drupal.Commenting.FunctionComment.ParamCommentNotCapital
39 Drupal.Commenting.FunctionComment.ParamNameNoMatch

Issues with @see

2 Drupal.Commenting.FunctionComment.SeeAdditionalText
29 Drupal.Commenting.FunctionComment.SeePunctuation

Spacing / Styling issues - #2708185: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to spacing and styling

7 Drupal.Commenting.FunctionComment.SpacingAfterParamType
1 Drupal.Commenting.FunctionComment.SpacingAfter
10 Drupal.Commenting.FunctionComment.ParamCommentNewLine
8 Drupal.Commenting.FunctionComment.WrongStyle

Issues with @throws - #2708179: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to @throws

5 Drupal.Commenting.FunctionComment.ThrowsComment
4 Drupal.Commenting.FunctionComment.ThrowsNoFullStop
1 Drupal.Commenting.FunctionComment.ThrowsNotCapital

vprocessor’s picture

vprocessor commented
Assigned: Unassigned » vprocessor
vprocessor’s picture

vprocessor commented
Status: Needs work » Needs review
StatusFileSize
new Drupal.Commenting.FunctionComment-2572645-9.patch719.15 KB

Reroll & refix with phpcbf is ready

vprocessor’s picture

vprocessor commented
StatusFileSize
new Drupal.Commenting.FunctionComment-2572645-10.interdiff.txt490 bytes
new Drupal.Commenting.FunctionComment-2572645-10.patch719.63 KB

Sorry, forgot about config - fixed

vprocessor’s picture

vprocessor commented
vprocessor’s picture

vprocessor commented
Assigned: vprocessor » Unassigned
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work

This needs to be split up into the sub sniffs there no way to review that patch.

vprocessor’s picture

vprocessor commented
Assigned: Unassigned » vprocessor

by which criteria @alexpott?

maybe by core folders? (lib, includes, modules)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@vprocessor no please don't do it by lib includes modules. Each sniff defines different error types - we can split it up by error type - See latest patch on #2707641: Ensure core compliance to Drupal.Commenting.FunctionComment.ParamCommentIndentation (part 2) for an example of how to dow this.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Issue summary: View changes
pfrenssen’s picture

pfrenssen
Location Sofia
commented
Issue summary: View changes
pfrenssen’s picture

pfrenssen
Location Sofia
commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

So here goes... created #2715965: Fix 'Drupal.Commenting.FunctionComment.IncorrectParamVarName' coding standard

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Fix 'Drupal.Commenting.FunctionComment' coding standard » [plan] Fix 'Drupal.Commenting.FunctionComment' coding standard
Category: Task » Plan

Let's make this a plan issue to coordinate each issue to this. Given we're going to be removing a line from phpcs.xml.dist each time should have two issues at a time - one from either end - should prevent patch conflicts.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Created #2716009: Fix 'Drupal.Commenting.FunctionComment.WrongStyle' coding standard

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

Updated the issue description with tentative grouping of issues and links to existing issues being worked on for the sub-sniffs. Hopefully this will help in preventing duplication of efforts around this.

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

Added reference to issue to fix @see references related sub-sniffs.

anoopjohn’s picture

anoopjohn commented
Assigned: vprocessor » Unassigned
anoopjohn’s picture

anoopjohn commented

Added fixes for a few sub-sniffs at

#2708185: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to spacing and styling
Drupal.Commenting.FunctionComment.SpacingAfterParamType
Drupal.Commenting.FunctionComment.SpacingAfter
Drupal.Commenting.FunctionComment.ParamCommentNewLine
Drupal.Commenting.FunctionComment.WrongStyle

#2716661: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to @see references
Drupal.Commenting.FunctionComment.SeeAdditionalText
Drupal.Commenting.FunctionComment.SeePunctuation

Edit: Please note that the patch for the second issue is built on top of head + first. So they have to be applied in that order.

anoopjohn’s picture

anoopjohn commented

Added fixes for a couple of sub-sniffs

#2708179: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to @throws

Drupal.Commenting.FunctionComment.ThrowsComment
Drupal.Commenting.FunctionComment.ThrowsNoFullStop
Drupal.Commenting.FunctionComment.ThrowsNotCapital

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

New issue #2721283: Fix Drupal.Commenting.FunctionComment.$InReturnType

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes
anoopjohn’s picture

anoopjohn commented

New issue with patch submitted - #2721309: Fix Drupal.Commenting.FunctionComment.ParamCommentFullStop

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes
anoopjohn’s picture

anoopjohn commented
Issue summary: View changes
anoopjohn’s picture

anoopjohn commented

New issue with patch submitted #2721901: Fix Drupal.Commenting.FunctionComment.MissingParamName

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes
anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

New issue with patch submitted - #2721909: Fix Drupal.Commenting.FunctionComment.ReturnCommentIndentation

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

New issue with patch submitted - #2722609: Fix Drupal.Commenting.FunctionComment.ParamNameNoMatch

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

New issue with patch submitted - #2722621: Fix Drupal.Commenting.FunctionComment.InvalidReturn and Drupal.Commenting.FunctionComment.VoidReturn

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

New issue with patch submitted - #2722647: [Meta] Fix Drupal.Commenting.FunctionComment.InvalidNoReturn

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

New issue with patch submitted - #2722699: Fix Drupal.Commenting.FunctionComment.InvalidReturnNotVoid

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes

New issue with patch submitted - #2723621: Fix Drupal.Commenting.FunctionComment.IncorrectTypeHint and Drupal.Commenting.FunctionComment.InvalidTypeHint

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs work » Active

Plans are active.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mfernea’s picture

mfernea commented

We are missing Drupal.Commenting.FunctionComment.ParamCommentIndentation and Drupal.Commenting.FunctionComment.ParamMissingDefinition from this plan.

mfernea’s picture

mfernea commented
Issue summary: View changes

I adjusted the issue reference for Drupal.Commenting.FunctionComment.ParamCommentNotCapital.

mfernea’s picture

mfernea commented
Issue summary: View changes

I added Drupal.Commenting.FunctionComment.ParamCommentIndentation and Drupal.Commenting.FunctionComment.ParamMissingDefinition to the plan.

mfernea’s picture

mfernea commented
Issue summary: View changes

I added Drupal.Commenting.FunctionComment.ParamTypeSpaces to the plan.

mfernea’s picture

mfernea commented
Issue summary: View changes

I added Drupal.Commenting.FunctionComment.ReturnTypeSpaces to the plan.

mfernea’s picture

mfernea commented
Issue summary: View changes

I added the issue number for Drupal.Commenting.FunctionComment.ParamCommentNewLine.

mfernea’s picture

mfernea commented
Issue summary: View changes

Drupal.Commenting.FunctionComment.ReturnCommentIndentation is still an issue so I added #2902723: Fix 'Drupal.Commenting.FunctionComment.ReturnCommentIndentation' coding standard for that sniff.

martin107’s picture

martin107 commented
Issue summary: View changes

I am making a trivial change to the issue summary to trigger an update of the link. So I can see at a glance what next to review.

mfernea’s picture

mfernea commented
Issue summary: View changes

I updated phpcs usage instructions and violations count for sniffs.

mfernea’s picture

mfernea commented
Issue summary: View changes

I added #2903908: Fix 'Drupal.Commenting.FunctionComment.ParamCommentIndentation' coding standard for Drupal.Commenting.FunctionComment.ParamCommentIndentation.

mfernea’s picture

mfernea commented
Issue summary: View changes

I added #2903911: Fix 'Drupal.Commenting.FunctionComment.ParamMissingDefinition' coding standard.

mfernea’s picture

mfernea commented
Issue summary: View changes

I updated the summary to add more formatting for readability.

martin107’s picture

martin107 commented
Issue summary: View changes

another trivial change to the issue summary to force a color update of the issues.

joachim’s picture

joachim commented

Step 3: Prepare the phpcs.xml file

To speed up the testing you should make a copy of the file phpcs.xml.dist (in the core/ folder) and save it as phpcs.xml. This is the configuration file for PHP CodeSniffer.

We only want this phpcs.xml file to specify the sniff we're interested in. So we need to remove all the rule items, and add only our own sniff's rule. Rule items look like this:

Remove all of them, and add only the sniff from this issue title. This will make sure that our tests run quickly, and are not going to contain any output from unrelated sniffs.

Why are you telling people to go to all the trouble of writing an XML file for this?

The PHPCS command line has an option --sniffs to do just the specified sniffs:

A comma separated list of sniff codes to include or exclude from checking
(all sniffs must be part of the specified standard)

-- see https://github.com/squizlabs/PHP_CodeSniffer/wiki/Usage

mfernea’s picture

mfernea commented

Indeed, that could be another option.
These instructions were used in all Fix CS like issues.
Sometimes it's easier to create the file especially when the sniffs need configuration, or you want to target x out of y errors for a particular sniff. In these cases it's less error prone to create a new file.

ivan berezhnov’s picture

ivan berezhnov commented
Issue tags: +CSKyiv18

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Issue tags: +Nwdug_may18

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

hardik_patel_12’s picture

hardik_patel_12 commented

I added #3103805 : @param tags don't use fully qualified class/interface names

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes

Lost the '#' character.

spokje’s picture

spokje
Primary language Dutch
commented
Issue summary: View changes
spokje’s picture

spokje
Primary language Dutch
commented
Issue summary: View changes

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Issue summary: View changes

Add #2941148: Fix Drupal.Commenting.FunctionComment.MissingReturnType to the IS.

quietone’s picture

quietone commented
Issue summary: View changes

Add #3212066: [meta] Fix Drupal.Commenting.FunctionComment.Missing

quietone’s picture

quietone commented
Issue summary: View changes

Removed fixed issues from the IS

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Title: [plan] Fix 'Drupal.Commenting.FunctionComment' coding standard » [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue tags: +Needs issue summary update

Thanks to everyone who has helped out on child issues, this has been slowly getting better over time. Would be great if someone can remove the fixed sniffs and update the counts for the remaining sniffs in the issue summary, and then we can figure out what to tackle next.

quietone’s picture

quietone commented
Issue summary: View changes
Issue tags: -Needs issue summary update

Updated counts and removed completed issues from the IS.

quietone’s picture

quietone commented
Issue summary: View changes

Add the number of files to the counts because this will indicate the need to split the work into child issues.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Issue summary: View changes

Removing items from the issue summary which have been fixed.

#2716661: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to @see references
Drupal.Commenting.FunctionComment.SeeAdditionalText
Drupal.Commenting.FunctionComment.SeePunctuation

Fixed in #2708185: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to spacing and styling
Drupal.Commenting.FunctionComment.SpacingAfterParamType
Drupal.Commenting.FunctionComment.SpacingAfter
Drupal.Commenting.FunctionComment.WrongStyle

#2708179: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to @throws
Drupal.Commenting.FunctionComment.ThrowsComment
Drupal.Commenting.FunctionComment.ThrowsNoFullStop
Drupal.Commenting.FunctionComment.ThrowsNotCapital

quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
Anonymous’s picture

Anonymous (not verified) commented

akulsaxena made their first commit to this issue’s fork.

quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes

One child completed.

quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Parent issue: #2571965: [meta] Fix PHP coding standards in core, stage 1 » #3553320: [meta] Fix PHP coding standards in core, stage 2

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.