Fix the issues reported by phpcs

Created on 27 March 2023, almost 2 years ago
Updated 16 July 2024, 6 months ago

Problem/Motivation

Getting following error/warnings.

FILE: /var/www/html/modules/contrib/relevant_content/src/RelevantContentService.php
---------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------------
59 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
133 | WARNING | Node::loadMultiple calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/relevant_content/src/Plugin/Derivative/RelevantContentBlock.php
---------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------
10 | ERROR | Doc comment is empty
22 | ERROR | Doc comment is empty
---------------------------------------------------------------------------------------------------

Time: 12.04 secs; Memory: 6MB

Steps to reproduce

Run following command

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml modules/contrib/relevant_content/

Proposed resolution

Above error/warnings need to be fixed.

šŸ“Œ Task
Status

Needs work

Version

1.0

Component

Code

Created by

šŸ‡®šŸ‡³India samit.310@gmail.com

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

Comments & Activities

  • Issue created by @samit.310@gmail.com
  • Issue was unassigned.
  • Status changed to Needs review almost 2 years ago
  • šŸ‡®šŸ‡³India samit.310@gmail.com

    Above errors/warnings has been fixed.

  • Assigned to akshaydalvi212
  • šŸ‡®šŸ‡³India akshaydalvi212

    I will review the provided #2 patch.

  • Issue was unassigned.
  • Status changed to Needs work almost 2 years ago
  • šŸ‡®šŸ‡¹Italy apaderno Brescia, šŸ‡®šŸ‡¹
     /**
    - *
    + * The Derivative class for RelevantContentBlock.
      */
     class RelevantContentBlock extends DeriverBase implements ContainerDeriverInterface {
    

    Derivative is not spelled correctly.
    That description just repeats what the code already says clear and it does not describe the class purpose.

       /**
    -   *
    +   * Constructs a new RelevantContentBlock instance.
        */

    The class namespace is missing.
    Parameters are not documented.

  • Assigned to akshaydalvi212
  • šŸ‡®šŸ‡³India akshaydalvi212

    As per #4 suggestion i will provide the patch for solving remaining issues as suggested.

  • Issue was unassigned.
  • Status changed to Needs review almost 2 years ago
  • šŸ‡®šŸ‡³India akshaydalvi212

    fixed the suggested issues for coding standards.

  • Status changed to Needs work almost 2 years ago
  • šŸ‡®šŸ‡¹Italy apaderno Brescia, šŸ‡®šŸ‡¹
     /**
    - *
    + * The RelevantContentBlock class to get relevant content from storage.
      */

    It is not necessary to repeat the class name. Probably it is not even necessary to say class.
    to get relevant content from storage should probably be expanded a little, to make clear which content that is. from storage is probably not necessary.

       /**
    +   * Constructs a new RelevantContentBlock instance.
        *
    +   * @param \Drupal\Core\Entity\EntityStorageInterface $preset_storage
    +   *   The preset stoage instance.
        */
       public function __construct(EntityStorageInterface $preset_storage) {
    

    The namespace is still missing.

  • Status changed to Needs review almost 2 years ago
  • šŸ‡®šŸ‡³India akshaydalvi212

    updated the class comment as per suggestion and also added the namespace.
    kindly review.

  • Status changed to RTBC almost 2 years ago
  • šŸ‡®šŸ‡³India TanujJain-TJ

    Tested patch #8 on drupal: 10.1.x, the patch applied successfully and removed all the errors reported by phpcs, and addresses all the points mentioned in #4 and #7. RTBC

  • Status changed to Needs work almost 2 years ago
  • šŸ‡®šŸ‡¹Italy apaderno Brescia, šŸ‡®šŸ‡¹

    The patch could apply, but it is wrong.

     namespace Drupal\relevant_content\Plugin\Derivative;
     
    +namespace Drupal\Core\Entity\EntityStorageInterface;
    +

    There must be a single line defining the namespace.
    Then, contributed modules do not use namespaces starting with Drupal\Core.

     /**
    - *
    + * To get relevant content list as per the categorization of current page.
      */

    Remove To.

       /**
    +   * Constructs a new RelevantContentBlock instance.
        *
    +   * @param \Drupal\Core\Entity\EntityStorageInterface $preset_storage
    +   *   The preset stoage instance.
        */

    The class namespace is missing.
    There is a typo in the parameter description. Furthermore, it is not clear what a preset storage would be; The entity storage service. is probably better.

       /**
        * Creates a Relevant Content Service class.
    +   *
    +   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
    +   *   The container service.
    +   * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher
    +   *   The eventDispatcher service.
    +   * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
    +   *   The route match service.
    +   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
    +   *   The entity type manager service.

    Since that comment is changed, also the first line needs to be changed.

  • Assigned to akshaydalvi212
  • Issue was unassigned.
  • Status changed to Needs review almost 2 years ago
  • šŸ‡®šŸ‡³India akshaydalvi212

    Providing #12 patch with all the changes mentioned by @apaderno in #10.
    @apaderno thanks for all the suggestions.
    kindly review the patch.

  • Hi, Reviewed the patch #12 , applied cleanly fixes all the phpcs error/warnings and addresses the comments in #10.

  • Status changed to Needs work 6 months ago
  • Hi @akshaydalvi212,

    Applied your patch, it was applied successfully however, it threw few errors. Please see below:

    relevant_content git:(1.x) curl https://www.drupal.org/files/issues/2023-03-29/3350530-12.patch | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100  4806  100  4806    0     0  15268      0 --:--:-- --:--:-- --:--:-- 15913
    patching file relevant_content.services.yml
    patching file src/Plugin/Derivative/RelevantContentBlock.php
    patching file src/RelevantContentService.php
    āžœ  relevant_content git:(1.x) āœ— cd ..
    āžœ  contrib git:(main) āœ— phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig relevant_content
    
    FILE: ...ules/contrib/relevant_content/src/Form/RelevantContentPresetDeleteForm.php
    --------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    --------------------------------------------------------------------------------
     7 | ERROR | [x] Use statements should be sorted alphabetically. The first
       |       |     wrong one is Drupal\Core\Form\FormStateInterface.
    --------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------
    
    
    FILE: ...gissue/web/modules/contrib/relevant_content/src/RelevantContentService.php
    --------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    --------------------------------------------------------------------------------
     10 | ERROR | [x] Use statements should be sorted alphabetically. The first
        |       |     wrong one is Drupal\Core\Entity\EntityTypeManagerInterface.
    --------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------
    
    Time: 286ms; Memory: 10MB

    Kindly check

    Thanks,
    Jake

Production build 0.71.5 2024