There are 268 sniff violations reported in 41 sources by phpcs.

phpcs --standard=Drupal,DrupalPractice --extensions=php,inc,module,install,info,test,profile,theme modules/contrib/token/

Report:

https://git.drupalcode.org/project/token/-/jobs/668566
To get lastest phpcs report check https://git.drupalcode.org/project/token/-/pipelines

List of violation by rules

Drupal.Arrays.Array.ArrayClosingIndentation                            1
Drupal.Arrays.Array.ArrayIndentation                                   14
Drupal.Arrays.Array.CommaLastItem                                      8
Drupal.Arrays.Array.LongLineDeclaration                                12
Drupal.Arrays.DisallowLongArraySyntax.Found                            1
Drupal.Classes.ClassDeclaration.CloseBraceAfterBody                    15
Drupal.Classes.UnusedUseStatement.UnusedUse                            1
Drupal.Commenting.ClassComment.Missing                                 6
Drupal.Commenting.DocComment.MissingShort                              17
Drupal.Commenting.DocComment.ShortSingleLine                           2
Drupal.Commenting.DocComment.TagGroupSpacing                           1
Drupal.Commenting.FileComment.Missing                                  1
Drupal.Commenting.FileComment.SpacingAfterComment                      1
Drupal.Commenting.FunctionComment.Missing                              33
Drupal.Commenting.FunctionComment.MissingParamComment                  1
Drupal.Commenting.FunctionComment.MissingParamType                     7
Drupal.Commenting.FunctionComment.MissingReturnType                    1
Drupal.Commenting.FunctionComment.ParamCommentIndentation              1
Drupal.Commenting.FunctionComment.ReturnCommentIndentation             1
Drupal.Commenting.FunctionComment.WrongStyle                           1
Drupal.Commenting.HookComment.HookCommentFormat                        3
Drupal.Commenting.InlineComment.InvalidEndChar                         13
Drupal.Commenting.InlineComment.NoSpaceBefore                          4
Drupal.Commenting.InlineComment.NotCapital                             1
Drupal.Commenting.InlineComment.SpacingAfter                           3
Drupal.Commenting.InlineComment.SpacingBefore                          11
Drupal.Commenting.InlineVariableComment.VarInline                      9
Drupal.Commenting.VariableComment.Missing                              3
Drupal.Files.LineLength.TooLong                                        7
Drupal.NamingConventions.ValidFunctionName.NotCamelCaps                1
Drupal.NamingConventions.ValidVariableName.LowerCamelName              2
Drupal.Scope.MethodScope.Missing                                       33
Drupal.Semantics.FunctionT.BackslashSingleQuote                        1
Drupal.WhiteSpace.ScopeIndent.IncorrectExact                           5
DrupalPractice.Commenting.CommentEmptyLine.SpacingAfter                3
Generic.PHP.UpperCaseConstant.Found                                    2
PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse                       1
SlevomatCodingStandard.ControlStructures.RequireNullCoalesceOperator.  12
SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.Incorrectl  11
SlevomatCodingStandard.PHP.ShortList.LongListUsed                      2
Squiz.ControlStructures.SwitchDeclaration.SpacingAfterBreak            4
Squiz.WhiteSpace.FunctionSpacing.AfterLast                             15
Squiz.WhiteSpace.FunctionSpacing.Before                                1
Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines                      1

Fix rules by MR.

MR 54

  • SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.IncorrectlyOrderedUses
  • Drupal.Classes.ClassDeclaration.CloseBraceAfterBody
  • Drupal.Classes.UnusedUseStatement.UnusedUse
  • PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse

MR 55

  • SlevomatCodingStandard.ControlStructures.RequireNullCoalesceOperator.NullCoalesceOperatorNotUsed

MR 56

  • Drupal.Arrays.Array.ArrayClosingIndentation
  • Drupal.Arrays.Array.ArrayIndentation
  • Drupal.Arrays.Array.CommaLastItem
  • Drupal.Arrays.Array.LongLineDeclaration
  • SlevomatCodingStandard.PHP.ShortList.LongListUsed

MR 57

  • Drupal.Scope.MethodScope.Missing
  • Drupal.NamingConventions.ValidFunctionName.NotCamelCaps

MR 58

  • Drupal.Commenting.ClassComment.Missing
  • Drupal.Commenting.DocComment.MissingShort
  • Drupal.Commenting.DocComment.ShortSingleLine
  • Drupal.Commenting.DocComment.TagGroupSpacing
  • Drupal.Commenting.FileComment.Missing
  • Drupal.Commenting.FileComment.SpacingAfterComment
  • Drupal.Commenting.FunctionComment.Missing
  • Drupal.Commenting.FunctionComment.MissingParamComment
  • Drupal.Commenting.FunctionComment.MissingParamType
  • Drupal.Commenting.FunctionComment.MissingReturnType
  • Drupal.Commenting.FunctionComment.ParamCommentIndentation
  • Drupal.Commenting.FunctionComment.ReturnCommentIndentation
  • Drupal.Commenting.FunctionComment.WrongStyle
  • Drupal.Commenting.HookComment.HookCommentFormat
  • Drupal.Commenting.InlineComment.InvalidEndChar
  • Drupal.Commenting.InlineComment.NoSpaceBefore
  • Drupal.Commenting.InlineComment.NotCapital
  • Drupal.Commenting.InlineComment.SpacingAfter
  • Drupal.Commenting.InlineComment.SpacingBefore
  • Drupal.Commenting.InlineVariableComment.VarInline
  • Drupal.Commenting.VariableComment.Missing
  • Drupal.Files.LineLength.TooLong
  • DrupalPractice.Commenting.CommentEmptyLine.SpacingAfter

MR 64

  • Squiz.ControlStructures.SwitchDeclaration.SpacingAfterBreak
  • Squiz.WhiteSpace.FunctionSpacing.AfterLast
  • Squiz.WhiteSpace.FunctionSpacing.Before
  • Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines

MR 65

  • Drupal.NamingConventions.ValidVariableName.LowerCamelName
  • Drupal.Semantics.FunctionT.BackslashSingleQuote
  • Generic.PHP.UpperCaseConstant.Found

Note.

Branch 2774071-fix-cs-base-branch used as base branch for all the MRs. This will add phpcs.xml file, and disable allow-faillure on phpcs check on Gitlab CI.

CommentFileSizeAuthor
#15 2774071-15.patch42.94 KBravi.shankar
#12 description_2774071_11.patch70.88 KBpurvitagupta
#10 fix-coding-standard-related-errors-2774071-10.patch71.95 KBamarphule
#7 codingstandards.txt23.98 KBYago Elias
#7 fix_coding_standard-2774071-7.patch159.64 KBYago Elias
#6 token.txt92.43 KBcmsMinds
#2 fix_coding_standard-2774071-2.patch14.71 KBvinod_kc

Issue fork token-2774071

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vinod_kc created an issue. See original summary.

vinod_kc’s picture

vinod_kc CreditAttribution: vinod_kc at Zyxware Technologies commented
FileSize
fix_coding_standard-2774071-2.patch14.71 KB

As there are lot of coding standard issues, fix can be done in separate patches. Adding my first patch.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
Status: Active » Needs work 
  1. +++ b/src/Plugin/Derivative/DevelLocalTask.php
@@ -9,12 +9,23 @@ use Drupal\Core\StringTranslation\StringTranslationTrait;
+ * Foe devel local tasks.

    Typo 

  2. +++ b/src/Token.php
@@ -88,8 +88,9 @@ class Token extends TokenBase implements TokenInterface {
+        // then it is a global token type that will always be replaced
+        // in any context.

    "in any" can go on the previous line. 

  3. +++ b/src/TreeBuilder.php
@@ -7,6 +7,9 @@ use Drupal\Core\Cache\CacheBackendInterface;
+ * Defines the tree bilder handler class for tokens.

    Typo. 

  4. +++ b/tests/modules/token_module_test/token_module_test.module
@@ -20,7 +25,9 @@ function token_module_test_page_attachments() {
+ * For node entities.

    The description isn't really useful. 

  5. +++ b/tests/modules/token_module_test/token_module_test.tokens.inc
@@ -1,7 +1,12 @@
+ * @file

    This is no longer encouraged. Core has removed @file everywhere. 

  6. +++ b/token.module
@@ -714,8 +731,8 @@ function token_node_menu_link_submit($entity_type, NodeInterface $node, &$form,
+            // important for token generation.

    "important" can go above.

rajeshwari10’s picture

rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented
Assigned: Unassigned » rajeshwari10
rajeshwari10’s picture

rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented
Assigned: rajeshwari10 » Unassigned
cmsMinds’s picture

cmsMinds CreditAttribution: cmsMinds as a volunteer commented
Assigned: Unassigned » cmsMinds
FileSize
token.txt92.43 KB

Please do check coding standards issues.

Yago Elias’s picture

Yago Elias CreditAttribution: Yago Elias as a volunteer and at CI&T commented
FileSize
fix_coding_standard-2774071-7.patch159.64 KB
codingstandards.txt23.98 KB

HI! I couldn't apply patch #2 .

used : phpcs --standard=Drupal --extensions='php,module,inc,install,test,profile,theme,css,info,txt,md'

and then passed phpcbf.

Here is what i have until now.

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen as a volunteer and at DevBranch, Drupal Ukraine Community commented
Issue tags: +LutskGSW18
amarphule’s picture

amarphule CreditAttribution: amarphule at DevsAdda for DevsAdda commented
Assigned: cmsMinds » amarphule
amarphule’s picture

amarphule CreditAttribution: amarphule at DevsAdda for DevsAdda commented
Assigned: amarphule » Unassigned
Status: Needs work » Needs review
Issue tags: -LutskGSW18 +#d8docpune
FileSize
fix-coding-standard-related-errors-2774071-10.patch71.95 KB

Fixed all module code as Drupal standard coding, Still one warning message is in modules/token/css/token.treetable.theme.css

1 | WARNING | File appears to be minified and cannot be processed

.

Status: Needs review » Needs work

The last submitted patch, 10: fix-coding-standard-related-errors-2774071-10.patch, failed testing. View results

purvitagupta’s picture

purvitagupta CreditAttribution: purvitagupta commented
FileSize
description_2774071_11.patch70.88 KB

Hi, here is the updated patch for module code as Drupal standard coding.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Needs review
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

There is a syntax error in that patch.

ravi.shankar’s picture

ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commented
Status: Needs work » Needs review
FileSize
2774071-15.patch42.94 KB

I have fixed.

urvashi_vora’s picture

urvashi_vora CreditAttribution: urvashi_vora at Valuebound for Valuebound commented
Status: Needs review » Reviewed & tested by the community

Hi @ravi

Patch #15 works fine.

Christopher Riley’s picture

Christopher Riley CreditAttribution: Christopher Riley commented

This patch does not apply to 8.x-1.11 so were all of the issues addressed?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Reviewed & tested by the community » Needs work

Like I said in #2933031: Drupal best practices and coding standards:

Coding standard improvements must be provided as merge requests now, so that we can verify it using GitlabCI.

Additionally, this is way too big as a single patch and overlaps with many other issues. This is impossible to review and needs to be split up into issues for specific changes or groups of related changes.

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

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

sakthi_dev opened merge request !53

sakthi_dev’s picture

sakthi_dev CreditAttribution: sakthi_dev at Specbee for Drupal India Association commented
Issue summary: View changes

Updated Issue Summary and also created a MR 53. Please review. Still minified css error is there.

FILE: ...me/administrator/Projects/contribution/token/css/token.treetable.theme.css
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
--------------------------------------------------------------------------------

Nikolay Shapovalov made their first commit to this issue’s fork.

Nikolay Shapovalov opened merge request !54

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue tags: -#d8docpune +Coding standards

As requested by @Berdir I will create several smaller MRs, not sure if each MR should have separate issue.
First one https://git.drupalcode.org/project/token/-/merge_requests/54 is ready for review.

Nikolay Shapovalov opened merge request !55

Nikolay Shapovalov opened merge request !56

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Status: Needs work » Needs review

MRs are ready for review:

Nikolay Shapovalov opened merge request !57

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented

MR 57 is ready for review.

Nikolay Shapovalov opened merge request !58

Nikolay Shapovalov changed the visibility of the branch 2774071-fix-coding-standard to hidden.

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented

MRs are ready for review:

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue summary: View changes
apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno as a volunteer commented
Title: Fix coding standard related errors » Fix the errors/warnings reported by PHP_CodeSniffer

Nikolay Shapovalov changed the visibility of the branch 2774071-fix-comments to hidden.

Nikolay Shapovalov changed the visibility of the branch 2774071-fix-comments to active.

Nikolay Shapovalov opened merge request !64

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented

I create branch 2774071-fix-cs-base-branch with phpcs.xml file and exclude all the rules that has any violation. Also gitlab ci should fail when there is phpcs found.

I rebased all existing MRs on this branch, and remove rules from phpcs that exists in this branch.

My suggestion to Merge base 2774071-fix-cs-base-branch to 8.x-1.x, in that case MR diff will be more transperent, right now it's impossible to see what rules will be fixed in the MR. But I add rules to MR description.

At the moment there are violations that still need to be fixed: 

  <rule ref="Drupal">
    <exclude name="Drupal.NamingConventions.ValidVariableName.LowerCamelName"/>
    <exclude name="Drupal.Semantics.FunctionT.BackslashSingleQuote"/>
    <exclude name="Generic.PHP.UpperCaseConstant.Found"/>
  </rule>

Todo: add list of all violations name with the link to the MR that will fix it.

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue summary: View changes
Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue summary: View changes
Status: Needs review » Needs work
Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue summary: View changes
Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue summary: View changes

Nikolay Shapovalov opened merge request !65

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue summary: View changes

Created MR 65, now all MRs ready for review.

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Status: Needs work » Needs review
Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Issue summary: View changes
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

Not sure what to do with this.

Each of those MR's will conflict as they all change the phpcs and gitlab config.

I'd rather not change those files, including the allow fail. I'd rather rely on the code quality difference, which gitlabCI I think is still working on properly supporting. Then we can see if the situation gets worse or improves in a MR.

drupal.org issues are also not well suited to merge multiple merge requests per issue. We should either split them into separate issues or combine it.