Fix the issues reported by phpcs

Created on 21 March 2023, over 1 year ago
Updated 16 July 2024, 4 months ago

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

FILE: ./README.txt
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 45 | ERROR | [x] Expected 1 newline at end of file; 3 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: .logout_redirect.install
----------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
  7 | ERROR | [x] There must be exactly one blank line after the file
    |       |     comment
 10 | ERROR | [ ] Doc comment short description must be on a single
    |       |     line, further text should be a separate paragraph
 19 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./logout_redirect.libraries.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./logout_redirect.module
----------------------------------------------------------------------
FOUND 6 ERRORS AND 1 WARNING AFFECTING 6 LINES
----------------------------------------------------------------------
  1 | ERROR   | [x] The PHP open tag must be followed by exactly one
    |         |     blank line
  4 | WARNING | [ ] Line exceeds 80 characters; contains 95
    |         |     characters
  6 | ERROR   | [ ] Unnecessarily gendered language in a comment
  8 | ERROR   | [x] There must be exactly one blank line after the
    |         |     file comment
 14 | ERROR   | [x] Expected 1 blank line before function; 2 found
 17 | ERROR   | [x] Expected 1 space after IF keyword; 0 found
 17 | ERROR   | [x] Expected 1 space after closing parenthesis; found
    |         |     0
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./logout_redirect.routing.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | WARNING | The administration page callback should probably use
   |         | "administer site configuration" - which implies the
   |         | user can change something - rather than "access
   |         | administration pages" which is about viewing but not
   |         | changing configurations.
----------------------------------------------------------------------


FILE: ./src/Form/LogoutBackConfigForm.php
----------------------------------------------------------------------
FOUND 11 ERRORS AND 3 WARNINGS AFFECTING 13 LINES
----------------------------------------------------------------------
   3 | ERROR   | [x] Namespaced classes, interfaces and traits should
     |         |     not begin with a file doc comment
  12 | WARNING | [x] Unused use statement
  12 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Database\Database.
  48 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
  56 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
  66 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
  72 | ERROR   | [x] Short array syntax must be used to define arrays
  74 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
  75 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
  76 | ERROR   | [x] No space found before comment text; expected "//
     |         |     '#required' => TRUE," but found "//'#required'
     |         |     => TRUE,"
  80 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
  91 | WARNING | [ ] Possible useless method overriding detected
 102 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
 103 | ERROR   | [x] Object operator not indented correctly; expected
     |         |     6 spaces but found 8
----------------------------------------------------------------------
PHPCBF CAN FIX THE 11 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 85ms; Memory: 6MB
๐Ÿ“Œ Task
Status

RTBC

Version

2.0

Component

Code

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia rassoni Bangalore

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.

  • Needs reroll

    The patch will have to be re-rolled with new suggestions/changes described in the comments in the issue.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @rassoni
  • @rassoni opened merge request.
  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia rassoni Bangalore

    Fixed PHPCS issues and generate MR
    https://git.drupalcode.org/project/logout_redirect/-/merge_requests/1
    Please review.

  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    -Enter url on which you wants to redirect user after logging off and if 
    +Enter url on which you wants to redirect user after logging off and if

    Since that line is changed, the change should also correctly spell URL and use redirect users instead of redirect user.

     /**
      * @file
    - * This module After user logout if user click browser back button then redirect to login page.
    + * User click browser back button then redirect to login page.
      *
    - * Administrators can add url of page on which he want's to ,
    + * Administrators can add url of page on which wants to
      * redirect if browser back button is clicked after logout.
      */

    The usual comment is Hook implementations for [module name]. It does not need to explain the module purpose, which is already explained in its hook_help() implementation.

    -    // Markup for example
    +    // Markup for example.
         $form['explaination_text'] = [
           '#type' => 'markup',
           '#markup' => $explaination_text,

    That comment can be removed, since it does not explain anything already understandable from the code.

    +    $form['redirect'] = [
           '#type' => 'textfield',
    -      '#title' => $this->t("Add url of page on which you want's to  "
    -          . "redirect if browser back button is clicked after logout"),
    -      //'#required' => TRUE,
    +      '#title' => $this->t("Add url of page on which you want's to redirect if browser back button is clicked after logout"),
    +     // '#required' => TRUE,
           '#default_value' => $config->get('logout_redirect'),
    -    );
    +    ];

    Commented out code should be simply removed.

  • First commit to issue fork.
  • Merge request !23349252 Phpcs issues โ†’ (Open) created by kkalashnikov
  • 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, ๐Ÿ‡ฎ๐Ÿ‡น
     /**
      * @file
    - * This module After user logout if user click browser back button then redirect to login page.
    - *
    - * Administrators can add url of page on which he want's to ,
    - * redirect if browser back button is clicked after logout.
    + * Hook implementation for the logout_redirect module.
      */

    It is Hook implementations and the module name should be shown as shown on the top of this page.

    -    // Markup for explaination
    -    $explaination_text = "<div>If you log out of a Drupal site and then hit 
    -      the back button, you can see pages from the authenticated user's previous 
    +    // Markup for explanation.

    The last comment is not necessary, since that is already clear from the code.

    +      '#title' => $this->t("Add url of page on which you want's to
    +        redirect if browser back button is clicked after logout"),
           '#default_value' => $config->get('logout_redirect'),

    url is not the correct spelling of URL.
    you want's to, while used in the original comment, is not grammatical.
    redirect if browser back button is clicked after logout is missing an article, not using the possessive, and it would probably say after users logged out.

    -    // Submit button
    +    // Submit button.

    It should be The submission button, but that comment is not necessary, since that is evident from the code.

    +      // Set the submitted configuration setting.
    +      ->set('logout_redirect', $logout_redirect_url)
    +      ->save();

    That comment should be placed some lines earlier, but it is not necessary, as that is understandable from the code itself.

  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia kkalashnikov Ghaziabad, India

    @apaderno I have updated the MR accordingly please review it.

  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    -Enter url on which you wants to redirect user after logging off and if 
    +Enter URL on which you wants to redirect users after logging off and if
     user press browser back button.

    That sentence is still not grammatically correct. Those errors need to be corrected, since that comment is changed.
    A correct sentence would be:

    Enter the URL to which you want to redirect users after they log off or when they press the browser's Back button.

    + * Hook implementations for the logout_redirect module.

    The module name is not logout_redirect; that is its machine name. The module name is the one reported after Project: on the top of this page, which is the same name reported in the module's .info.yml file (the value for the name key).

    +      '#title' => $this->t("Add URL of page on which you want's to
    +        redirect if browser back button is clicked after logout"),
           '#default_value' => $config->get('logout_redirect'),

    you want's to is not grammatical. Since that line is edited, that should be corrected too.

  • 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, ๐Ÿ‡ฎ๐Ÿ‡น
    +Enter the URL of page on which user wants to redirect if browser back button is
    +clicked after logout.
    +

    That could be grammatically correct, but it is not correct. It's not the user who wants to redirect, but the administrator user who is entering the URL.

    - * Administrators can add url of page on which he want's to ,
    - * redirect if browser back button is clicked after logout.
    + * Hook implementations for the logout redirect module.
      */

    The module name should be spelled using uppercase letters where the module name uses them.

    -      '#title' => $this->t("Add url of page on which you want's to  "
    -          . "redirect if browser back button is clicked after logout"),
    -      //'#required' => TRUE,
    +      '#title' => $this->t("Add URL of page on which user wants to
    +        redirect if browser back button is clicked after logout"),
           '#default_value' => $config->get('logout_redirect'),

    It is the same mistake done for the previous change.

  • Status changed to Needs review 11 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Hardik_Patel_12 India

    I have solved the warnings given by PHPCS and here is the patch.

  • Status changed to Needs work 11 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
     /**
      * @file
    - * This module After user logout if user click browser back button then redirect to login page.
    + * This module After user logout if user click browser back button
    + *  then redirect to login page.
      *
    - * Administrators can add url of page on which he want's to ,
    - * redirect if browser back button is clicked after logout.
    + * Hook implementations for the logout redirect module.
      */

    The module name is Logout Redirect, not logout redirect. Capitalized letter must be written capitalized.
    Furthermore, the @file description for a module is just the last line.

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

    Furthermore, running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml ./, I get a slightly different report.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Yashaswi18

    I cloned and ran the command :
    phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml logout_redirect/
    I'm not seeing any errors except for this one.

    --------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    --------------------------------------------------------------------------------
     1 | ERROR | [x] End of line character is invalid; expected "\n" but found
       |       |     "\r\n"
    --------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ------------------------------------------------------------
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia chaitanyadessai Goa

    Please review patch.

  • Status changed to Needs work 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    -core: 8.x
     core_version_requirement: ^8 || ^9

    It is not necessary to remove the core line, if the core_version_requirement line is not changed.

    - * Rename wrong name of the config from logut_redirect_config.settings
    - * to logout_redirect.settings.
    + * To logout_redirect.settings.

    That change is not correct, since to logout_redirect.settings is not a sentence on its own. It is part of the previous sentence.

     /**
      * @file
    - * This module After user logout if user click browser back button then redirect to login page.
    - *
    - * Administrators can add url of page on which he want's to ,
    - * redirect if browser back button is clicked after logout.
    + * This module After user logout if user click browser back button.
      */
     
    +// Then redirect back to login page.
    +// Administrators can add page url's as per requirement.
    +// redirect if browser back button is clicked after logout.

    The usual description for a module file is Hook implementations for the [module name] module. where [module name] is the module name reported in its .info.yml file.

    Outside of functions or classes, the comment delimiter used is not //. We use a single documentation comment per function/method.

    +    // Validate if the redirect field is not empty.
    +    $redirect_value = $form_state->getValue('redirect');
    +    if (empty($redirect_value)) {
    +      $form_state->setErrorByName('redirect', $this->t('Redirect URL is required.'));
    +    }

    Verify the redirect field is not empty. is better.

    Also, let's keep using merge requests.

  • Assigned to nitin_lama
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India

    Addressed #18. Providing updated patch.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India

    Please review. Thanks.

  • Issue was unassigned.
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Yashaswi18

    Hello, applied MR!2, found one error remaining on running the command phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml

    FILE: /home/yashaswi/contribs/logout_redirect/logout_redirect.install
    ------------------------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ------------------------------------------------------------------------------------------------------------------
     11 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
    ------------------------------------------------------------------------------------------------------------------
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    apaderno โ†’ changed the visibility of the branch logout_redirect-3349252 to hidden.

  • Pipeline finished with Success
    8 months ago
    Total: 138s
    #119228
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia dev16.addweb

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

  • Pipeline finished with Success
    5 months ago
    Total: 138s
    #209725
  • Status changed to Needs work 4 months ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines cleavinjosh

    Hi @silvi.addweb,

    I tried to apply MR!2 but was skipped.

    โžœ  logout_redirect git:(main) โœ— curl https://git.drupalcode.org/project/logout_redirect/-/merge_requests/2.diff | git apply -v
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100 14587    0 14587    0     0  41157      0 --:--:-- --:--:-- --:--:-- 41206
    Skipped patch '.gitlab-ci.yml'.
    Skipped patch 'README.txt'.
    Skipped patch 'logout_redirect.info.yml'.
    Skipped patch 'logout_redirect.install'.
    Skipped patch 'logout_redirect.libraries.yml'.
    Skipped patch 'logout_redirect.module'.
    Skipped patch 'logout_redirect.routing.yml'.
    Skipped patch 'src/Form/LogoutBackConfigForm.php'.
    โžœ  logout_redirect git:(main) โœ—

    Please check and advise.
    Thank you.

  • Status changed to Needs review 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    The issue fork is eight commits ahead of the upstream repository. There is nothing that needs to be fixed in the MR.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Pipeline finished with Success
    4 months ago
    Total: 166s
    #219841
  • Status changed to RTBC 4 months ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines roberttabigue

    Hi,

    I have applied the latest MR to the Logout Redirect module against 2.0.1 on Drupal 10 and confirmed all PHPCS errors have been fixed.

    I ran this command on the module:
    phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml logout_redirect

    Please see the attached file for reference.

    I'm moving this now to โ€˜RTBCโ€™.

    Thank you!

Production build 0.71.5 2024