Fix the issues reported by phpcs

Created on 25 August 2022, over 2 years ago
Updated 11 July 2024, 5 months ago

Problem/Motivation

FILE: /home/drupal/myproject/groupmenu/modules/groupmenu_block/src/Plugin/Block/GroupMenuBlock.php
------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
------------------------------------------------------------------------------
  67 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 112 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 113 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 114 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
------------------------------------------------------------------------------

FILE: /home/drupal/myproject/groupmenu/groupmenu.module
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 160 | WARNING | Unused variable $item.
----------------------------------------------------------------------

FILE: /home/drupal/myproject/groupmenu/src/Form/GroupMenuSettingsForm.php
------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------
 10 | WARNING | The class short comment should describe what the class does and not simply repeat the class name
------------------------------------------------------------------------

FILE: /home/drupal/myproject/groupmenu/src/GroupMenuConfigOverrides.php
------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------
 190 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
------------------------------------------------------------------------

FILE: /home/drupal/myproject/groupmenu/src/Controller/GroupMenuController.php
------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
------------------------------------------------------------------------
  98 | WARNING | Unused variable $plugin_id.
 125 | ERROR   | The array declaration extends to column 85 (the limit is 80). The array content should be split up over multiple lines
------------------------------------------------------------------------

Steps to reproduce

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig "module_name"

Proposed resolution

Resolve the PHPCS issues.

๐Ÿ“Œ Task
Status

RTBC

Version

1.0

Component

Documentation

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia Dharti Patel

Live updates comments and jobs are added and updated live.
  • Coding standards

    It involves compliance with, or the content of coding standards. Requires broad community agreement.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, report which command has been used, which arguments have been used, and which report that command shown.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia indrapatil Bangalore
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia indrapatil Bangalore
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
       protected function getGroupMenus(GroupInterface $group) {
    -    return \Drupal::service('groupmenu.menu')->loadUserGroupMenusByGroup('view', $group->id());
    +    return $this->service('groupmenu.menu')->loadUserGroupMenusByGroup('view', $group->id());
       }

    There is any service() method for that class or its parent classes.

    +      $tree = $this->menuTree()->load($menu_name, $parameters);
    +      $tree = $this->menuTree()->transform($tree, $manipulators);

    That method does not exist either.

    -    foreach ($this->addPageBundles($group, $create_mode) as $plugin_id => $bundle_name) {
    +    foreach ($this->addPageBundles($group, $create_mode) as => $bundle_name) {
    

    That is not correct PHP code.

     /**
    - * Class GroupMenuSettingsForm.
    + * Class of GroupMenuSettingsForm.
      */
    

    Adding a preposition is not sufficient to write a correct documentation comment. The comment is still just showing the class name, not its purpose.

         // We can't use dependency injection for entity type manager, since this
         // will cause circular dependencies.
    -    $entity_type_manager = \Drupal::service('entity_type.manager');
    +    $entity_type_manager = $this->service('entity_type.manager');
    

    See the comment before the code. The maintainer is explaining why dependency injection has not been used.

  • Assigned to kkalashnikov
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia kkalashnikov Ghaziabad, India
  • Issue was unassigned.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia kkalashnikov Ghaziabad, India

    Fixed rest of the phpcs issues and include #12 comment as well.

  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia kkalashnikov Ghaziabad, India
  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    +  /**
    +   * Group menu service.
    +   *
    +   * @var \Drupal\groupmenu\GroupMenuService
    +   */
    +  protected $groupMenuService;
    +
    +  /**
    +   * Menu link tree service.
    +   *
    +   * @var \Drupal\Core\Menu\MenuLinkTreeInterface
    +   */
    +  protected $menuTree;

    The descriptions are each missing an article.

    +  /**
    +   * Constructs a new group menu block.
    +   *

    The comment does not show the class name nor its namespace.

    -    // We can't use dependency injection for entity type manager, since this
    -    // will cause circular dependencies.
    -    $entity_type_manager = \Drupal::service('entity_type.manager');
    -
         $plugin_id = 'group_menu:menu';
    -    $group_content_types = $entity_type_manager->getStorage('group_content_type')
    +    $group_content_types = $this->entityTypeManager->getStorage('group_content_type')
    We can't use dependency injection for entity type manager, since this will cause circular dependencies.
  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia kkalashnikov Ghaziabad, India
  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    +  /**
    +   * Constructs a group menu block.
    +   *
    +   * @param \Drupal\groupmenu\GroupMenuService $groupMenuService

    The class name and its namespace are still missing from the short description.

       /**
    -   * Constructs a new GroupMenuController.
    +   * Constructs a GroupMenuController.
        *

    The namespace is still missing.

  • First commit to issue fork.
  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Mahima_Mathur23

    Changes suggested by @aparderno has been incorporated in the MR changes done in #20 by @mukesh88.

  • Status changed to Needs work 9 months ago
  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands seanB Netherlands

    Thanks for this. There is 1 thing that I noticed, the entity type manager is now injected when we explicitly didn't do that before to prevent a circular dependency.

  • Assigned to nitin_lama
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India

    Please review. Thanks.

  • Issue was unassigned.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    apaderno โ†’ changed the visibility of the branch 3305893-phpcs-coding-standard to hidden.

  • Pipeline finished with Success
    9 months ago
    Total: 139s
    #119243
  • Status changed to Needs work 9 months ago
  • First commit to issue fork.
  • Pipeline finished with Success
    9 months ago
    Total: 139s
    #119645
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia pray_12

    There is one error that needs to be addressed.

    FILE: /groupmenu/src/GroupMenuConfigOverrides.php
    ------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ------------------------------------------------------------------------------------------------------
     190 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    ------------------------------------------------------------------------------------------------------
    
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India

    @pray_12, as per comment #23. we are not sure if DI issue is fixed or not. For now i have reverted the DI for entity type manager.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia dev16.addweb

    silvi.addweb โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Success
    6 months ago
    Total: 177s
    #202753
  • Status changed to RTBC 5 months ago
  • Hi

    Applied latest changes on MR !6, confirmed no errors remaining.

    groupmenu git:(3.0.0-alpha1) curl https://git.drupalcode.org/project/groupmenu/-/merge_requests/6.diff | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100 19680    0 19680    0     0  44509      0 --:--:-- --:--:-- --:--:-- 45874
    patching file .gitlab-ci.yml
    patching file groupmenu.module
    patching file modules/groupmenu_block/src/Plugin/Block/GroupMenuBlock.php
    patching file src/Access/GroupMenuAccessInterface.php
    patching file src/Controller/GroupMenuController.php
    patching file src/Form/GroupMenuSettingsForm.php
    patching file src/GroupMenuConfigOverrides.php
    patching file src/GroupMenuContentListBuilder.php
    patching file src/GroupMenuListBuilder.php
    patching file src/GroupMenuService.php
    patching file src/GroupMenuServiceInterface.php
    patching file src/Plugin/GroupContentEnabler/GroupMenu.php
    patching file src/Plugin/GroupContentEnabler/GroupMenuDeriver.php
    โžœ  groupmenu git:(3.0.0-alpha1) โœ— cd ..
    โžœ  contrib git:(master) โœ— phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig groupmenu
    โžœ  contrib git:(master) โœ—

    Will now move this to RTBC

    Thanks,
    Jake

Production build 0.71.5 2024