Problem/Motivation

Here is a list of the phpcs issue to fix:

E.E....E.E.... 14 / 14 (100%)



FILE: /var/www/html/web/modules/custom/auto_username/src/Form/AutoUsernameSettingsForm.php
------------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 1 WARNING AFFECTING 5 LINES
------------------------------------------------------------------------------------------------------------------------
  11 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is
     |         |     Drupal\auto_username\AutoUsernameUtilities.
  18 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
  35 | ERROR   | [ ] Missing short description in doc comment
 287 | ERROR   | [x] Expected 1 blank line after function; 2 found
 290 | ERROR   | [x] The closing brace for the class must have an empty line before it
------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/auto_username/src/AutoUsernameUtilities.php
------------------------------------------------------------------------------------------------------------------------
FOUND 48 ERRORS AND 33 WARNINGS AFFECTING 49 LINES
------------------------------------------------------------------------------------------------------------------------
  16 | ERROR   | [x] Missing class doc comment
  18 | ERROR   | [ ] Missing short description in doc comment
  23 | ERROR   | [ ] Missing short description in doc comment
  28 | ERROR   | [ ] Missing short description in doc comment
  33 | ERROR   | [ ] Missing short description in doc comment
  38 | ERROR   | [ ] Missing short description in doc comment
  43 | ERROR   | [ ] Missing short description in doc comment
  48 | ERROR   | [ ] Missing short description in doc comment
  56 | ERROR   | [ ] Missing parameter comment
  57 | ERROR   | [ ] Missing parameter comment
  58 | ERROR   | [ ] Missing parameter comment
  59 | ERROR   | [ ] Missing parameter comment
  60 | ERROR   | [ ] Missing parameter comment
  61 | ERROR   | [ ] Missing parameter comment
  62 | ERROR   | [ ] Missing parameter comment
 121 | ERROR   | [ ] The array declaration extends to column 99 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 121 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 122 | ERROR   | [ ] The array declaration extends to column 113 (the limit is 80). The array content should be split
     |         |     up over multiple lines
 122 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 123 | ERROR   | [ ] The array declaration extends to column 86 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 123 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 124 | ERROR   | [ ] The array declaration extends to column 82 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 124 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 125 | ERROR   | [ ] The array declaration extends to column 83 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 125 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 126 | ERROR   | [ ] The array declaration extends to column 83 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 126 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 127 | ERROR   | [ ] The array declaration extends to column 87 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 127 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 128 | ERROR   | [ ] The array declaration extends to column 82 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 128 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 129 | ERROR   | [ ] The array declaration extends to column 86 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 129 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 130 | ERROR   | [ ] The array declaration extends to column 96 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 130 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 131 | ERROR   | [ ] The array declaration extends to column 95 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 131 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 132 | ERROR   | [ ] The array declaration extends to column 96 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 132 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 133 | ERROR   | [ ] The array declaration extends to column 96 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 133 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 134 | ERROR   | [ ] The array declaration extends to column 97 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 134 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 135 | ERROR   | [ ] The array declaration extends to column 86 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 135 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 136 | ERROR   | [ ] The array declaration extends to column 87 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 136 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 137 | ERROR   | [ ] The array declaration extends to column 85 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 137 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 138 | ERROR   | [ ] The array declaration extends to column 86 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 138 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 139 | ERROR   | [ ] The array declaration extends to column 89 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 139 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 140 | ERROR   | [ ] The array declaration extends to column 82 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 140 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 141 | ERROR   | [ ] The array declaration extends to column 88 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 141 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 142 | ERROR   | [ ] The array declaration extends to column 107 (the limit is 80). The array content should be split
     |         |     up over multiple lines
 142 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 143 | ERROR   | [ ] The array declaration extends to column 84 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 143 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 144 | ERROR   | [ ] The array declaration extends to column 93 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 144 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 145 | ERROR   | [ ] The array declaration extends to column 82 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 145 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 146 | ERROR   | [ ] The array declaration extends to column 93 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 146 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 147 | ERROR   | [ ] The array declaration extends to column 94 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 147 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 148 | ERROR   | [ ] The array declaration extends to column 90 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 148 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 149 | ERROR   | [ ] The array declaration extends to column 91 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 149 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 150 | ERROR   | [ ] The array declaration extends to column 94 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 150 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 151 | ERROR   | [ ] The array declaration extends to column 82 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 151 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 152 | ERROR   | [ ] The array declaration extends to column 87 (the limit is 80). The array content should be split up
     |         |     over multiple lines
 152 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 203 | WARNING | [ ] The use of function eval() is discouraged
 325 | ERROR   | [ ] The array declaration extends to column 89 (the limit is 80). The array content should be split up
     |         |     over multiple lines
------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/auto_username/config/install/auto_username.settings.yml
----------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------
 11 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/auto_username/auto_username.module
------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AND 10 WARNINGS AFFECTING 7 LINES
------------------------------------------------------------------------------------------------------------------------
 13 | WARNING | [ ] All constants defined by a module must be prefixed with the module's name, expected
    |         |     "AUTO_USERNAME_AUN_CASE_LEAVE_ASIS" but found "AUN_CASE_LEAVE_ASIS"
 13 | WARNING | [ ] Global constants should not be used, move it to a class or interface
 18 | WARNING | [ ] All constants defined by a module must be prefixed with the module's name, expected
    |         |     "AUTO_USERNAME_AUN_CASE_LOWER" but found "AUN_CASE_LOWER"
 18 | WARNING | [ ] Global constants should not be used, move it to a class or interface
 23 | WARNING | [ ] All constants defined by a module must be prefixed with the module's name, expected
    |         |     "AUTO_USERNAME_AUN_PUNCTUATION_REMOVE" but found "AUN_PUNCTUATION_REMOVE"
 23 | WARNING | [ ] Global constants should not be used, move it to a class or interface
 28 | WARNING | [ ] All constants defined by a module must be prefixed with the module's name, expected
    |         |     "AUTO_USERNAME_AUN_PUNCTUATION_REPLACE" but found "AUN_PUNCTUATION_REPLACE"
 28 | WARNING | [ ] Global constants should not be used, move it to a class or interface
 33 | WARNING | [ ] All constants defined by a module must be prefixed with the module's name, expected
    |         |     "AUTO_USERNAME_AUN_PUNCTUATION_DO_NOTHING" but found "AUN_PUNCTUATION_DO_NOTHING"
 33 | WARNING | [ ] Global constants should not be used, move it to a class or interface
 63 | ERROR   | [x] Short array syntax must be used to define arrays
 93 | ERROR   | [x] Expected 1 newline at end of file; 3 found
------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------

Steps to reproduce

Proposed resolution

Refactor legacy code and fix phpcs issues.

Remaining tasks

User interface changes

API changes

Data model changes

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:

Comments

Grevil created an issue. See original summary.

anybody’s picture

@Grevil: Perhaps we should better split off the code style fixes from the probably legacy code?

An example of legacy code would be the implementation of "auto_username_user_insert". Instead of simply calling
$user->setUsername($new_name) we access the database and change the name there.

I think these are different concerns and the maintainer should have a look at that code, while fixing the code style issues should be a quick-win?

grevil’s picture

Title: Refactor legacy code and fix phpcs issues » Fix phpcs issues
Issue summary: View changes

Sure thing! Although a lot phpcs issues are related to legacy code.

shreya_th’s picture

Assigned: Unassigned » shreya_th

shreya_th’s picture

Assigned: shreya_th » Unassigned
Status: Active » Needs review

Hi @Grevil,
I have fixed all the code standard issues and also created MR for this issue . Kindly review the changes.

shreya_th’s picture

StatusFileSize
new111.21 KB

Attached screenshot for your reference.

anybody’s picture

Status: Needs review » Needs work

Thanks @Shreya_th looks good, but some things need to be resolved, see my comments.

mohd sahzad’s picture

Assigned: Unassigned » mohd sahzad
mohd sahzad’s picture

Assigned: mohd sahzad » Unassigned
viren18febs’s picture

Status: Needs work » Needs review
liam morland’s picture

Status: Needs review » Needs work

The two new class files have no newline at end of file.

The constants are moved into these class files but the new constants are not used.

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

sidharth_soman’s picture

Status: Needs work » Needs review
tirupati_singh’s picture

Status: Needs review » Needs work
StatusFileSize
new142.99 KB

I've installed this module but getting error after installing this module. Attaching screenshot for reference. This is occurred after fixing phpcs issues.

tirupati_singh’s picture

Status: Needs work » Needs review

Fixed issue of string translation undefined method, the module is now being install. Please review.

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

venkatraman ganesan’s picture

I ran the command phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml and found no phpcs errors.

liam morland’s picture

For a full test, use phpcbf --standard=Drupal,DrupalPractice.

pray_12’s picture

hello,
I ran the command phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml and found no phpcs errors.

liam morland’s picture

Perhaps your version of phpcs is out-of-date. I got a bunch of errors when I that command. You do need to provide a filename to check. I usually check the current directory by adding a . at the end of the command you gave and running it from the repository directory.

yashaswi18’s picture

Is this the command phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml .?
I ran this command inside the repository directory, still not able to find any remaining errors or warnings. Please correct if the command I used is wrong.

Branch: 3391633-fix-phpcs-issues

liam morland’s picture

This is the exact command I use:

phpcs --standard=Drupal,DrupalPractice --report-width=auto --colors --extensions=php,module,inc,install,test,profile,theme,info,txt,md,scss,yml,twig .

I don't know why we're getting different results. If automated testing is configured that can be used as the ultimate arbiter of coding standards.

silvi.addweb’s picture

Status: Needs review » Needs work
StatusFileSize
new51.15 KB

I have tested and I got some deprecation error in create_function(), please review attached image.

liam morland’s picture

In the merge request, I have added .gitlab-ci.yml so that coding standards checks will run.

liam morland’s picture

Issue tags: +PHP 8.0

create_function() was removed in PHP 8.0. It needs to be removed. It is used once in src/AutoUsernameUtilities.php.

tirupati_singh’s picture

StatusFileSize
new105.44 KB

@Liam Morland, I've used the exact command that you've provided phpcs --standard=Drupal,DrupalPractice --report-width=auto --colors --extensions=php,module,inc,install,test,profile,theme,info,txt,md,scss,yml,twig ., still I'm not getting any phpcs issues. Attaching the screenshot for reference. Moving the issue status to Needs Review.

tirupati_singh’s picture

Status: Needs work » Needs review
liam morland’s picture

Looks good. phpcs is passing on the GitLab CI.

liam morland’s picture

Status: Needs review » Needs work

Sorry, commit 3a3f791, "Fixed phpcs issues", adds the use of create_function(), which was removed in PHP 8.0.

A good way to start would be to merge commit 3cb2db2, "Add .gitlab-ci.yml from template". Then every merge request will be getting coding standards checking.

liam morland’s picture

Status: Needs work » Needs review

I reverted the change that introduced create_function(). I added a phpcs:ignore so that the use of eval() does not cause a phpcs failure.

I have fixed some spelling errors and added a list of words to ignore. Now all checks pass.

paraderojether’s picture

Status: Needs review » Reviewed & tested by the community

Hi

I reviewed MR!5, and confirmed it fixes all the issues reported by phpcs.

auto_username git:(8.x-1.5) curl https://git.drupalcode.org/project/auto_username/-/merge_requests/5.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 25823    0 25823    0     0  67039      0 --:--:-- --:--:-- --:--:-- 68496
patching file .gitlab-ci.yml
patching file auto_username.module
patching file auto_username.services.yml
patching file 'config/install/auto_username.settings.yml'
patching file 'src/AutoUsernameOptions.php'
patching file 'src/AutoUsernamePunctuationOptions.php'
patching file 'src/AutoUsernameUtilities.php'
patching file 'src/Form/AutoUsernameSettingsForm.php'
➜  auto_username git:(8.x-1.5) ✗ cd ..
➜  contrib phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig auto_username
➜  contrib

Moving this to RTBC.
Thank you.

  • jlscott committed ea700989 on 8.x-1.x
    Issue #3391633 by Liam Morland, Shreya_th, sidharth_soman,...
jayelless’s picture

Status: Reviewed & tested by the community » Fixed

Tanks everyone for your contributions in improving the module. Changes committed.

liam morland’s picture

Status: Fixed » Needs review
StatusFileSize
new764 bytes

There is one more error; fix attached.

tirupati_singh’s picture

Fixed the remaining phpcs issue. Please review the MR.

hetal.solanki’s picture

Assigned: Unassigned » hetal.solanki
riddhi.addweb’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new102.59 KB

The mentioned phpcs issue is resolved, & I have also checked and it is working as expected. I am attaching the Screenshots & doing RTBC for the same.

hetal.solanki’s picture

Assigned: hetal.solanki » Unassigned
StatusFileSize
new28.14 KB

@Grevil, @Tirupati_Singh

I have tested MR and it is working properly with no errors found. I will move it forward to RTBC.

Thank you!!

jayelless’s picture

Status: Reviewed & tested by the community » Fixed

Merged to 8.x-1.x

  • jayelless committed ea700989 on 2.0.x
    Issue #3391633 by Liam Morland, Shreya_th, sidharth_soman,...

liam morland’s picture

Version: 8.x-1.x-dev » 2.0.x-dev
Status: Fixed » Needs review

phpcs now passes.

jayelless’s picture

Status: Needs review » Fixed

Merged to branch 2.0.x.

Thanks for all the help.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.