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
- 941/387 - Drupal.Commenting.FunctionComment.Missing - 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. - #3212066: Fix Drupal.Commenting.FunctionComment.Missing
- 675/388 - Drupal.Commenting.FunctionComment.MissingReturnComment
- 762/236 - Drupal.Commenting.FunctionComment.MissingParamType - #3207949: Fix Drupal.Commenting.FunctionComment.MissingParamType
Other Issues with Return Comment
- 33/20 - Drupal.Commenting.FunctionComment.InvalidNoReturn - #2722647: [Meta] Fix Drupal.Commenting.FunctionComment.InvalidNoReturn
Issues with Type Hint
- 926/455 - Drupal.Commenting.FunctionComment.TypeHintMissing - #3107000: Fix Drupal.Commenting.FunctionComment.TypeHintMissing
Other minor issues with Param Comment
- 92/81 - Drupal.Commenting.FunctionComment.ParamCommentFullStop - #2721309: Fix Drupal.Commenting.FunctionComment.ParamCommentFullStop
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
Comment | File | Size | Author |
---|
Comments
Comment #2
attiks CreditAttribution: attiks commentedComment #3
DuaelFrAs 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.
Comment #4
tatarbjUnfortunately 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.
Comment #5
pfrenssenComment #7
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedI 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
Comment #8
vprocessor CreditAttribution: vprocessor at Skilld commentedComment #9
vprocessor CreditAttribution: vprocessor at Skilld commentedReroll & refix with phpcbf is ready
Comment #10
vprocessor CreditAttribution: vprocessor at Skilld commentedSorry, forgot about config - fixed
Comment #11
vprocessor CreditAttribution: vprocessor at Skilld commentedComment #12
vprocessor CreditAttribution: vprocessor at Skilld commentedComment #13
alexpottThis needs to be split up into the sub sniffs there no way to review that patch.
Comment #14
vprocessor CreditAttribution: vprocessor at Skilld commentedby which criteria @alexpott?
maybe by core folders? (lib, includes, modules)
Comment #15
alexpott@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.
Comment #16
pfrenssenComment #17
pfrenssenComment #18
pfrenssenComment #19
alexpottSo here goes... created #2715965: Fix 'Drupal.Commenting.FunctionComment.IncorrectParamVarName' coding standard
Comment #20
alexpottLet'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.
Comment #21
alexpottCreated #2716009: Fix 'Drupal.Commenting.FunctionComment.WrongStyle' coding standard
Comment #22
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedUpdated 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.
Comment #23
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedAdded reference to issue to fix @see references related sub-sniffs.
Comment #24
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedComment #25
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedAdded 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.
Comment #26
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedAdded 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
Comment #27
alexpottNew issue #2721283: Fix Drupal.Commenting.FunctionComment.$InReturnType
Comment #28
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedComment #29
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted - #2721309: Fix Drupal.Commenting.FunctionComment.ParamCommentFullStop
Comment #30
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedComment #31
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedComment #32
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted #2721901: Fix Drupal.Commenting.FunctionComment.MissingParamName
Comment #33
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedComment #34
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted - #2721909: Fix Drupal.Commenting.FunctionComment.ReturnCommentIndentation
Comment #35
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted - #2722609: Fix Drupal.Commenting.FunctionComment.ParamNameNoMatch
Comment #36
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted - #2722621: Fix Drupal.Commenting.FunctionComment.InvalidReturn and Drupal.Commenting.FunctionComment.VoidReturn
Comment #37
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted - #2722647: [Meta] Fix Drupal.Commenting.FunctionComment.InvalidNoReturn
Comment #38
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted - #2722699: Fix Drupal.Commenting.FunctionComment.InvalidReturnNotVoid
Comment #39
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedNew issue with patch submitted - #2723621: Fix Drupal.Commenting.FunctionComment.IncorrectTypeHint and Drupal.Commenting.FunctionComment.InvalidTypeHint
Comment #40
Mile23Plans are active.
Comment #44
mfernea CreditAttribution: mfernea at AmeXio commentedWe are missing Drupal.Commenting.FunctionComment.ParamCommentIndentation and Drupal.Commenting.FunctionComment.ParamMissingDefinition from this plan.
Comment #45
mfernea CreditAttribution: mfernea at AmeXio commentedI adjusted the issue reference for Drupal.Commenting.FunctionComment.ParamCommentNotCapital.
Comment #46
mfernea CreditAttribution: mfernea at AmeXio commentedI added Drupal.Commenting.FunctionComment.ParamCommentIndentation and Drupal.Commenting.FunctionComment.ParamMissingDefinition to the plan.
Comment #47
mfernea CreditAttribution: mfernea at AmeXio commentedI added Drupal.Commenting.FunctionComment.ParamTypeSpaces to the plan.
Comment #48
mfernea CreditAttribution: mfernea at AmeXio commentedI added Drupal.Commenting.FunctionComment.ReturnTypeSpaces to the plan.
Comment #49
mfernea CreditAttribution: mfernea at AmeXio commentedI added the issue number for Drupal.Commenting.FunctionComment.ParamCommentNewLine.
Comment #50
mfernea CreditAttribution: mfernea at AmeXio commentedDrupal.Commenting.FunctionComment.ReturnCommentIndentation is still an issue so I added #2902723: Fix 'Drupal.Commenting.FunctionComment.ReturnCommentIndentation' coding standard for that sniff.
Comment #51
martin107 CreditAttribution: martin107 as a volunteer commentedI 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.
Comment #52
mfernea CreditAttribution: mfernea at AmeXio commentedI updated phpcs usage instructions and violations count for sniffs.
Comment #53
mfernea CreditAttribution: mfernea at AmeXio commentedI added #2903908: Fix 'Drupal.Commenting.FunctionComment.ParamCommentIndentation' coding standard for Drupal.Commenting.FunctionComment.ParamCommentIndentation.
Comment #54
mfernea CreditAttribution: mfernea at AmeXio commentedI added #2903911: Fix 'Drupal.Commenting.FunctionComment.ParamMissingDefinition' coding standard.
Comment #55
mfernea CreditAttribution: mfernea at AmeXio commentedI updated the summary to add more formatting for readability.
Comment #56
martin107 CreditAttribution: martin107 as a volunteer commentedanother trivial change to the issue summary to force a color update of the issues.
Comment #57
joachim CreditAttribution: joachim as a volunteer commentedWhy 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:
-- see https://github.com/squizlabs/PHP_CodeSniffer/wiki/Usage
Comment #58
mfernea CreditAttribution: mfernea at AmeXio commentedIndeed, 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.
Comment #59
Ivan Berezhnov CreditAttribution: Ivan Berezhnov as a volunteer and at Drupal Ukraine Community for Levi9 commentedComment #61
rakesh.gectcrComment #65
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedI added #3103805 : @param tags don't use fully qualified class/interface names
Comment #68
quietone CreditAttribution: quietone as a volunteer commentedComment #69
quietone CreditAttribution: quietone as a volunteer commentedComment #70
quietone CreditAttribution: quietone as a volunteer commentedComment #71
quietone CreditAttribution: quietone as a volunteer commentedLost the '#' character.
Comment #72
SpokjeComment #73
SpokjeComment #75
quietone CreditAttribution: quietone as a volunteer commentedAdd #2941148: Fix Drupal.Commenting.FunctionComment.MissingReturnType to the IS.
Comment #76
quietone CreditAttribution: quietone as a volunteer commentedAdd #3212066: Fix Drupal.Commenting.FunctionComment.Missing
Comment #77
quietone CreditAttribution: quietone as a volunteer commentedRemoved fixed issues from the IS
Comment #79
quietone CreditAttribution: quietone at PreviousNext commentedComment #81
longwaveThanks 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.
Comment #82
quietone CreditAttribution: quietone at PreviousNext commentedUpdated counts and removed completed issues from the IS.
Comment #83
quietone CreditAttribution: quietone at PreviousNext commentedAdd the number of files to the counts because this will indicate the need to split the work into child issues.