Fix the issues reported by phpcs

Created on 31 May 2022, about 2 years ago
Updated 4 June 2024, 19 days ago
📌 Task
Status

Needs review

Version

1.0

Component

Code

Created by

🇮🇳India Meeni_Dhobale

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.

  • 🇮🇳India arpitk

    The patch #4 applied cleanly however there is still following errors/warnings reported :

    FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/src/EventSubscriber/OrderEventSubscriber.php
    -------------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
    -------------------------------------------------------------------------------------------------------------------------
    52 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    166 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    -------------------------------------------------------------------------------------------------------------------------

    FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/src/Form/StockInventoryControlForm.php
    -------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
    -------------------------------------------------------------------------------------------------------------------
    142 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    145 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    164 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    -------------------------------------------------------------------------------------------------------------------

    FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/src/OrderProcessor/AvailabilityOrderProcessor.php
    ------------------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ------------------------------------------------------------------------------------------------------------------------------
    30 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    ------------------------------------------------------------------------------------------------------------------------------

    FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/js/inventory_control.js
    ----------------------------------------------------------------------------------------------------
    FOUND 44 ERRORS AFFECTING 22 LINES
    ----------------------------------------------------------------------------------------------------
    3 | ERROR | [x] Space before opening parenthesis of function call prohibited
    4 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    4 | ERROR | [x] Expected 1 space before opening brace; found 0
    11 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    11 | ERROR | [x] Expected 1 space before opening brace; found 0
    13 | ERROR | [x] Functions must not contain multiple empty lines in a row; found 2 empty lines
    15 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    17 | ERROR | [x] Expected 1 space before "=="; 0 found
    17 | ERROR | [x] Expected 1 space after "=="; 0 found
    23 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    23 | ERROR | [x] Expected 1 space before opening brace; found 0
    29 | ERROR | [x] Whitespace found at end of line
    30 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    30 | ERROR | [x] Expected 1 space before opening brace; found 0
    37 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    42 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    53 | ERROR | [x] Whitespace found at end of line
    66 | ERROR | [x] Expected 1 space before "+"; 0 found
    66 | ERROR | [x] Expected 1 space after "+"; 0 found
    71 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    79 | ERROR | [x] Expected 1 space before "+"; 0 found
    79 | ERROR | [x] Expected 1 space after "+"; 0 found
    79 | ERROR | [x] Expected 1 space before "+"; 0 found
    79 | ERROR | [x] Expected 1 space after "+"; 0 found
    80 | ERROR | [x] Expected 1 space before "+"; 0 found
    80 | ERROR | [x] Expected 1 space after "+"; 0 found
    80 | ERROR | [x] Expected 1 space before "+"; 0 found
    80 | ERROR | [x] Expected 1 space after "+"; 0 found
    80 | ERROR | [x] Expected 1 space before "+"; 0 found
    80 | ERROR | [x] Expected 1 space after "+"; 0 found
    81 | ERROR | [x] Expected 1 space before "+"; 0 found
    81 | ERROR | [x] Expected 1 space after "+"; 0 found
    81 | ERROR | [x] Expected 1 space before "+"; 0 found
    81 | ERROR | [x] Expected 1 space after "+"; 0 found
    82 | ERROR | [x] Expected 1 space before "+"; 0 found
    82 | ERROR | [x] Expected 1 space after "+"; 0 found
    82 | ERROR | [x] Expected 1 space before "+"; 0 found
    82 | ERROR | [x] Expected 1 space after "+"; 0 found
    82 | ERROR | [x] Expected 1 space before "+"; 0 found
    82 | ERROR | [x] Expected 1 space after "+"; 0 found
    89 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    90 | ERROR | [x] Inline control structures are not allowed
    98 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
    99 | ERROR | [x] Equals sign not aligned correctly; expected 1 space but found 2 spaces
    ----------------------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 44 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------------------------

    command used phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,js .

    Working on updating the patch.

    Thanks!

  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • 🇮🇳India arpitk

    Here is the updated patch. Please review.

    Thanks!

  • Status changed to RTBC about 1 year ago
  • 🇵🇭Philippines clarkssquared

    Hi arpitk

    I applied your patch #8 to my local with Drupal 9.5.9 and confirmed that your patch fixed all the PHPCS errors/issue,

    Please look at the screenshots I provided for your reference.

    Thank you.

  • Status changed to Needs work about 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    The patch could fix the reported errors/warnings, but it contains wrong changes.

     /**
    - * Class CommerceSimpleStockController.
    + * Implementation of Commerce Simple Stock Controller.
      */

    There is no need to start a class description with Implementation of, especially when it is followed by the class name (which is essentially saying the class is an implementation of itself). That description does not say what the class does.

    +  /**
    +   * The entity_type.manager service.
    +   *
    +   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
    +   *   The entity_type.manager service.
    +   */
    +  protected $entityTypeManager;

    The property description is only given before the @var line. The entity type manager. is a sufficient description.

    +  /**
    +   * The config factory service.
    +   *
    +   * @var \Drupal\Core\Config\ConfigFactoryInterface
    +   */
    +  protected $configFactory;
    +

    There is no need to say it is a service.

    -    $events['commerce_order.place.post_transition'] = ['postTransitionOrder', -100];
    -    $events['commerce_order.cancel.post_transition'] = ['postTransitionCancelOrder', -100];
    +    $events['commerce_order.place.post_transition'] = ['postTransitionOrder',
    +      -100,
    +    ];
    +    $events['commerce_order.cancel.post_transition'] = ['postTransitionCancelOrder',
    +      -100,
    +    ];
    -          if ($order && !in_array($order->getState()->value, ['draft', 'canceled'])) {
    +          if ($order && !in_array($order->getState()->value, ['draft',
    +            'canceled',
    +          ])) {
                 $purchasedEntity = $item->getPurchasedEntity();

    Code lines are not required to be shorter than 81 characters. Line length and wrapping → , part of the Drupal coding standards, says:

    • Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 characters.
    • Control structure conditions may exceed 80 characters, if they are simple to read and understand.

    (Emphasis is mine.)

    In both the cases, the existing code is simpler to read and understand.

    +  /**
    +   * Constructs a new StockInventoryControlForm object.
    +   *
    +   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
    +   *   The entity type manager.
    +   */

    The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.

  • Status changed to Needs review about 1 year ago
  • 🇮🇳India arpitk

    Hi here is the updated patch addressing #10. Please review.

    Thanks!

  • Status changed to Needs work about 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    -    if(e.keyCode==13 && ($(this).val() != '')) {
    +    if(e.keyCode == 13 && ($(this).val() != '')) {

    There must be a space between if and the opening parenthesis.

    +   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
    +   *   The config factory service.

    There is no need to say it is a service.

     /**
    - * Class StockInventoryConfigForm.
    + * Implementing Stock Inventory Config Form.
      */
    

    That description is still wrong for the reasons I reported in my previous comment.

    +  /**
    +   * The entity_type.manager service.
    +   *
    +   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
    +   *   The entity_type.manager service.
    +   */
    +  protected $entityTypeManager;
    

    See my previous comment for what needs to be changed.

    +  /**
    +   * Constructs a new StockInventoryControlForm object.
    +   *

    That has not been changed since my previous comment.

    +  /**
    +   * The config factory service.
    +   *
    +   * @var \Drupal\Core\Config\ConfigFactoryInterface
    +   */
    +  protected $configFactory;
    

    It is sufficient to say The config factory.

    +  /**
    +   * Constructs a new AvailabilityOrderProcessor object.
    +   *
    +   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
    +   *   The config factory service.
    +   */
    

    That needs to be changes as per my previous comment, which holds true for every constructor description where the class namespace is missing.

    Furthermore, the issue summary should always describe what the issue is trying to fix and, in the case of coding standards issues, show which command and arguments have been used and which report that command shown. In this way, project maintainers can verify the patch/MR fixes all the warnings/errors.

  • Status changed to Needs review about 1 year ago
  • 🇮🇳India sakthi_dev

    Created a patch based on comment #12. Please review.

  • 🇮🇳India Preethy_ray

    Hi,
    Above patch addressed the #12 comments, but there are still few errors and warnings found that need correction.

  • 🇮🇳India dev16.addweb

    Hi here is the updated patch addressing #14. Please review.

    Thanks!

  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Pipeline finished with Success
    19 days ago
    Total: 181s
    #190936
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Pipeline finished with Success
    19 days ago
    Total: 144s
    #190952
Production build 0.69.0 2024