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

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 to the whitelist

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". There are dozens of such coding standards, and to make the work easier we have started by only whitelisting the sniffs that pass. For the moment all coding standards that are not yet fixed are simply skipped during the test.

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
$ ./vendor/bin/phpcs --config-set installed_paths ../../drupal/coder/coder_sniffer

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:

$ cd core/
$ ../vendor/bin/phpcs -ps

This takes a couple of minutes. The -p flag shows the progress, so you have a bunch of nice dots to look at while it is running. The -s flag shows the sniffs when displaying results.

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:

$ ../vendor/bin/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.

List of sub-sniffs related to Drupal.Commenting.FunctionComment

The following is the list of sub-sniffs related to Drupal.Commenting.FunctionComment. The number prefix is the number of failures/number of files for the sub-sniff in a run of coder on 10.0.x branch.

The grouping is a proposed way of handling all the issues. Please remember to update specific issue numbers in the list below if any other sub-sniffs are taken up to avoid duplication of efforts.

Missing

Other Issues with Return Comment

Issues with Type Hint

Other minor issues with Param Comment

Issues with @see

  • Drupal.Commenting.FunctionComment.SeeAdditionalText
  • Drupal.Commenting.FunctionComment.SeePunctuation

Spacing / Styling issues

  • Drupal.Commenting.FunctionComment.SpacingAfterParamType
  • Drupal.Commenting.FunctionComment.SpacingAfter
  • Drupal.Commenting.FunctionComment.WrongStyle

Issues with @throws

  • Drupal.Commenting.FunctionComment.ThrowsComment
  • Drupal.Commenting.FunctionComment.ThrowsNoFullStop
  • Drupal.Commenting.FunctionComment.ThrowsNotCapital
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

attiks created an issue. See original summary.

attiks’s picture

DuaelFr’s picture

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

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

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

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

Assigned: Unassigned » vprocessor
vprocessor’s picture

Status: Needs work » Needs review
FileSize
719.15 KB

Reroll & refix with phpcbf is ready

vprocessor’s picture

vprocessor’s picture

vprocessor’s picture

Assigned: vprocessor » Unassigned
alexpott’s picture

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

Assigned: Unassigned » vprocessor

by which criteria @alexpott?

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

alexpott’s picture

@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

Issue summary: View changes
pfrenssen’s picture

Issue summary: View changes
pfrenssen’s picture

Issue summary: View changes
alexpott’s picture

alexpott’s picture

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

anoopjohn’s picture

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

Issue summary: View changes

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

anoopjohn’s picture

Assigned: vprocessor » Unassigned
anoopjohn’s picture

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

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

anoopjohn’s picture

Issue summary: View changes
anoopjohn’s picture

anoopjohn’s picture

Issue summary: View changes
anoopjohn’s picture

Issue summary: View changes
anoopjohn’s picture

anoopjohn’s picture

Issue summary: View changes
anoopjohn’s picture

anoopjohn’s picture

Issue summary: View changes
anoopjohn’s picture

anoopjohn’s picture

anoopjohn’s picture

anoopjohn’s picture

Mile23’s picture

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

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

mfernea’s picture

Issue summary: View changes

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

mfernea’s picture

Issue summary: View changes

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

mfernea’s picture

Issue summary: View changes

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

mfernea’s picture

Issue summary: View changes

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

mfernea’s picture

Issue summary: View changes

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

mfernea’s picture

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

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

Issue summary: View changes

I updated phpcs usage instructions and violations count for sniffs.

mfernea’s picture

Issue summary: View changes

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

mfernea’s picture

mfernea’s picture

Issue summary: View changes

I updated the summary to add more formatting for readability.

martin107’s picture

Issue summary: View changes

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

joachim’s picture

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

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

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

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

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

Issue summary: View changes
quietone’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes

Lost the '#' character.

Spokje’s picture

Issue summary: View changes
Spokje’s picture

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’s picture

quietone’s picture

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

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

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

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

Updated counts and removed completed issues from the IS.

quietone’s picture

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.