[meta] Fix PHP coding standards in core, stage 1

Please please consider automating this with github.com/grom358/pharborist - the library that powers drupal_module_upgrader

I'm starting to check which groups we can use for creating child tickets on this topic.
As we see in the Coder there are already groups under the Sniffs folder, but for checking maybe these are a bit too big ones.
The current structure is the following:

• Array
• CSS
• Classes
• Commenting
• ControlStructures
• Files
• Formatting
• Functions
• InfoFiles
• NamingConventions
• Semantics
• Strings
• WhiteSpaces

Obviously if there is a ticket for fixing all of the 'Strings' it can be very easy to review the patch, but if you see the topic of 'Commenting' where 7 classes are, we should split it for sub-sub topics to make the review easier as we discusses with @pfrenssen and @xjm.
I'm diving to the folders to take some proposals about the groups.

First of all, there are the types of coding standards:
Errors which need to be fix by manual.
Errors which can be fixed by PHP Code Beautifier and Fixer.
Warnings which need to be fix by manual.
Warnings which can be fixed by PHPCBF.
In this case based on what i mentioned above, we should focus on the manually fixable coding standards which cannot be fixed by PHPCBF, i mean those issues what can be fixed automatically are easy to check by a review, so those can be in one task, the other ones are in separated tickets.
Here is the sniffs regarding the arrays and my proposal to split those to 3 tickets.

1. If the line declaring an array spans longer than 80 characters, each element should be broken into its own line.
2. Array closing indentation error, expected X spaces but found Y (Fixable Error)
   Array indentation error, expected X spaces but found Y (Fixable Error)
3. A comma should follow the last multiline array item. (Fixable Warning)

All of the new tasks should contain at least one example regarding the fixable coding standards (obviously if the ticket contains more than 1 coding standard it has to have more examples) then all of the information about fixing of those should be there, like does it fixable by PHPCBF or not, is it an error or warning, stuff like this.

Array Sniff
Errors:

• If the line declaring an array spans longer than 80 characters, each element should be broken into its own line.
• Array closing indentation error, expected X spaces but found Y (Fixable Error)
• Array indentation error, expected X spaces but found Y (Fixable Error)

Warnings:

• A comma should follow the last multiline array item. (Fixable Warning)

CSS
Errors:

• Multiple selectors should each be on a single line (Fixable Error)
• Selectors must be on a single line (Fixable Error)
• Blank lines are not allowed between class names (Fixable Error)
• CSS colours must be defined in lowercase; expected X but found Y (Fixable Error)

Classes
Errors:

• Calling class constructors must always include parentheses (Fixable Error)
• Closing X brace must be on a line by itself
• Opening brace must be the last content on the line (Fixable Error)
• Opening brace should be on the same line as the declaration (Fixable Error)
• Expected 1 space before opening brace; found X (Fixable Error)
• The closing brace for the X must have an empty line before it (Fixable Error)

Warnings:

• Possible parse error: X missing opening or closing brace
• Interface names should always have the suffix “Interface”

Commenting
Errors:

• Missing X doc comment (Fixable Error, but it will generate another coding standard bug which needs to be fixed by manual!)
• You must use "/**" style comments for a X comment (Fixable error)
• There must be exactly one newline after the X comment (Fixable error)
• Doc comment is empty
• The open comment tag must be the only content on the line (Fixable error)
• Wrong function doc comment end; expected "*/", found “X”
• Additional blank lines found at end of doc comment (Fixable error)
• Missing short description in doc comment
• Doc comment short description must be on the first line (Fixable error)
• Function comment short description must start with exactly one space (Fixable error)
• Doc comment short description must start with a capital letter (Fixable error)
• Doc comment short description must end with a full stop (Fixable error)
• Doc comment short description must be on a single line, further text should be a separate paragraph
• There must be exactly one blank line between descriptions in a doc comment (Fixable error)
• Doc comment long description must start with a capital letter (Fixable error)
• Doc comment long description must end with a full stop (Fixable error)
• There must be exactly one blank line before the tags in a doc comment (Fixable error)
• Parameter tags must be grouped together in a doc comment
• Separate the X and Y sections by a blank line. (Fixable error)
• There must be a single blank line after a tag group (Fixable error)
• Tag value indented incorrectly; expected 1 space but found X (Fixable error)
• Parameter tags must be defined first in a doc comment
• Tags must be grouped together in a doc comment
• Doc comment star missing (Fixable error)
• You must use "/**" style comments for a file comment (Fixable error)
• Missing file doc comment (Fixable error, but it will generate another coding standard bug which needs to be fixed by manual!)
• The second line in the file doc comment must be “@file" (Fixable error, but it will generate another coding standard bug which needs to be fixed by manual!)
• There must be exactly one blank line after the file comment
• Missing function doc comment (Fixable error, but it will generate another coding standard bug which needs to be fixed by manual!)
• You must use "/**" style comments for a function comment
• There must be no blank lines after the function comment
• Content missing for @see tag in function comment
• Only 1 @return tag is allowed in a function comment
• Return type missing for @return tag in function comment
• Function return type "X" is invalid, Expected "X" but found "X" for function return type.

Warnings:
-

ControlStructures
Errors:
Warnings:

Files
Errors:
Warnings:

Formatting
Errors:
Warnings:

Functions
Errors:
Warnings:

InfoFiles
Errors:
Warnings:

NamingConventions
Errors:
Warnings:

Semantics
Errors:
Warnings:

Strings
Errors:
Warnings:

WhiteSpaces
Errors:
Warnings:

We can say:

phpcs --standard=/path/to/coder/coder_sniffer/Drupal -e

This gives us a list of all the available sniffs. Let's check for Drupal.Files.EndFileNewline which tells us whether the file has only one newline at the end. This is easy to apply without requiring a lot of rerolls.

And while we're at it, have phpcs generate a patch for us. (Really it's a diff, so we have to apply it and then have git make a patch, but still...)

$ phpcs --standard=/path/to/coder/coder_sniffer/Drupal --sniffs=Drupal.Files.EndFileNewline --ignore=vendor,assets/vendor --report-diff=file_end.diff -p core/
$ patch -p0 -ui file_end.diff
$ git diff > patch_name.patch

This gets us pretty far, but it does weird stuff like this, which I believe is an artifact of how phpcs makes its diff file:

diff --git a/core/misc/dialog.theme.css b/core/misc/dialog.theme.css
index 837e49f..bc3efe3 100644
--- a/core/misc/dialog.theme.css
+++ b/core/misc/dialog.theme.css
@@ -6,7 +6,7 @@
   position: absolute;
   z-index: 1260;
   overflow: visible;
-  color: #000;
+  color: 1000;
   background: #fff;
   border: solid 1px #ccc;
   padding: 0;
@@ -68,4 +68,3 @@
 .ui-dialog .ajax-progress-throbber .message {
   display: none;
 }
-

The workaround is to review the patch. :-) We might also exclude css from review, though we shouldn't.

So having fixed those things, here's a patch to fix violations of the one-newline-at-the-end-of-the-file rule for the core/ directory. Presented here as a proof of concept for mulling over, or, you know, committing. :-)

Will set back to 'Active' once the test is over.

#9 Patch looks clean, but why not use git to create the patch?

@attiks: The point here is that I used a tool to automatically generate most of the fixes. And I did use git to generate the patch as a last step.

#11 You're right, I was assuming phpcbf had an option to limit the sniffs as well

#11 I tried doing a sniff, see #2572307: Fix 'Generic.PHP.UpperCaseConstant' coding standard

@Mile23 i think we should focus only the php files instead of checking css and js as well. i'll create those tickets tomorrow with mentioning this as a parent one, but keep this issue to discuss about the topic instead of putting patches here.
@attiks you can use git instead of patch command to generate the patch file, i don't get why we need diff, i mean when a coding standard, a sniff can be fixed by phpcbf it's good, it can be very easy to review it, then no need to wait for years to see it in the CI test bot and obviously in the core :)
The only problem with the sniffs sometimes those has tons of coding standards (like the comments and functions packages) which are not that easy to review it in one step. The origin idea regarding check the modules and bigger part of the core one-by-one was cancelled because for instance there is the block module which still have at least 15 different coding standards, if you've cleaned it up then made the patch file a simple person who is not a big fan of this topic (like we are) wants to review it to help us he will scream a lot just about the different topic, the first one is one case, the second is another, the third is one another, it makes him crazy :) So that's why we should split somehow those sniffs to smaller, more direct, more easier to check parts.

There are better tools for CSS and js, so +1 to focus on php only

I created a github repo at https://github.com/attiks/Fix-coding-standards-in-core, PR's welcome.

Best to keep the discussion in this issue

I'm running into coder issues from time to time, so we probably will need to fix some things upstream.

Some patches are huge, we need to find a way to split these in smaller chunks, but before we can do it, we need an automatic way to run this. #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard is 500K, we might be able to split it for each module, but this will give us 50+ issues for 1 sniff.

@attiks there is a chance to add an xml file to drupal core in the future what will contain a black list with all of the sniffs in the beginning, then when one issue is solved and committed to the core we just remove that sniff from the list and ci test bot will know it can check already that one, and so on - but i see the creativity of your github repo, i think it can be a good start for using phpcbf but it won't work with those issue which are not solvable via this way but need manual fixes. That's why i started to collect all of the "topics" based on the sniffs, but as you saw there are couple of it which not that easy to review (look at the reaction under #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard and unfortunately there are some fake fixes what shouldn't be part of the core like semicolon then a dot and stuff like this.

#21 I saw your comment, I just wanted to find out how much could be automated, I briefly spoke to Klausi and he asked to file issues against https://www.drupal.org/project/issues/coder and cross link them with our issue, so once it gets fixed upstream we can use it.

Sure, very good idea to create a link between this plan and the Coder! Maybe we should find someone from the Documentation team as well because i'm not sure all of the sniffs what Coder has documented in the Coding standards area, it would be great when someone fix a sniff (or a part of a sniff) in the same step the coding standard docs will be also checked here.

All auto fixes are ran,

- 'Drupal.Array.Array' => 2572613, // @see https://www.drupal.org/node/2572587
- 'Drupal.CSS.ClassDefinitionNameSpacing' => 0, // Skipping css
- 'Drupal.CSS.ColourDefinition' => 0, // Skipping css
- 'Drupal.Classes.ClassCreateInstance' => 2572601,
- 'Drupal.Classes.ClassDeclaration' => 2572619,
- 'Drupal.Classes.InterfaceName' => 2572631,
- 'Drupal.Commenting.ClassComment' => 2572633,
- 'Drupal.Commenting.DocComment' => 2572635,
- 'Drupal.Commenting.DocCommentStar' => 2572641,
- 'Drupal.Commenting.FileComment' => 2572643,
- 'Drupal.Commenting.FunctionComment' => 2572645,
- 'Drupal.Commenting.HookComment' => 2572649, // No fixable errors were found
- 'Drupal.Commenting.InlineComment' => 2572659,
- 'Drupal.ControlStructures.ControlSignature' => 2572693,
- 'Drupal.ControlStructures.ElseIf' => 2572695,
- 'Drupal.ControlStructures.InlineControlStructure' => 2572699,
- 'Drupal.ControlStructures.TemplateControlStructure' => 2572701,
- 'Drupal.Files.EndFileNewline' => 2572707,
- 'Drupal.Files.LineLength' => 2572709,
- 'Drupal.Files.TxtFileLineLength' => 0,
- 'Drupal.Formatting.MultiLineAssignment' => 0, // No fixable errors were found
- 'Drupal.Formatting.SpaceInlineIf' => 0, // No fixable errors were found
- 'Drupal.Formatting.SpaceUnaryOperator' => 2572731,
- 'Drupal.Functions.DiscouragedFunctions' => 0, // No fixable errors were found
- 'Drupal.Functions.FunctionDeclaration' => 0, // No fixable errors were found
- 'Drupal.InfoFiles.ClassFiles' => 0, // No fixable errors were found
- 'Drupal.InfoFiles.Required' => 0, // No fixable errors were found
- 'Drupal.NamingConventions.KeywordLowerCase' => 0, // No fixable errors were found
- 'Drupal.NamingConventions.ValidClassName' => 0, // No fixable errors were found
- 'Drupal.NamingConventions.ValidFunctionName' => 0, // No fixable errors were found
- 'Drupal.NamingConventions.ValidGlobal' => 0, // No fixable errors were found
- 'Drupal.NamingConventions.ValidVariableName' => 0, // No fixable errors were found
- 'Drupal.Semantics.ConstantName' => 0, // No fixable errors were found
- 'Drupal.Semantics.EmptyInstall' => 0, // No fixable errors were found
- 'Drupal.Semantics.FunctionAlias' => 0, // No fixable errors were found
- 'Drupal.Semantics.FunctionT' => 0, // No fixable errors were found
- 'Drupal.Semantics.FunctionWatchdog' => 0, // No fixable errors were found
- 'Drupal.Semantics.InstallHooks' => 0, // No fixable errors were found
- 'Drupal.Semantics.InstallT' => 0, // No fixable errors were found
- 'Drupal.Semantics.LStringTranslatable' => 0, // No fixable errors were found
- 'Drupal.Semantics.PregSecurity' => 0, // No fixable errors were found
- 'Drupal.Semantics.RemoteAddress' => 0, // No fixable errors were found
- 'Drupal.Semantics.TInHookMenu' => 0, // No fixable errors were found
- 'Drupal.Semantics.TInHookSchema' => 0, // No fixable errors were found
- 'Drupal.Strings.ConcatenationSpacing' => 2572777,
- 'Drupal.Strings.UnnecessaryStringConcat' => 0, // No fixable errors were found
- 'Drupal.WhiteSpace.CloseBracketSpacing' => 2572787,
- 'Drupal.WhiteSpace.Comma' => 2572787,
- 'Drupal.WhiteSpace.EmptyLines' => 0,
- 'Drupal.WhiteSpace.ObjectOperatorIndent' => 0,
- 'Drupal.WhiteSpace.ObjectOperatorSpacing' => 0,
- 'Drupal.WhiteSpace.OpenBracketSpacing' => 2572791,
- 'Drupal.WhiteSpace.OperatorSpacing' => 2572793,
- 'Drupal.WhiteSpace.ScopeClosingBrace' => 2572795,
- 'Drupal.WhiteSpace.ScopeIndent' => 2572801,
- 'Generic.PHP.UpperCaseConstant' => 2572307,

We need to add our own phpcs.xml file to the root (or only in core?), more info can be found at https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml

Initially we blacklist all rules one by one and commit it asap, see #2573377: Add our own phpcs.xml file

We probably need to update composer.json as well to pull in code sniffer

{
    "require": {
        "drupal/coder": "~8.2",
        "fabpot/php-cs-fixer": "~1.0",
        "squizlabs/php_codesniffer": "*"
    }
}

@attiks did you used 8.x-2.3 of coder? I'm checking those issues what you created yesterday, but all of them has other issues what your patches don't fix - as you see the released version was updated in June and the latest version of it just couple of days old in the dev version, i always use it to make sure all of our new fixes are inside the process, and as i see under the #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard there is a lot of other issues what your patch didn't fix, btw the patches are also failed :(

So are we fixing coding standards, or are we including a fixer tool in core?

I'd suggest that the turnaround on changes to the testbot will be a relatively long time.

We can make changes in child issues here per the meta and then review them.

postponed on testbot being able to test for the rule that each child will try and fix. #1266444: DrupalCI should return failed for poor code style - php

Im un-postponing this. Having the testbots run this will act as a gatekeeper going forward to prevent new patches from going in with coding standards violations, but having the testbots run phpcs isnt a requirement for fixing existing code standards violations. Everybody who has an IDE should be able to configure their environment for code standards checks, and everybody else can run phpcs from the command line. This will be implemented on the testbots eventually, but it might take a while to get this in, as there is still a lot of work to do to add it.

In the meantime, even new patches can be reviewed by folks locally - i.e. theres really nothing special or more official that the testbots are going to do that cannot be done locally.

That makes sense to me. In that case, let's enable each rule in the XML file as core becomes compliant? Should we create a followup for each fix? See #2572695: Fix 'Drupal.ControlStructures.ElseIf' coding standard.

Also, child issues can be tagged "rc eligible" so committers can find them when they are RTBC. If a rule is risky or disruptive we may choose to postpone it, but we can handle that as part of the normal patch review process.

the summary talks about a blacklist which we would remove rules from one at a time. which in general is the idea. but if coder added some rules, it might get confusingly auto enabled, since it isn't in the blacklist. might be better to have an explicit whitelist, so only rules we have really thought about are tested. (maybe this is what we mean, and the words are just a bit confusing)

Re #35 I don't get why we need a followup to remove the exclude - surely the patch that fixes so we're compliant should enable the rule. Doing that and running phpcs should be part of the proof that the patch is ready. It also makes it possible to combine running phpcs with a commit workflow and ensure we don't introduce regressions.

I think that better to enable the integration to drupalci already to prevent accepting patches that break already fixed at least

@andypost well obviously we want DrupalCI integration but wait for that is the reason why we don't do things. So having a phpcs.xml.dist that reflects what we expect to pass makes sense to me because then I can adapt my commit hooks to scan every changed file.

There are a few changes to automate code review for the different parts:

• #1266444: DrupalCI should return failed for poor code style - php
• #2575497: DrupalCI should report code style issues - CSS
• #2575501: [PP-2] DrupalCI should return failed for poor code style - JS
• #2591827: Add YAML linting to core coding standards checks

Most of them are held up by container changes. Which are in turn held up by #2580007: Add phantomjs2 to the testrunners, which is a child issue of #2469713: Step 2: Create a JavaScriptTestBase using PhantomJs Driver/Binary.

We are attempting to make changes easier to make to the containers with #2609560: Base DCI containers off official containers.

We really just need more people working on it all.

Created #2623680: Fix phpcs.xml.dist to work with the latest rules and not have unnecessary lines to update the phpcs.xml.dist to represent the current state of things

adding information about what is blocking this to the summary.

I ran the 8.2.x branch of core against the 8.x.2.x branch (head) of the Drupal standards and got the following set of errors. Although this issue is against 8.1.x branch I wanted to see if we need to have a comprehensive list of coder issues that we have to fix.

This was run using

phpcs -s --standard=Drupal --ignore=core/assets/vendor/* --ignore=core/modules/system/tests/fixtures/HtaccessTest --extensions=inc,install,module,php,profile,test,theme core

I have attached the list of errors and also a summary of the list of errors below

7860	Drupal.WhiteSpace.CloseBracketSpacing.ClosingWhitespace
7797	Drupal.WhiteSpace.OpenBracketSpacing.OpeningWhitespace
6585	Generic.WhiteSpace.DisallowTabIndent.NonIndentTabsUsed
3045	Drupal.WhiteSpace.ScopeIndent.Incorrect
1542	Drupal.Commenting.InlineComment.InvalidEndChar
1476	Drupal.Commenting.FunctionComment.Missing
1070	Squiz.Scope.MethodScope.Missing
1046	Squiz.WhiteSpace.FunctionSpacing.After
930	Drupal.Commenting.FunctionComment.MissingParamType
926	Drupal.Commenting.DocComment.MissingShort
855	Drupal.Classes.ClassDeclaration.CloseBraceAfterBody
815	Generic.WhiteSpace.DisallowTabIndent.TabsUsed
448	Drupal.Commenting.DocComment.ShortSingleLine
396	Drupal.Commenting.FunctionComment.MissingReturnComment
373	Drupal.WhiteSpace.OperatorSpacing.NoSpaceBefore
362	Drupal.WhiteSpace.OperatorSpacing.NoSpaceAfter
359	Squiz.CSS.ClassDefinitionClosingBraceSpace.ContentBeforeClose
359	Squiz.CSS.ClassDefinitionOpeningBraceSpace.ContentBefore
336	Squiz.ControlStructures.SwitchDeclaration.SpacingAfterBreak
287	Drupal.Commenting.InlineComment.SpacingAfter
267	Drupal.ControlStructures.ControlSignature.NewlineAfterCloseBrace
254	Drupal.Commenting.DocComment.TagGroupSpacing
235	Drupal.Commenting.FunctionComment.MissingReturnType
228	Squiz.Commenting.DocCommentAlignment.SpaceAfterStar
202	Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps
183	Drupal.Array.Array.CommaLastItem
176	Drupal.WhiteSpace.ScopeClosingBrace.Line
176	Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines
174	Squiz.Arrays.ArrayDeclaration.CloseBraceNewLine
169	Drupal.Commenting.PostStatementComment.Found
167	Drupal.Commenting.DocComment.TagsNotGrouped
161	Drupal.Commenting.ClassComment.Missing
149	Squiz.WhiteSpace.FunctionSpacing.Before
145	Drupal.Commenting.FunctionComment.ParamCommentFullStop
142	Drupal.NamingConventions.ValidVariableName.LowerCamelName
139	Squiz.Strings.ConcatenationSpacing.PaddingFound
125	Drupal.ControlStructures.InlineControlStructure.NotAllowed
120	Drupal.WhiteSpace.OperatorSpacing.SpacingAfter
115	Drupal.Commenting.FunctionComment.MissingParamComment
115	Drupal.Commenting.InlineComment.NotCapital
106	Squiz.Commenting.DocCommentAlignment.SpaceBeforeStar
98	Generic.PHP.UpperCaseConstant.Found
96	Drupal.Commenting.InlineComment.SpacingBefore
90	Drupal.Files.EndFileNewline.TooMany
88	Drupal.NamingConventions.ValidFunctionName.NotCamelCaps
87	Drupal.Commenting.DocComment.SpacingBeforeTags
81	Drupal.Commenting.InlineComment.DocBlock
77	Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing
77	Squiz.CSS.SemicolonSpacing.NotAtEnd
69	Drupal.Array.Array.ArrayClosingIndentation
67	Drupal.Commenting.FunctionComment.TypeHintMissing
61	Drupal.Commenting.InlineComment.NoSpaceBefore
61	Squiz.WhiteSpace.SemicolonSpacing.Incorrect
59	Generic.Functions.OpeningFunctionBraceKernighanRitchie.ContentAfterBrace
57	PSR2.Classes.PropertyDeclaration.ScopeMissing
56	Squiz.Arrays.ArrayDeclaration.FirstIndexNoNewline
55	Drupal.WhiteSpace.EmptyLines.EmptyLines
54	Drupal.Semantics.FunctionT.ConcatString
49	Drupal.Commenting.DocComment.ShortNotCapital
45	Squiz.ControlStructures.ForLoopDeclaration.SpacingBeforeFirst
44	Drupal.ControlStructures.ControlSignature.SpaceAfterKeyword
43	Drupal.Semantics.FunctionT.NotLiteralString
40	Drupal.Commenting.FunctionComment.ReturnCommentIndentation
39	Drupal.Commenting.FunctionComment.MissingParamName
38	Drupal.Commenting.FunctionComment.ParamNameNoMatch
37	Drupal.WhiteSpace.ScopeIndent.IncorrectExact
36	Drupal.Strings.UnnecessaryStringConcat.Found
36	Squiz.PHP.NonExecutableCode.Unreachable
31	Drupal.Commenting.FunctionComment.IncorrectTypeHint
31	Drupal.CSS.ClassDefinitionNameSpacing.MultipleSelectors
30	Drupal.NamingConventions.ValidFunctionName.InvalidName
30	Generic.Functions.OpeningFunctionBraceKernighanRitchie.BraceOnNewLine
29	Squiz.Arrays.ArrayDeclaration.CommaAfterLast
27	Squiz.Arrays.ArrayDeclaration.SpaceAfterKeyword
26	Drupal.CSS.ColourDefinition.NotLower
26	PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse
25	Drupal.Commenting.DocComment.SpacingAfter
24	Drupal.Commenting.FunctionComment.SeePunctuation
24	Squiz.Arrays.ArrayDeclaration.SpaceInEmptyArray
24	Squiz.WhiteSpace.SuperfluousWhitespace.StartFile
23	Drupal.ControlStructures.ControlSignature.SpaceAfterCloseParenthesis
23	PSR2.Classes.PropertyDeclaration.Underscore
23	Squiz.CSS.DisallowMultipleStyleDefinitions.Found
22	Drupal.Commenting.FunctionComment.InvalidNoReturn
22	Drupal.ControlStructures.ControlSignature.NewlineAfterOpenBrace
21	Drupal.Classes.ClassDeclaration.SpaceBeforeBrace
20	Drupal.WhiteSpace.ScopeClosingBrace.Indent
18	Drupal.Commenting.FileComment.Missing
18	Drupal.CSS.ClassDefinitionNameSpacing.SeletorSingleLine
17	Drupal.Commenting.DocComment.LongNotCapital
16	Drupal.Commenting.DocComment.ShortStartSpace
15	Generic.Functions.FunctionCallArgumentSpacing.SpaceBeforeComma
15	Generic.Functions.OpeningFunctionBraceKernighanRitchie.SpaceAfterBracket
15	Squiz.Arrays.ArrayDeclaration.NoSpaceAfterDoubleArrow
14	Squiz.Arrays.ArrayDeclaration.SpaceAfterDoubleArrow
13	PEAR.Functions.ValidDefaultValue.NotAtEnd
13	Squiz.CSS.ClassDefinitionOpeningBraceSpace.NoneBefore
12	Drupal.Classes.ClassCreateInstance.ParenthesisMissing
12	Drupal.Files.EndFileNewline.NoneFound
12	Squiz.Arrays.ArrayDeclaration.NoSpaceBeforeDoubleArrow
11	PEAR.Functions.FunctionCallSignature.SpaceBeforeOpenBracket
10	Drupal.Commenting.FunctionComment.ParamCommentNewLine
9	Drupal.Commenting.FunctionComment.InvalidReturnNotVoid
9	PSR2.Namespaces.UseDeclaration.UseAfterNamespace
8	Drupal.Commenting.FunctionComment.IncorrectParamVarName
8	Drupal.Commenting.FunctionComment.$InReturnType
8	Drupal.Commenting.InlineComment.TabBefore
8	Drupal.Semantics.FunctionT.BackslashSingleQuote
8	Squiz.ControlStructures.ForLoopDeclaration.SpacingBeforeSecond
8	Squiz.WhiteSpace.SuperfluousWhitespace.EndFile
7	Drupal.Commenting.FunctionComment.SpacingAfterParamType
7	Drupal.Commenting.FunctionComment.WrongStyle
7	Drupal.WhiteSpace.OperatorSpacing.NoSpaceAfterAmp
7	Drupal.WhiteSpace.OperatorSpacing.NoSpaceBeforeAmp
7	PEAR.Functions.FunctionCallSignature.SpaceAfterCloseBracket
7	Squiz.Arrays.ArrayDeclaration.SpaceBeforeComma
6	Drupal.CSS.ClassDefinitionNameSpacing.BlankLinesFound
6	Squiz.Arrays.ArrayBracketSpacing.SpaceBeforeBracket
6	Squiz.CSS.EmptyStyleDefinition.Found
5	Drupal.Classes.ClassDeclaration.SpaceAfterName
5	Drupal.Commenting.FunctionComment.ThrowsComment
5	Drupal.ControlStructures.ControlSignature.SpaceBeforeSemicolon
5	Drupal.NamingConventions.ValidVariableName.LowerStart
5	Squiz.Arrays.ArrayDeclaration.SpaceBeforeDoubleArrow
5	Squiz.Commenting.DocCommentAlignment.NoSpaceAfterStar
4	Drupal.Commenting.FileComment.SpacingAfterComment
4	Drupal.Commenting.FunctionComment.ParamCommentNotCapital
4	Drupal.Commenting.FunctionComment.ThrowsNoFullStop
4	Drupal.Formatting.SpaceInlineIf.SpaceInlineElse
3	Drupal.Classes.InterfaceName.InterfaceSuffix
3	Drupal.Commenting.DocComment.SpacingBeforeShort
3	Drupal.ControlStructures.ControlSignature.SpaceAfterCloseBrace
3	Drupal.Files.LineLength.TooLong
3	Drupal.Functions.FunctionDeclaration.SpaceBeforeParenthesis
3	Drupal.Semantics.FunctionAlias.FunctionAlias
3	Squiz.ControlStructures.ForEachLoopDeclaration.SpacingBeforeAs
2	Drupal.Classes.ClassDeclaration.BraceOnNewLine
2	Drupal.Commenting.DocComment.SpacingAfterTagGroup
2	Drupal.Commenting.DocComment.SpacingBetween
2	Drupal.Commenting.FunctionComment.InvalidTypeHint
2	Drupal.Commenting.FunctionComment.SeeAdditionalText
2	Drupal.Formatting.MultiLineAssignment
2	Drupal.Formatting.SpaceUnaryOperator.BooleanNot
2	Drupal.NamingConventions.ValidClassName.StartWithCaptial
2	Drupal.WhiteSpace.ScopeClosingBrace.BreakIdent
2	Generic.Formatting.SpaceAfterCast.NoSpace
2	Generic.NamingConventions.ConstructorName.OldStyle
2	Squiz.Arrays.ArrayBracketSpacing.SpaceAfterBracket
2	Squiz.Arrays.ArrayDeclaration.NoKeySpecified
2	Squiz.ControlStructures.ForEachLoopDeclaration.NoSpaceAfterArrow
2	Squiz.ControlStructures.SwitchDeclaration.SpacingAfterDefault
2	Squiz.CSS.ClassDefinitionOpeningBraceSpace.Before
2	Squiz.CSS.EmptyClassDefinition.Found
2	Squiz.CSS.Indentation.BlankLine
2	Squiz.CSS.SemicolonSpacing.SpaceFound
2	Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceAfterDefault
2	Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceBeforeEquals
2	Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBeforeClose
2	Squiz.PHP.NonExecutableCode.ReturnNotRequired
1	Drupal.Classes.ClassDeclaration.SpaceBeforeExtends
1	Drupal.Classes.ClassDeclaration.SpaceBeforeImplements
1	Drupal.Commenting.DocComment.ParamNotFirst
1	Drupal.Commenting.DocComment.TagValueIndent
1	Drupal.Commenting.FileComment.NamespaceNoFileDoc
1	Drupal.Commenting.FunctionComment.InvalidReturn
1	Drupal.Commenting.FunctionComment.SpacingAfter
1	Drupal.Commenting.FunctionComment.ThrowsNotCapital
1	Drupal.Commenting.FunctionComment.VoidReturn
1	Drupal.Formatting.SpaceUnaryOperator.IncDecLeft
1	Drupal.Functions.FunctionDeclaration.SpaceAfter
1	Drupal.NamingConventions.ValidGlobal.GlobalUnderScore
1	Drupal.Semantics.FunctionT.Concat
1	Drupal.Semantics.RemoteAddress.RemoteAddress
1	Drupal.WhiteSpace.ObjectOperatorSpacing.After
1	Squiz.ControlStructures.ForLoopDeclaration.NoSpaceAfterFirst
1	Squiz.ControlStructures.ForLoopDeclaration.NoSpaceAfterSecond
1	Squiz.ControlStructures.SwitchDeclaration.SpaceBeforeColonCase
1	Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceBeforeComma
1	Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingAfterOpenHint
1	Squiz.WhiteSpace.LanguageConstructSpacing.Incorrect
1	Squiz.WhiteSpace.LanguageConstructSpacing.IncorrectSingle
1	Squiz.WhiteSpace.SuperfluousWhitespace.EndLine

The list of errors alphabetically

Drupal.Array.Array.ArrayClosingIndentation
Drupal.Array.Array.CommaLastItem
Drupal.Classes.ClassCreateInstance.ParenthesisMissing
Drupal.Classes.ClassDeclaration.BraceOnNewLine
Drupal.Classes.ClassDeclaration.CloseBraceAfterBody
Drupal.Classes.ClassDeclaration.SpaceAfterName
Drupal.Classes.ClassDeclaration.SpaceBeforeBrace
Drupal.Classes.ClassDeclaration.SpaceBeforeExtends
Drupal.Classes.ClassDeclaration.SpaceBeforeImplements
Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing
Drupal.Classes.InterfaceName.InterfaceSuffix
Drupal.Commenting.ClassComment.Missing
Drupal.Commenting.DocComment.LongNotCapital
Drupal.Commenting.DocComment.MissingShort
Drupal.Commenting.DocComment.ParamNotFirst
Drupal.Commenting.DocComment.ShortNotCapital
Drupal.Commenting.DocComment.ShortSingleLine
Drupal.Commenting.DocComment.ShortStartSpace
Drupal.Commenting.DocComment.SpacingAfter
Drupal.Commenting.DocComment.SpacingAfterTagGroup
Drupal.Commenting.DocComment.SpacingBeforeShort
Drupal.Commenting.DocComment.SpacingBeforeTags
Drupal.Commenting.DocComment.SpacingBetween
Drupal.Commenting.DocComment.TagGroupSpacing
Drupal.Commenting.DocComment.TagsNotGrouped
Drupal.Commenting.DocComment.TagValueIndent
Drupal.Commenting.FileComment.Missing
Drupal.Commenting.FileComment.NamespaceNoFileDoc
Drupal.Commenting.FileComment.SpacingAfterComment
Drupal.Commenting.FunctionComment.$InReturnType
Drupal.Commenting.FunctionComment.IncorrectParamVarName
Drupal.Commenting.FunctionComment.IncorrectTypeHint
Drupal.Commenting.FunctionComment.InvalidNoReturn
Drupal.Commenting.FunctionComment.InvalidReturn
Drupal.Commenting.FunctionComment.InvalidReturnNotVoid
Drupal.Commenting.FunctionComment.InvalidTypeHint
Drupal.Commenting.FunctionComment.Missing
Drupal.Commenting.FunctionComment.MissingParamComment
Drupal.Commenting.FunctionComment.MissingParamName
Drupal.Commenting.FunctionComment.MissingParamType
Drupal.Commenting.FunctionComment.MissingReturnComment
Drupal.Commenting.FunctionComment.MissingReturnType
Drupal.Commenting.FunctionComment.ParamCommentFullStop
Drupal.Commenting.FunctionComment.ParamCommentNewLine
Drupal.Commenting.FunctionComment.ParamCommentNotCapital
Drupal.Commenting.FunctionComment.ParamNameNoMatch
Drupal.Commenting.FunctionComment.ReturnCommentIndentation
Drupal.Commenting.FunctionComment.SeeAdditionalText
Drupal.Commenting.FunctionComment.SeePunctuation
Drupal.Commenting.FunctionComment.SpacingAfter
Drupal.Commenting.FunctionComment.SpacingAfterParamType
Drupal.Commenting.FunctionComment.ThrowsComment
Drupal.Commenting.FunctionComment.ThrowsNoFullStop
Drupal.Commenting.FunctionComment.ThrowsNotCapital
Drupal.Commenting.FunctionComment.TypeHintMissing
Drupal.Commenting.FunctionComment.VoidReturn
Drupal.Commenting.FunctionComment.WrongStyle
Drupal.Commenting.InlineComment.DocBlock
Drupal.Commenting.InlineComment.InvalidEndChar
Drupal.Commenting.InlineComment.NoSpaceBefore
Drupal.Commenting.InlineComment.NotCapital
Drupal.Commenting.InlineComment.SpacingAfter
Drupal.Commenting.InlineComment.SpacingBefore
Drupal.Commenting.InlineComment.TabBefore
Drupal.Commenting.PostStatementComment.Found
Drupal.ControlStructures.ControlSignature.NewlineAfterCloseBrace
Drupal.ControlStructures.ControlSignature.NewlineAfterOpenBrace
Drupal.ControlStructures.ControlSignature.SpaceAfterCloseBrace
Drupal.ControlStructures.ControlSignature.SpaceAfterCloseParenthesis
Drupal.ControlStructures.ControlSignature.SpaceAfterKeyword
Drupal.ControlStructures.ControlSignature.SpaceBeforeSemicolon
Drupal.ControlStructures.InlineControlStructure.NotAllowed
Drupal.CSS.ClassDefinitionNameSpacing.BlankLinesFound
Drupal.CSS.ClassDefinitionNameSpacing.MultipleSelectors
Drupal.CSS.ClassDefinitionNameSpacing.SeletorSingleLine
Drupal.CSS.ColourDefinition.NotLower
Drupal.Files.EndFileNewline.NoneFound
Drupal.Files.EndFileNewline.TooMany
Drupal.Files.LineLength.TooLong
Drupal.Formatting.MultiLineAssignment
Drupal.Formatting.SpaceInlineIf.SpaceInlineElse
Drupal.Formatting.SpaceUnaryOperator.BooleanNot
Drupal.Formatting.SpaceUnaryOperator.IncDecLeft
Drupal.Functions.FunctionDeclaration.SpaceAfter
Drupal.Functions.FunctionDeclaration.SpaceBeforeParenthesis
Drupal.NamingConventions.ValidClassName.StartWithCaptial
Drupal.NamingConventions.ValidFunctionName.InvalidName
Drupal.NamingConventions.ValidFunctionName.NotCamelCaps
Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps
Drupal.NamingConventions.ValidGlobal.GlobalUnderScore
Drupal.NamingConventions.ValidVariableName.LowerCamelName
Drupal.NamingConventions.ValidVariableName.LowerStart
Drupal.Semantics.FunctionAlias.FunctionAlias
Drupal.Semantics.FunctionT.BackslashSingleQuote
Drupal.Semantics.FunctionT.Concat
Drupal.Semantics.FunctionT.ConcatString
Drupal.Semantics.FunctionT.NotLiteralString
Drupal.Semantics.RemoteAddress.RemoteAddress
Drupal.Strings.UnnecessaryStringConcat.Found
Drupal.WhiteSpace.CloseBracketSpacing.ClosingWhitespace
Drupal.WhiteSpace.EmptyLines.EmptyLines
Drupal.WhiteSpace.ObjectOperatorSpacing.After
Drupal.WhiteSpace.OpenBracketSpacing.OpeningWhitespace
Drupal.WhiteSpace.OperatorSpacing.NoSpaceAfter
Drupal.WhiteSpace.OperatorSpacing.NoSpaceAfterAmp
Drupal.WhiteSpace.OperatorSpacing.NoSpaceBefore
Drupal.WhiteSpace.OperatorSpacing.NoSpaceBeforeAmp
Drupal.WhiteSpace.OperatorSpacing.SpacingAfter
Drupal.WhiteSpace.ScopeClosingBrace.BreakIdent
Drupal.WhiteSpace.ScopeClosingBrace.Indent
Drupal.WhiteSpace.ScopeClosingBrace.Line
Drupal.WhiteSpace.ScopeIndent.Incorrect
Drupal.WhiteSpace.ScopeIndent.IncorrectExact
Generic.Formatting.SpaceAfterCast.NoSpace
Generic.Functions.FunctionCallArgumentSpacing.SpaceBeforeComma
Generic.Functions.OpeningFunctionBraceKernighanRitchie.BraceOnNewLine
Generic.Functions.OpeningFunctionBraceKernighanRitchie.ContentAfterBrace
Generic.Functions.OpeningFunctionBraceKernighanRitchie.SpaceAfterBracket
Generic.NamingConventions.ConstructorName.OldStyle
Generic.PHP.UpperCaseConstant.Found
Generic.WhiteSpace.DisallowTabIndent.NonIndentTabsUsed
Generic.WhiteSpace.DisallowTabIndent.TabsUsed
PEAR.Functions.FunctionCallSignature.SpaceAfterCloseBracket
PEAR.Functions.FunctionCallSignature.SpaceBeforeOpenBracket
PEAR.Functions.ValidDefaultValue.NotAtEnd
PSR2.Classes.PropertyDeclaration.ScopeMissing
PSR2.Classes.PropertyDeclaration.Underscore
PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse
PSR2.Namespaces.UseDeclaration.UseAfterNamespace
Squiz.Arrays.ArrayBracketSpacing.SpaceAfterBracket
Squiz.Arrays.ArrayBracketSpacing.SpaceBeforeBracket
Squiz.Arrays.ArrayDeclaration.CloseBraceNewLine
Squiz.Arrays.ArrayDeclaration.CommaAfterLast
Squiz.Arrays.ArrayDeclaration.FirstIndexNoNewline
Squiz.Arrays.ArrayDeclaration.NoKeySpecified
Squiz.Arrays.ArrayDeclaration.NoSpaceAfterDoubleArrow
Squiz.Arrays.ArrayDeclaration.NoSpaceBeforeDoubleArrow
Squiz.Arrays.ArrayDeclaration.SpaceAfterDoubleArrow
Squiz.Arrays.ArrayDeclaration.SpaceAfterKeyword
Squiz.Arrays.ArrayDeclaration.SpaceBeforeComma
Squiz.Arrays.ArrayDeclaration.SpaceBeforeDoubleArrow
Squiz.Arrays.ArrayDeclaration.SpaceInEmptyArray
Squiz.Commenting.DocCommentAlignment.NoSpaceAfterStar
Squiz.Commenting.DocCommentAlignment.SpaceAfterStar
Squiz.Commenting.DocCommentAlignment.SpaceBeforeStar
Squiz.ControlStructures.ForEachLoopDeclaration.NoSpaceAfterArrow
Squiz.ControlStructures.ForEachLoopDeclaration.SpacingBeforeAs
Squiz.ControlStructures.ForLoopDeclaration.NoSpaceAfterFirst
Squiz.ControlStructures.ForLoopDeclaration.NoSpaceAfterSecond
Squiz.ControlStructures.ForLoopDeclaration.SpacingBeforeFirst
Squiz.ControlStructures.ForLoopDeclaration.SpacingBeforeSecond
Squiz.ControlStructures.SwitchDeclaration.SpaceBeforeColonCase
Squiz.ControlStructures.SwitchDeclaration.SpacingAfterBreak
Squiz.ControlStructures.SwitchDeclaration.SpacingAfterDefault
Squiz.CSS.ClassDefinitionClosingBraceSpace.ContentBeforeClose
Squiz.CSS.ClassDefinitionOpeningBraceSpace.Before
Squiz.CSS.ClassDefinitionOpeningBraceSpace.ContentBefore
Squiz.CSS.ClassDefinitionOpeningBraceSpace.NoneBefore
Squiz.CSS.DisallowMultipleStyleDefinitions.Found
Squiz.CSS.EmptyClassDefinition.Found
Squiz.CSS.EmptyStyleDefinition.Found
Squiz.CSS.Indentation.BlankLine
Squiz.CSS.SemicolonSpacing.NotAtEnd
Squiz.CSS.SemicolonSpacing.SpaceFound
Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceAfterDefault
Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceBeforeComma
Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceBeforeEquals
Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingAfterOpenHint
Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBeforeClose
Squiz.PHP.NonExecutableCode.ReturnNotRequired
Squiz.PHP.NonExecutableCode.Unreachable
Squiz.Scope.MethodScope.Missing
Squiz.Strings.ConcatenationSpacing.PaddingFound
Squiz.WhiteSpace.FunctionSpacing.After
Squiz.WhiteSpace.FunctionSpacing.Before
Squiz.WhiteSpace.LanguageConstructSpacing.Incorrect
Squiz.WhiteSpace.LanguageConstructSpacing.IncorrectSingle
Squiz.WhiteSpace.SemicolonSpacing.Incorrect
Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines
Squiz.WhiteSpace.SuperfluousWhitespace.EndFile
Squiz.WhiteSpace.SuperfluousWhitespace.EndLine
Squiz.WhiteSpace.SuperfluousWhitespace.StartFile

I hope the test was done right. If so, which of these are we shooting to fix?

Is it coding standards time since we're close to 8.1.0 release?

I just looked at #2572791: Fix 'Drupal.WhiteSpace.OpenBracketSpacing' coding standard (at random) and the instructions are wrong. I'll update that issue, but I bet the other ones are wrong, as well.

@anoopjohn you should be able to see links to issues created for each rule in the related issues linked above. Any sniff that does not have an issue we should create one.

I have compiled all the coder rules that were listed as failed into a Google spreadsheet and have also mapped the rules to the sniffs and to the respective issues in the issue queue.

https://docs.google.com/spreadsheets/d/1CtpbUdx8F0rGLuVcOOup--pWAhmQz-fL...

I hope the sheet is useful

Work in child issues, centered on Drupal\Component:

• #2707371: Fix several errors in the 'Drupal.Commenting.DocComment' coding standard
• #2572659: Fix 'Drupal.Commenting.InlineComment.SpacingAfter' coding standard
• #2706753: Fix 'PSR2.Classes.PropertyDeclaration.ScopeMissing/VarUsed' coding standard

@anoopjohn are you sure that the list you generated was against core and using the latest released coder ruleset? Some things listed in there were fixed at the time you commented.

Yet another issue filed and patched... #2710081: Fix 'Drupal.Formatting.SpaceInlineIf' coding standard

Just created #2710201: Fix Drupal.Semantics.FunctionT.WhiteSpace and Drupal.Semantics.FunctionT.Concat - this one fixes real bugs!

I ran the 8.2.x branch of core against the 8.x.2.x branch (head) of the coder (commit a5b5f6b5769393a016da0d106a3b33f932c73331). Should I run coder against their latest release or head? Head has moved forward after my last check as well.

@anoopjohn: Drupal has a phpcs.xml.dist file under the core/ directory.

Copy it, rename it to phpcs.xml, add/remove the sniffs you want to, and then run phpcs from there.

And please: It'd be great if you could show off whatever shell script you're using to get counts of errors by sniff... We could use that to do a lot of easily-reviewable issues.

@Mile23 well the point is to run against the Drupal standard provided by coder... our phpcs.xml.dist is a list of the rules that we comply with.

Right, but if you want to make a meaningful report of what needs to happen for CS, you have to use the same exclusions and file extensions as the phpcs.xml.dist file, and also limit the report to core/.

@Mile23 good point... here's a command to run from the core directory... phpcs -p --extensions=inc,install,module,php,profile,test,theme --ignore=./assets/vendor/* --standard=Drupal ./

I used the command as listed in comment #48, given below.

phpcs -s --standard=Drupal --ignore=core/assets/vendor/* --ignore=core/modules/system/tests/fixtures/HtaccessTest --extensions=inc,install,module,php,profile,test,theme core

I had used the same exclusions and file extensions as in phpcs.xml. Also it is running only on core.

Here is the command i used to get the report

cat core-issues-DrupalStandard.latest.4.txt | grep " (" | cut -f2 -d'(' | cut -f1 -d')' | sort | uniq -c | sort -nr

core-issues-DrupalStandard.latest.4.txt is the file created with the phpcs command listed earlier. There will be some incorrectly identified sub-sniff names which I have to manually edit and remove but this should approximately be alright.

Here's a little script I made I call sniffreport.php.

It reads a phpcs json report and then outputs each error encountered as a line.

You can then pipe it into sort and uniq to get a report about which error to tackle.

<?php

/**
 * sniffreport.php
 *
 * Reads in a json file generated by phpcs and spits out each error as a line.
 *
 * 1) Modify core/phpcs.xml to sniff for the sniffs you're sniffing for.
 *
 * 2) Run phpcs like this:
 *
 * $ ../vendor/bin/phpcs -p --report-json=sniff.json
 *
 * 3) Now you can run sniffreport like this:
 *
 * $ php ./scripts/sniffreport.php | sort | uniq -c | sort
 *
 * This will show you a sort-of histogram of all the error codes in the json
 * file. You can use this to determine which patches will be small if you fix
 * a specific error.
 *
 * 4) ????
 *
 * 5) PROFIT.
 */

$errors = json_decode(file_get_contents(__DIR__ . '/../sniff.json'), TRUE);

$files = $errors['files'];

foreach ($files as $file) {
  foreach ($file['messages'] as $message) {
    echo $message['source'] . "\n";
  }
}

For instance, I configured core/phpcs.xml to sniff for Drupal.Semantics, and this is the report for that sniffspace:

   1 Drupal.Semantics.FunctionT.Concat
   1 Drupal.Semantics.RemoteAddress.RemoteAddress
   3 Drupal.Semantics.ConstantName.ConstantStart
   6 Drupal.Semantics.FunctionT.WhiteSpace
  17 Drupal.Semantics.FunctionT.BackslashSingleQuote
  39 Drupal.Semantics.FunctionAlias.FunctionAlias
  62 Drupal.Semantics.FunctionT.NotLiteralString
  66 Drupal.Semantics.FunctionT.ConcatString

This a) verifies that all the current Drupal.Semantics sniffs are still valid, an b) gives us a handy list of things we could work on.

In this case, I decided to see what those ConstantName.ConstantStart errors were, and here's the issue with its teeny tiny patch: #2714815: Enable 'Drupal.Semantics.ConstantName.ConstantStart' coding standard

Also this one: #2714829: Fix 'Generic.Functions.FunctionCallArgumentSpacing' coding standard

@vaplas this is issue is a plan issue - each coding standard has its own issue. See the related issues at the top.

somehow current 8.2.x has

    148 Drupal.Commenting.InlineComment.NotCapital

EDIT #2719657: Fix 'Drupal.Commenting.InlineComment.NotCapital' coding standard

There are a few patches that are posted under the sub-issues under the FunctionComment fix plan issue - #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard. Would be great if somebody could review those.

#2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core

Changing version to 8.4.x-dev

Using commands from #65 I get

composer require drupal/coder squizlabs/php_codesniffer
Using version ^8.2 for drupal/coder
Using version ^3.0 for squizlabs/php_codesniffer
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

Problem 1
- Installation request for squizlabs/php_codesniffer ^3.0 -> satisfiable by squizlabs/php_codesniffer[3.0.0, 3.0.0RC1, 3.0.0RC2, 3.0.0RC3, 3.0.0RC4, 3.0.0a1, 3.0.1, 3.0.x-dev, 3.x-dev].
- drupal/coder 8.2.12 requires squizlabs/php_codesniffer >=2.8.1 <3.0 -> satisfiable by squizlabs/php_codesniffer[2.9.x-dev].
- drupal/coder 8.2.12 requires squizlabs/php_codesniffer >=2.8.1 <3.0 -> satisfiable by squizlabs/php_codesniffer[2.9.x-dev].
- drupal/coder 8.2.12 requires squizlabs/php_codesniffer >=2.8.1 <3.0 -> satisfiable by squizlabs/php_codesniffer[2.9.x-dev].
- Conclusion: don't install squizlabs/php_codesniffer 2.9.x-dev
- Installation request for drupal/coder 8.2.12 -> satisfiable by drupal/coder[8.2.12].

Installation failed, reverting ./composer.json to its original content.

From my pov you only need to run: "composer require drupal/coder". squizlabs/php_codesniffer is listed as a dependency for drupal/coder as ">=2.8.1 <3.0" so no need to require squizlabs/php_codesniffer directly.

phpcs and coder should already be depdendencies of both 8.3.x and 8.4.x.

Updated IS with instructions for how to perform the sniffs, repeated here:

$ cd /path/to/drupal/root/
$ composer install
$ cd core
$ phpcs -ps

Thanks for the update. I changed the path.

I can now work on #2886719: Follow code style for complex hooks question to extend Drupal_Sniffs_Commenting_HookCommentSniff for it. Thanks.

Hm there's some incorrect info in the summary here; this is not blocked. Also removing the beta eval since that's no longer relevant.

I added comments in the sheet in #52 added by @anoopjohn to update the current status of sniffs. We should try to keep that up to date as much as possible, otherwise it's a bit hard to know what to work on.

To get the latest report I use this command:

cd core
../vendor/bin/phpcs -ps --standard=Drupal ./ --ignore='core/assets/vendor/*' --ignore=core/modules/system/tests/fixtures/HtaccessTest  --ignore='*.css,*info,*.txt,*.md,*.yml' --report=source --report-width=120 --report-file=../phpcs-results.txt

I didn't succeed in running this with --extensions. Drupal CS ruleset.xml lists the targeted extensions, so we have to somehow exclude those not yet in phpcs.xml.dist.

Here is the list of sniffs not passing in the latest 8.5.x and not present in the sheet above:

• Drupal.Array.Array.ArrayIndentation - #2902636: Fix Drupal.Array.Array.ArrayIndentation coding standard
• Drupal.Array.Array.LongLineDeclaration
• Drupal.Classes.PropertyDeclaration.Underscore
• Drupal.Classes.UseLeadingBackslash.SeparatorStart - #2902406: Fix 'Drupal.Classes.UseLeadingBackslash' coding standard
• Drupal.Commenting.DataTypeNamespace.DataTypeNamespace - #2902407: Fix 'Drupal.Commenting.DataTypeNamespace' coding standard
• Drupal.Commenting.DocComment.LongFullStop - #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard
• Drupal.Commenting.DocComment.ParamGroup - #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard
• Drupal.Commenting.DocComment.ShortFullStop - #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard
• Drupal.Commenting.DocCommentAlignment.NoSpaceAfterStar
• Drupal.Commenting.DocCommentAlignment.SpaceAfterStar
• Drupal.Commenting.DocCommentAlignment.SpaceBeforeStar
• Drupal.Commenting.FunctionComment.ParamCommentIndentation - #2700661: Ensure core compliance to Drupal.Commenting.FunctionComment.ParamCommentIndentation (part 1)
• Drupal.Commenting.FunctionComment.ParamMissingDefinition
• Drupal.Commenting.FunctionComment.ParamTypeSpaces - #2803893: Fix 'Drupal.Commenting.FunctionComment.ParamTypeSpaces' coding standard
• Drupal.Commenting.FunctionComment.ReturnTypeSpaces - #2803891: Fix 'Drupal.Commenting.FunctionComment.ReturnTypeSpaces' coding standard
• Drupal.Commenting.HookComment - #2623718: Fix 'Drupal.Commenting.HookComment' coding standard
• Drupal.Commenting.HookComment.HookParamDoc
• Drupal.Commenting.HookComment.HookReturnDoc
• Drupal.Commenting.InlineComment.WrongStyle - #2715485: Fix 'Drupal.Commenting.InlineComment.NoSpaceBefore' coding standard
• Drupal.Commenting.VariableComment.DuplicateVar
• Drupal.Commenting.VariableComment.EmptyVar
• Drupal.Commenting.VariableComment.IncorrectVarType
• Drupal.Commenting.VariableComment.InlineVariableName
• Drupal.Commenting.VariableComment.MissingVar
• Drupal.Commenting.VariableComment.VarOrder
• Drupal.Commenting.VariableComment.WrongStyle
• Drupal.Formatting.MultipleStatementAlignment.Incorrect
• Drupal.Formatting.MultipleStatementAlignment.NotSame
• Drupal.Methods.MethodDeclaration.AbstractAfterVisibility - #2901727: Fix 'Drupal.Methods.MethodDeclaration' coding standard
• Drupal.Methods.MethodDeclaration.FinalAfterVisibility - #2901727: Fix 'Drupal.Methods.MethodDeclaration' coding standard
• Drupal.Methods.MethodDeclaration.StaticBeforeVisibility - #2901727: Fix 'Drupal.Methods.MethodDeclaration' coding standard
• Drupal.Semantics.ConstantName.ConstantStart - #2714815: Enable 'Drupal.Semantics.ConstantName.ConstantStart' coding standard
• Drupal.WhiteSpace.ScopeClosingBrace.BreakIndent
• Generic.CodeAnalysis.UselessOverridingMethod.Found
• Generic.Functions.OpeningFunctionBraceKernighanRitchie.SpaceBeforeBrace
• Generic.PHP.LowerCaseKeyword.Found - #2901779: Fix 'Generic.PHP.LowerCaseKeyword' coding standard
• PSR2.Namespaces.NamespaceDeclaration.BlankLineAfter - #2901744: Fix 'PSR2.Namespaces' coding standard
• PSR2.Namespaces.UseDeclaration.SpaceAfterUse - #2901744: Fix 'PSR2.Namespaces' coding standard
• Squiz.Arrays.ArrayDeclaration.IndexNoNewline - #2901739: Fix 'Squiz.Arrays.ArrayDeclaration' coding standard
• Squiz.Arrays.ArrayDeclaration.KeySpecified - #2901739: Fix 'Squiz.Arrays.ArrayDeclaration' coding standard
• Squiz.ControlStructures.ForEachLoopDeclaration.SpacingBeforeArrow - #2901718: Fix 'Squiz.ControlStructures.ForEachLoopDeclaration' coding standard
• Squiz.ControlStructures.SwitchDeclaration.MissingCase - #2824935: Fix Squiz.ControlStructures.SwitchDeclaration coding standard
• Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingAfterHint - #2901722: Fix 'Squiz.Functions.FunctionDeclarationArgumentSpacing' coding standard
• Squiz.Functions.MultiLineFunctionDeclaration.OneParamPerLine - #2901726: Fix 'Squiz.Functions.MultiLineFunctionDeclaration' coding standard
• Squiz.Functions.MultiLineFunctionDeclaration.SpaceAfterFunction - #2901726: Fix 'Squiz.Functions.MultiLineFunctionDeclaration' coding standard
• Squiz.Functions.MultiLineFunctionDeclaration.SpaceAfterUse - #2901726: Fix 'Squiz.Functions.MultiLineFunctionDeclaration' coding standard
• Squiz.Functions.MultiLineFunctionDeclaration.SpaceBeforeBrace - #2901726: Fix 'Squiz.Functions.MultiLineFunctionDeclaration' coding standard
• Squiz.Functions.MultiLineFunctionDeclaration.SpaceBeforeUse - #2901726: Fix 'Squiz.Functions.MultiLineFunctionDeclaration' coding standard

I modified the issue summary:

• adjusted the paragraph that states how new sniffs are added in phpcs.xml.dist.
• adjusted to Approach section to be more accurate.
• added Remaining tasks section.

I added info about what phpcs.xml.dist should contain eventually and how we should use Drupal CS ruleset.xml.

Drupal.NamingConventions.ValidVariableName.LowerStart should be fixed in #2903331: Fix 'Drupal.NamingConventions.ValidVariableName.LowerStart' coding standard.

See: #2902381: Revert phpcs.xml.dist changes from "Ensure that we're using the right ruleset for coding standards checking"

I removed from Remaining tasks all the sniffs related to Drupal.Commenting.FunctionComment. They are handled in #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard.

There are some child issues that need a review and are quite easy. Like:

• #2901779: Fix 'Generic.PHP.LowerCaseKeyword' coding standard
• #2901791: Fix 'Squiz.ControlStructures.ForLoopDeclaration' coding standard
• #2902396: Add sniffs already passing
• #2902728: Fix 'Drupal.Semantics.FunctionT.BackslashSingleQuote' coding standard
• #2901790: Fix 'PEAR.Functions.ValidDefaultValue' coding standard

Anyone? :)

I think we should focus next on these low complexity issues:

• #2572631: Fix 'Drupal.Classes.InterfaceName' coding standard
• #2759789: Fix 'Drupal.Semantics.FunctionAlias' coding standard
• #2901718: Fix 'Squiz.ControlStructures.ForEachLoopDeclaration' coding standard
• #2901744: Fix 'PSR2.Namespaces' coding standard
• #2901785: Fix 'Generic.Formatting.SpaceAfterCast' coding standard
• #2901789: Fix 'PEAR.Functions.FunctionCallSignature' coding standard
• #2909163: Fix 'Drupal.Commenting.InlineComment.WrongStyle' coding standard
• #2715485: Fix 'Drupal.Commenting.InlineComment.NoSpaceBefore' coding standard
• #2901722: Fix 'Squiz.Functions.FunctionDeclarationArgumentSpacing' coding standard
• #2901735: Fix 'Squiz.Arrays.ArrayBracketSpacing' coding standard
• #2572699: Fix 'Drupal.ControlStructures.InlineControlStructure' coding standard

I added #2909364: [meta] Fix 'Drupal.Commenting.VariableComment' coding standard, a plan for all Drupal.Commenting.VariableComment sniffs, and child issues.

I'm concerned about content of phpcs.xml.dist file
and created follow up issue about approaches used for define and exclude rules.
https://www.drupal.org/node/2912811

I selected (in the order of complexity) another bunch of tickets on which, I think, we should focus on next:

• #2572787: Fix 'Drupal.WhiteSpace.CloseBracketSpacing' coding standard
• #2572795: Fix coding standard for closures - Drupal.WhiteSpace.ScopeClosingBrace and Generic.Functions.OpeningFunctionBraceKernighanRitchie
• #2902407: Fix 'Drupal.Commenting.DataTypeNamespace' coding standard
• #2901562: Fix 'Squiz.WhiteSpace.SuperfluousWhitespace' coding standard
• #2901730: Fix 'Squiz.WhiteSpace.FunctionSpacing' coding standard
• #2824935: Fix Squiz.ControlStructures.SwitchDeclaration coding standard
• #2909368: Fix 'Drupal.Commenting.VariableComment.InlineVariableName' coding standard
• #2723621: Fix Drupal.Commenting.FunctionComment.IncorrectTypeHint and Drupal.Commenting.FunctionComment.InvalidTypeHint
• #2719653: Fix 'Drupal.Commenting.InlineComment.DocBlock' coding standard

I'm a bit tired of re-rolling this ever couple of hours... I'm considering searching out all the cs fixes going on in core and postponing them until this backwards compatibility break and regression is fixed #2902381: Revert phpcs.xml.dist changes from "Ensure that we're using the right ruleset for coding standards checking"

I agree it's not fun chasing head with that issue.
But how soon that patch will be merged once re-rolled and RTBCed? Don't we loose too much time waiting for it? Currently there are about 10-15 issues with recent activity.

Updating here since #2902381: Revert phpcs.xml.dist changes from "Ensure that we're using the right ruleset for coding standards checking" will change the script in the IS. the easiest way to run phpcs after its committed will probably be

$ cd /path/to/drupal/root/
$ composer install
$ cd core
$ composer run-script phpcs

Commited change may need a lot of rerolls #2902381: Revert phpcs.xml.dist changes from "Ensure that we're using the right ruleset for coding standards checking"

Looked through the first page of the coding standards tagged issues and think I got things rerolled that needed it.

Removed Drupal.NamingConventions.ValidVariableName.LowerCamelName from Remaining tasks.
Starting to work on issue #2937514: Fix Drupal.NamingConventions.ValidVariableName.LowerCamelName in tests

Removed Drupal.Commenting.DocComment.TagGroupSpacing from Remaining tasks.

Starting to work on #2937513: Fix 'Drupal.Commenting.DocComment.TagGroupSpacing' coding standard

Removed Drupal.Array.Array.ArrayClosingIndentation from the remaining tasks and starting to work on #2937515: Fix Drupal.Array.Array.[ArrayClosingIndentation, ArrayIndentation] coding standard.

Removed Drupal.Commenting.DocComment.TagsNotGrouped from Remaining tasks and starting to work on #2937552: Fix 'Drupal.Commenting.DocComment.TagsNotGrouped' coding standard.

Removed Squiz.PHP.NonExecutableCode from remaining tasks and added sub-issue with patch for fixing it. #2937844: Fix 'Squiz.PHP.NonExecutableCode' coding standard

Removed Drupal.Formatting.MultipleStatementAlignment from remaining tasks and added sub-issue with patch. #2937853: Fix 'Drupal.Formatting.MultipleStatementAlignment' coding standard

Removed Drupal.Commenting.DocCommentAlignment from remaining tasks and added sub-issue #2937858: Fix 'Drupal.Commenting.DocCommentAlignment' coding standard.

Updated instructions on how to get current state overview.

Also forgot to mention before, in last couple sub-issue created, like #2937844: Fix 'Squiz.PHP.NonExecutableCode' coding standard and #2937858: Fix 'Drupal.Commenting.DocCommentAlignment' coding standard, I have changed issue description because code sniffer and coder are now part of development dependencies of core. So instructions are slightly simplified.

Removed Drupal.Classes.PropertyDeclaration from remaining tasks and added sub-issue with patch #2937882: Fix 'Drupal.Classes.PropertyDeclaration' coding standard.

Removed Drupal.Commenting.DocComment.LongNotCapital from remaining tasks and added sub-issue with patch #2937888: Fix 'Drupal.Commenting.DocComment.LongNotCapital' coding standard.

The 'Drupal.Commenting.ClassComment.Missing' is being fixed via the https://www.drupal.org/project/drupal/issues/3105950

Added a subsection Comments under Remaining tasks to easy tracking comments relevant ones, with remains tasks exacting from core/phpcs.xml.dist file which were excluded and child issues linked.

    <rule ref="Drupal.Commenting.DocComment">
    <!-- Sniff for these errors: SpacingAfterTagGroup, WrongEnd, SpacingBetween,
      ContentAfterOpen, SpacingBeforeShort, TagValueIndent, ShortStartSpace,
      SpacingAfter, LongNotCapital -->
    <!-- ParamNotFirst still not decided for PHPUnit-based tests.
      @see https://www.drupal.org/node/2253915 -->
    <exclude name="Drupal.Commenting.DocComment.ParamNotFirst"/>
    <exclude name="Drupal.Commenting.DocComment.SpacingBeforeTags"/>
    <exclude name="Drupal.Commenting.DocComment.LongFullStop"/>
    <exclude name="Drupal.Commenting.DocComment.ShortNotCapital"/>
    <exclude name="Drupal.Commenting.DocComment.ShortFullStop"/>
    <!-- TagsNotGrouped and ParamGroup have false-positives.
      @see https://www.drupal.org/node/2060925 -->
    <exclude name="Drupal.Commenting.DocComment.TagsNotGrouped"/>
    <exclude name="Drupal.Commenting.DocComment.ParamGroup"/>
    <exclude name="Drupal.Commenting.DocComment.ShortSingleLine"/>
    <exclude name="Drupal.Commenting.DocComment.TagGroupSpacing"/>
    <exclude name="Drupal.Commenting.DocComment.MissingShort"/>
  </rule>

Created 6 more new issues for the rest of the remaining tasks and updated links to IS

There are many more child 'fix' issues in the sidebar which are active and are not listed in "Remaining Tasks". From previous posts it looked like when an issue was created the sniff was deleted from the list in the IS. So I think that's how it is meant to work. All those new issues are set as children to this, so removing them from the summary.

Added child: #3088730: Include 'composer' directory in phpcs scans

Added #3183673: Fix 'Drupal.Commenting.DocComment.ShortFullStop' coding standard - part 1 auto-fixes

The comment in #84 above associated #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard with three sniffs, but this (correctly) now only deals with LongFullStop.

ParamGroup now has #3123058: Fix 'Drupal.Commenting.DocComment.ParamGroup' coding standard but ShortFullStop was missed.

Added how to change drupalci.yml to not run the tests as suggested by jonathan1055 in #3180696#comment-3.

I needed a bit more order to the remaining issues so I added the child issues to the issue summary.

Update IS

Updating list of issues in the IS.

Add new meta, #3268741: [meta] Fix Drupal.Commenting.DocComment.ShortSingleLine

Add related issue

All of the child issues here appear to fix Drupal. standards. Is there a separate issue which works on DrupalPractice. sniffs? Or should those also be added here? Searched for a drupalPractice meta issue but could not find it.

edit
I have now created #3310127: [META] Fix DrupalPractice best practice in Core

Remove completed issues and add error numbers and number of files for some of the issues.

Making this just about PHP and re-parent.

Created a Meta for Fix Drupal.NamingConventions.ValidVariableName.LowerCamelName

#3180696: Fix 'Drupal.Commenting.TodoComment' coding standard was committed.

#3123067: Fix 'Drupal.Semantics.FunctionT.NotLiteralString' coding standard committed, and removed from the issue summary.

I forgot to update the IS. Also adding credit.

#2909364: [meta] Fix 'Drupal.Commenting.VariableComment' coding standard fixed.

This issue has been open for 10 years and there is still a great deal of work to do. I think that the people who have managed this so far should be credited and not have to wait any more years for this to be completed.

Therefore I have updated credit. I am changing the title to include 'stage 1' and will open a new issues for 'stage 2'. I will also change the parent on any still open child. The new issue is #3553320: [meta] Fix PHP coding standards in core, stage 2.

Thank you to everyone who contributed to get the coding standards process for core up and running and for keeping this meta up to date.

Thanks, can I also get credit on this issue? I adapted Coder over the last 10 years to support what drupal core needs 😊

I've credited @klausi - I think it is going to be hard to find everyone who has contributed to this over the years but I agree that @klausi certainly has.

Agree it's time to close this one out and start afresh. Some great work done here, but there is lots more to do!

If anyone else has been missed out from the credits here please let us know. Otherwise, see you in stage 2.