Fix the issues reported by phpcs

Created on 18 May 2023, over 1 year ago
Updated 12 July 2024, 4 months ago
šŸ“Œ Task
Status

Fixed

Version

1.0

Component

Code

Created by

šŸ‡®šŸ‡³India urvashi_vora Madhya Pradesh, India

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

  • Issue created by @urvashi_vora
  • Status changed to Needs work over 1 year ago
  • šŸ‡®šŸ‡¹Italy apaderno Brescia, šŸ‡®šŸ‡¹
     /**
      * @file
    + * Module book_access.
      */

    The usual module description is Hook implementations for the [module name] module. where [module name] is replaced by the module name shown in the .info.yml file.

    -  array_combine($node->getOwnerId() == 0 ? BookAccess::defaultGrants() : \Drupal::config('book_access.settings')->get("book_access_default_author_access"), $node->getOwnerId() == 0 ? BookAccess::defaultGrants() : \Drupal::config('book_access.settings')->get("book_access_default_author_access"))
    +  array_combine($node->getOwnerId() == 0 ?
    +  BookAccess::defaultGrants() :
    +  \Drupal::config('book_access.settings')->
    +  get("book_access_default_author_access"),
    +  $node->getOwnerId() == 0 ?
    +  BookAccess::defaultGrants() :
    +  \Drupal::config('book_access.settings')->
    +  get("book_access_default_author_access"))
       );

    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.)

    -  // $roleDefaults = variable_get( "book_access_default_role_{$rid}_access", array());
    +  // $roleDefaults = variable_get(
    +  "book_access_default_role_{$rid}_access", array());

    That change is not correct for the same reason given for the previous change. Furthermore the "book_access_default_role_{$rid}_access", array()); part must be commented out too.

    +  /* if (isset($node->book['bid']) && $node->book['bid']) {
    +  // Check for existing permissions
    +  //    $rowCount = \Drupal::database()->
    +  // select('book_access_author', 'book_access_author')
    +  //      ->condition( 'nid', $node->book['bid'], '=')
    +  //      ->condition( 'uid', $node->getOwnerId())
    +  //      ->countQuery()
    +  //      ->execute()
    +  //      ->fetchField();
    +  if ($rowCount == 0) {
    +  // @FIXME
    +  // Could not extract the default value because it is either indeterminate, or
    +  // not scalar. You'll need to provide a default value in
    +  // config/install/book_access.settings.yml and
    +  // config/schema/book_access.schema.yml.
    +  // @FIXME
    +  // Could not extract the default value because it is either indeterminate, or
    +  // not scalar. You'll need to provide a default value in
    +  // config/install/book_access.settings.yml and
    +  // config/schema/book_access.schema.yml.
    +  // BookAccess::setAuthorGrants(
    +  //        $node->book['bid'],
    +  //        $node->getOwnerId(),
    +  //        array_combine($node->getOwnerId() == 0 ?
    +  // BookAccess::defaultGrants() :
    +  // \Drupal::config('book_access.settings')->
    +  // get("book_access_default_author_access"),
    +  // $node->getOwnerId() == 0 ? BookAccess::defaultGrants() :
    +  // \Drupal::config('book_access.settings')->
    +  // get("book_access_default_author_access"))
    +  //      );

    Commented out code must still be indented and formatted as per coding standards.

    +  $roles = user_roles();
    +  $rids = array_keys($roles);
    +  $roleResultSet = \Drupal::database()->
    +  // select('book_access_role', 'book_access_role')

    Why is the call to select() commented out?

    +  $roleResultSet = \Drupal::database()->
    +  // select('book_access_role', 'book_access_role')
    +  ->condition('nid', $node->book['bid'], '=')
    +  ->condition('rid', $rids, 'IN')
    +  ->fields('book_access_role', ['rid'])
    +  ->distinct()
    +  ->execute();

    Those lines are not correctly indented.

    +  // @FIXME
    +  // // @FIXME
    +  // // The correct configuration object could not be determined.
    +  // You'll need to
    +  // // rewrite this call manually.

    Avoiding lines exceed 80 characters does not mean avoiding they exceed 20 characters.

    +  // BookAccess::addRoleGrants( $node->book['bid'],
    +  // array($rid), $roleDefaults);.

    Periods are not appended to commented out code.

    + *   Form.
      * @param \Drupal\Core\Form\FormStateInterface $form_state
    + *   Form state.

    The descriptions are each missing a definite article.

    -  $items['node/%node/outline']['access callback'] = '_book_access_outline_access';
    +  $items['node/%node/outline']['access callback'] =
    +  'bookAccessOutlineAccess';
       }
    -  if ($user = \Drupal::entityTypeManager()->getStorage('user')->load($row->gid)) {
    +  if ($user = \Drupal::entityTypeManager()->
    +  getStorage('user')->load($row->gid)) {

    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.)

    + * The class BookAccess.
      */
    -
     final class BookAccess extends EntityAccessControlHandler implements EntityHandlerInterface

    Class descriptions must not start with The class or Class nor repeat the class name.

    +  /**
    +   * Creates an NodeViewController object.
    +   *
    +   * @param \Drupal\Core\Session\AccountInterface $current_user
    +   *   The current user.
    +   */
    +  public function __construct(
    +    AccountInterface $current_user = NULL
    +  ) {

    The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.
    The class name is also wrong, since that class is BookAccess.
    Method declarations must be written in a single line.

    -    drupal_write_record('book_access_author', $row, $bool ? array('nid', 'uid') : array());
    +    drupal_write_record('book_access_author', $row, $bool ? ['nid', 'uid'] : []);

    I am not sure it makes sense to make that change, since drupal_write_record() is not implemented in Drupal 9.5.

  • šŸ‡®šŸ‡³India urvashi_vora Madhya Pradesh, India

    Hi @apaderno, I have made the required changes as per your feedback.

    Please review.

    Thanks

  • Status changed to Needs review over 1 year ago
  • šŸ‡®šŸ‡³India urvashi_vora Madhya Pradesh, India
  • Status changed to Needs work 6 months ago
  • Hi @urvashi_vora,

    Applied your patch not-so successfully, might be the reason errors are still thrown. Please see below.

    book_access git:(1.0.x) curl https://www.drupal.org/files/issues/2023-05-19/coding-standard-fixes-3.patch | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100 45468  100 45468    0     0  94912      0 --:--:-- --:--:-- --:--:-- 96740
    patching file book_access.api.php
    patching file book_access.module
    patching file src/Access/BookAccess.php
    patching file src/Routing/RouteSubscriber.php
    patching file tests/src/Kernel/BookAccessDefaultsTest.php
    Hunk #1 succeeded at 98 (offset 6 lines).
    Hunk #2 succeeded at 114 (offset 7 lines).
    āžœ  book_access git:(1.0.x) āœ— cd ..
    āžœ  contrib git:(main) āœ— phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig book_access
    
    FILE: ...mo-site/drupal-orgissue/web/modules/contrib/book_access/book_access.module
    --------------------------------------------------------------------------------
    FOUND 1 ERROR AND 5 WARNINGS AFFECTING 6 LINES
    --------------------------------------------------------------------------------
       9 | ERROR   | [x] Use statements should be sorted alphabetically. The first
         |         |     wrong one is Drupal\Core\Session\AccountInterface.
      58 | WARNING | [ ] Line exceeds 80 characters; contains 85 characters
     109 | WARNING | [ ] Line exceeds 80 characters; contains 221 characters
     275 | WARNING | [ ] Line exceeds 80 characters; contains 91 characters
     375 | WARNING | [ ] Line exceeds 80 characters; contains 82 characters
     376 | WARNING | [ ] Line exceeds 80 characters; contains 84 characters
    --------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------
    
    
    FILE: .../drupal-orgissue/web/modules/contrib/book_access/src/Access/BookAccess.php
    --------------------------------------------------------------------------------
    FOUND 8 ERRORS AFFECTING 8 LINES
    --------------------------------------------------------------------------------
       6 | ERROR | [x] Use statements should be sorted alphabetically. The first
         |       |     wrong one is Drupal\Core\Entity\EntityAccessControlHandler.
      40 | ERROR | [x] Multi-line function declarations must have a trailing comma
         |       |     after the last parameter
     440 | ERROR | [x] The first parameter of a multi-line function declaration
         |       |     must be on the line after the opening bracket
     441 | ERROR | [x] Multi-line function declarations must have a trailing comma
         |       |     after the last parameter
     730 | ERROR | [x] The first parameter of a multi-line function declaration
         |       |     must be on the line after the opening bracket
     732 | ERROR | [x] Multi-line function declarations must have a trailing comma
         |       |     after the last parameter
     764 | ERROR | [x] The first parameter of a multi-line function declaration
         |       |     must be on the line after the opening bracket
     765 | ERROR | [x] Multi-line function declarations must have a trailing comma
         |       |     after the last parameter
    --------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------
    
    
    FILE: ...e/drupal-orgissue/web/modules/contrib/book_access/src/BookAccessHelper.php
    --------------------------------------------------------------------------------
    FOUND 4 ERRORS AFFECTING 2 LINES
    --------------------------------------------------------------------------------
     43 | ERROR | [x] The first parameter of a multi-line function declaration must
        |       |     be on the line after the opening bracket
     44 | ERROR | [x] Multi-line function declaration not indented correctly;
        |       |     expected 4 spaces but found 30
     44 | ERROR | [x] Multi-line function declarations must have a trailing comma
        |       |     after the last parameter
     44 | ERROR | [x] The closing parenthesis of a multi-line function declaration
        |       |     must be on a new line
    --------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------
    
    Time: 503ms; Memory: 14MB

    Kindly check.

    Thanks,
    Jake

  • First commit to issue fork.
  • Merge request !1Issue #3361257: fixed the phpcs. ā†’ (Closed) created by Unnamed author
  • Pipeline finished with Failed
    5 months ago
    Total: 140s
    #199198
  • šŸ‡®šŸ‡¹Italy apaderno Brescia, šŸ‡®šŸ‡¹
  • Status changed to Needs review 5 months ago
  • Pipeline finished with Failed
    5 months ago
    Total: 141s
    #200942
  • Pipeline finished with Failed
    5 months ago
    Total: 141s
    #200961
  • Pipeline finished with Failed
    5 months ago
    Total: 140s
    #201102
  • Pipeline finished with Failed
    5 months ago
    Total: 201s
    #201117
  • Pipeline finished with Failed
    5 months ago
    Total: 139s
    #201133
  • Status changed to Fixed 5 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024