Problem/Motivation

Steps to reproduce

Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shows the following warnings/errors, which should be fixed.

Result:

FILE: ...toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The
   |       |     first wrong one is
   |       |     Drupal\Core\Config\ConfigFactoryInterface.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...toolbar/admin_toolbar_tools/src/Controller/ToolbarController.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 17 | ERROR | [x] Use statements should be sorted alphabetically. The
    |       |     first wrong one is
    |       |     Drupal\Core\Template\TwigEnvironment.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...m/admin_toolbar/admin_toolbar_search/admin_toolbar_search.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 9 | ERROR | [x] Use statements should be sorted alphabetically. The
   |       |     first wrong one is
   |       |     Drupal\Core\Routing\RouteMatchInterface.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /app/web/modules/custom/admin_toolbar/admin_toolbar.module
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 11 | ERROR | [x] Use statements should be sorted alphabetically. The
    |       |     first wrong one is Drupal\Component\Utility\Html.
 60 | ERROR | [ ] All functions defined in a module file must be
    |       |     prefixed with the module's name, found
    |       |     "toolbar_tools_menu_navigation_links" but expected
    |       |     "admin_toolbar_toolbar_tools_menu_navigation_links"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...bar_links_access_filter/admin_toolbar_links_access_filter.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 11 | ERROR | [x] Use statements should be sorted alphabetically. The
    |       |     first wrong one is
    |       |     Drupal\Core\Routing\RouteMatchInterface.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Proposed resolution

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

Tirupati_Singh created an issue. See original summary.

tirupati_singh’s picture

Assigned: tirupati_singh » Unassigned
Status: Active » Needs review

I've fixed phpcs issues please review.

clarkssquared’s picture

Hi

I confirmed that your MR !66 fixes most of the issues but I will retain the status to needs review because there is still a PHPCS warnings flagged in the module so that others can review and give feedback if the warnings needs to be fixed or can be ignored.

➜  admin_toolbar git:(master) ✗ curl https://git.drupalcode.org/project/admin_toolbar/-/merge_requests/66.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  5437    0  5437    0     0  10558      0 --:--:-- --:--:-- --:--:-- 10681
patching file admin_toolbar.module
patching file 'admin_toolbar_links_access_filter/admin_toolbar_links_access_filter.module'
patching file 'admin_toolbar_search/admin_toolbar_search.module'
patching file 'admin_toolbar_tools/src/Controller/ToolbarController.php'
patching file 'admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks.php'
patching file 'src/Render/Element/AdminToolbar.php'
➜  admin_toolbar git:(master) ✗ ..
➜  contrib git:(master) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml admin_toolbar 

FILE: ...bing-subing/Projects/d9/d9-local/web/modules/contrib/admin_toolbar/admin_toolbar_links_access_filter/admin_toolbar_links_access_filter.info.yml
-----------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-----------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/admin_toolbar/admin_toolbar_search/admin_toolbar_search.info.yml
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-----------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/admin_toolbar/admin_toolbar_tools/admin_toolbar_tools.info.yml
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/admin_toolbar/admin_toolbar.info.yml
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-------------------------------------------------------------------------------------------------------------

Time: 937ms; Memory: 16MB

➜  contrib git:(master) ✗ 

tirupati_singh’s picture

StatusFileSize
new53.53 KB

Hi @clarkssquared, I've run the phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig command but didn't get any mentioned warnings. Attaching the screenshot for your reference.

silvi.addweb’s picture

After applying patch, I'm still receiving several phpcs problems.

FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/admin_toolbar/admin_toolbar_search/admin_toolbar_search.info.yml
---------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
---------------------------------------------------------------------------------------------------------------------------------------------


FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/admin_toolbar/admin_toolbar.info.yml
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-----------------------------------------------------------------------------------------------------------------


FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/admin_toolbar/PATCHES.txt
------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------
 1 | WARNING | [ ] Line exceeds 80 characters; contains 104 characters
 5 | ERROR   | [x] Expected 1 newline at end of file; 3 found
------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------


FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/admin_toolbar/admin_toolbar_tools/admin_toolbar_tools.info.yml
-------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-------------------------------------------------------------------------------------------------------------------------------------------


FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/admin_toolbar/admin_toolbar_links_access_filter/admin_toolbar_links_access_filter.info.yml
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
tirupati_singh’s picture

@silvi.addweb, I've fixed the phpcs issues. Please apply the patch again and verify the changes.

silvi.addweb’s picture

Status: Needs review » Reviewed & tested by the community

Hii,
Yes, I've checked and all phpcs issues are resolved now.

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

rajeshreeputra’s picture

Changes looks good, CI passing, can we get it merged? this will unblock the Drupal 11 compatibility issue - #3363604: Add Drupal 11 support.
Thanks!

dydave’s picture

Title: Fix issues reported by phpcs. » GitlabCI: Fix PHPCS and PHPStan validation errors
Version: 3.4.2 » 3.x-dev
Category: Bug report » Task
Status: Reviewed & tested by the community » Needs review

Rebased MR!66 on 3.x with gitlabci tests and required phpcs and phpstan jobs to pass.

✅ Merge request seems to complete successfully with all PHPCS and PHPSTAN validation errors fixed, see:

 
Back to Needs review for more reviews, testing and feedback.
Thanks!

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

dydave’s picture

Thanks Jakob (@japerry)!

Ticket's merge request MR!66 should be completing successfully with PHPCS and PHPStan jobs passing 🟢

Everything is green except ESLint:
https://git.drupalcode.org/issue/admin_toolbar-3411005/-/pipelines/242227

Hope we could get this one in as well!
Thanks in advance!

japerry’s picture

Status: Needs review » Fixed
japerry’s picture

Made PHPstan optional as its more likely to change due to no fault of the module.

Status: Fixed » Closed (fixed)

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