Fix the issues reported by phpcs

Created on 17 June 2022, about 2 years ago
Updated 19 June 2024, 9 days 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: ./editor_mailto_link.info.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 6 | WARNING | All dependencies must be prefixed with the project
   |         | name, for example "drupal:"
----------------------------------------------------------------------


FILE: ./editor_mailto_link.module
----------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
   1 | ERROR | [x] Missing file doc comment
   4 | ERROR | [x] Use statements should be sorted alphabetically.
     |       |     The first wrong one is
     |       |     Drupal\Component\Utility\UrlHelper.
  24 | ERROR | [x] Expected 1 space before "=>"; 0 found
  77 | ERROR | [x] Missing function doc comment
 104 | ERROR | [x] Expected 1 blank line after function; 2 found
 107 | ERROR | [x] Missing function doc comment
 117 | ERROR | [x] Expected 1 blank line after function; 2 found
 120 | ERROR | [x] Missing function doc comment
----------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

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

Needs review

Version

1.0

Component

Code

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia Rakhi Soni

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, ๐Ÿ‡ฎ๐Ÿ‡น
    +/**
    + * @file
    + * Contains editor_mailto_link.module.
    + */

    That is not much helpful, especially because it says the file contains itself.

    +/**
    + * Function to parse url field.
    + */

    It does not describe the parameters nor the return value.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • First commit to issue fork.
  • 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
    + * Primary module hooks for editor mailto link.
    + */
    +

    There is no need to say they are module hooks. Probably, it would be better to say Hook implementations for [module name]. The module name should use upper-case characters where they are used in the module name. (See the project name, which is shown on the breadcrumb of this page.)

    +/**
    + * Function to parse url field.
    + *
    + * @param string $href
    + *   The url href.
    + *
    + * @return array
    + *   The array of mailto link.

    url is not correctly spelled. The last line still does not make clear what the return value is. What is an array of mailto link?

    +/**
    + * Validation handler for editor_mailto_link_form_editor_link_dialog_alter().
    + */

    What that line should say is the is the class implementing the form, not the function that adds the validation handler.

    +/**
    + * Function to build query parameter.
    + */
     function _editor_mailto_link_build_mailto_link(array $mailto_values) {

    Parameters and return value are not documented. Also, it is useless to start a document comment for a function with Function to, which is not even what the Drupal coding standards says to use for those comments.

  • Assigned to imustakim
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad

    Working on this.

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad

    Updated the patch.
    Please review.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad
  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    + * @return array
    + *   Returns the array of links.

    Returns is not used in the line describing the return value.

    +/**
    + * The validation handler.
    + *
    + * @param array $form
    + *   The form.
    + * @param Drupal\Core\Form\FormStateInterface $form_state
    + *   The form state.
    + */

    The short description still does not say for which that function is a validation handler. Also, the parameters are not described for validation and submission handlers. API documentation and comment standards / Form-generating functions โ†’ has example code, although it still Drupal 7 code.

    /**
     * Form validation handler for user_login_form().
     *
     * @see user_login_form_submit()
     */
    function user_login_form_validate($form, &$form_state) {
      // โ€ฆ
    }
    
    /**
     * Form submission handler for user_login_form().
     *
     * @see user_login_form_validate()
     */
    function user_login_form_submit($form, &$form_state) {
      // โ€ฆ
    }
  • Assigned to imustakim
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad

    Working on this.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad

    Updated patch.
    Please review.

  • Issue was unassigned.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Mahima_Mathur23

    Patch #29 seems to have incorporated the raised changes by @apaderno in #27 except the 1st one.

  • Assigned to imustakim
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad

    Working on this.

  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad

    Patch updated.
    Please review.

  • Status changed to RTBC about 1 year ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines roberttabigue

    Hi @Rhaki,

    Confirmed fixed the PHPCS errors after applying patch #32 to the Editor Mailto Link against 1.0.x-dev and with Drupal core 9.5.6.

    Moving this now to RTBC.
    Kindly refer to the attached screenshots, please.

    Thank you.

  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    + * @param array $mailto_values
    + *   The array of mailto form values.

    The description is not much clear and it repeats the parameter type, which is already given from the @param line.

    + * @return mixed
    + *   Returns the query parameters.

    The return value description must not start with Returns.

  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sourabhjain

    Fixed the issues mentioned in #34

  • Status changed to RTBC about 1 year ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines roberttabigue

    Hi @sourabhjain,

    Confirmed no PHPCS errors shown after applying patch #35 to the Editor Mailto Link module 1.0.x-dev and with Drupal core 9.5.8.

    Moving this now to RTBC.
    Kindly refer to the attached screenshots, please.

  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    + * The URL field parser.
    + *
    + * @param string $href
    + *   The URL href.
    + *
    + * @return array
    + *   The type and href.
    + */
    +function _editor_mailto_link_parse_url_field(string $href) {

    The short description does not describe what the function does.
    The return value description does not correctly describes what the returned array contains.

    + * @param array $mailto_values
    + *   The mailto form values i.e. email, subject, body.

    It is preferable not to use i.e.

  • Status changed to Needs review 12 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sakthi_dev

    Please review.

  • Status changed to Needs work 12 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    +/**
    + * @file
    + * Hook implementations for Editor Mailto Link.
    + */
    +

    It is Hook implementations for the Editor Mailto Link module.

    +/**
    + * The URL field parser based on user values.
    + *

    The description does not say much about the function. bases on user values is not necessary, as most of the functions handle user input.

    + * @return array
    + *   The type of field and it's content.

    It would be its content.
    That is not sufficient to describe what the function returns, since the function returns an array with different array indexes basing on the link type.

    +/**
    + * Builds query parameters.
    + *
    + * @param array $mailto_values
    + *   The mailto form values such as email, subject, body.
    + *
    + * @return mixed
    + *   The query parameters.
    + */
     function _editor_mailto_link_build_mailto_link(array $mailto_values) {
    

    Truly, it build the mailto: link.
    $mailto_values is an array of values used to build the mailto: link. Whenever they come from a form is irrelevant for the function.
    The return value is the mailto: link, not query parameters. It is a string, not mixed values.

  • Status changed to Needs review 12 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sakthi_dev

    Please review.

  • Status changed to RTBC 12 months ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines roberttabigue

    Hi @sourabhjain,

    Confirmed no PHPCS errors were shown after applying patch #40.

    Checking patch editor_mailto_link.info.yml...
    Checking patch editor_mailto_link.module...
    Applied patch editor_mailto_link.info.yml cleanly.
    Applied patch editor_mailto_link.module cleanly.

    Please see the attached file for reference.

    Moving this now to RTBC.
    Thank you!

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

    The patch could apply, but that does not mean the changes are correct.

    +/**
    + * The URL field parser whether the field is mail or telephone.

    Grammatically, that does not make sense. As sentence, it has a subject (The URL field parser), but it doesn't have a verb. (whether is not a verb, but an adverb.)

    +/**
    + * Builds query parameters.
    + *
    + * @param array $mailto_values
    + *   The mailto values such as email, subject, body.
    + *
    + * @return string
    + *   The mailto: link.
    + */
     function _editor_mailto_link_build_mailto_link(array $mailto_values) {
    

    That is not the correct description, for a function that returns a link. (Query parameters and link are two different things.)

    + * @param array $mailto_values
    + *   The mailto values such as email, subject, body.

    Probably An array containing email address, subject, and body. is a better description.

  • Assigned to imustakim
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad
  • Issue was unassigned.
  • Status changed to Needs review 11 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad

    Patch updated and interdiff attached.
    Please review.

  • Status changed to RTBC 11 months ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines roberttabigue

    Hi,

    The Patch #44 was applied cleanly and no PHPCS errors were shown.

    Checking patch editor_mailto_link.info.yml...
    Checking patch editor_mailto_link.module...
    Applied patch editor_mailto_link.info.yml cleanly.
    Applied patch editor_mailto_link.module cleanly.

    Please see the attached file for reference.

    Moving this to RTBC.

    Thank you!

  • Status changed to Needs work 11 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    +/**
    + * The URL field parser checks whether the field is mail or telephone.
    + *
    + * @param string $href
    + *   The URL href.
    + *
    + * @return array
    + *   The type of field mail or telephone.
    + */
    +function _editor_mailto_link_parse_url_field(string $href) {

    The URL field parser ins the description is not necessary.
    That description is not correct, since the function returns the following value.

        return [
          'type' => 'mailto',
          'mailto' => [
            'email' => $matches[1],
            'body' => $output['body'] ?? NULL,
            'subject' => $output['subject'] ?? NULL,
          ],
        ];
    

    It cannot be said to check whether the field is mail or telephone because, in that case, it would return a Boolean value.

    The return value description is wrong too. It should also describe each of the array indexes the returned array contains.

    +/**
    + * Form validation handler for form_editor_link_dialog_alter().
    + */

    A form ID is either a string or a function name that does not end with alter(). That is probably an alter hook.

    +/**
    + * Builds query parameters.
    + *
    + * @param array $mailto_values
    + *   An array containing email address, subject, and body.
    + *
    + * @return string
    + *   The query parameter.
    + */
     function _editor_mailto_link_build_mailto_link(array $mailto_values) {
    

    The function build a query string, not query parameters.

  • Status changed to Needs review 11 months ago
  • Status changed to RTBC 11 months ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines roberttabigue

    Hi,

    Patch #47 was applied cleanly and no PHPCS errors were detected.

    Please see the attached file.

    Moving this to RTBC.

    Thank you!

  • Status changed to Needs work 11 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    +/**
    + * The URL field parser for creating array of required fields for mail.
    + *
    + * @param string $href
    + *   The URL href.
    + *
    + * @return array
    + *   Array of values with keys type and mailto.
    + */
    +function _editor_mailto_link_parse_url_field(string $href) {

    The description is still not correct. It is for a function, which cannot be described as The URL field parser. That is a description that could be used for a class or a class property, not for a function or a method.

    + * @return array
    + *   Array of values with keys type and mailto.

    The description is missing an indefinite article and an definite article.
    keys should be placed after type and mailto.
    I am not sure that description is correct, as the array contains more than just two keys.

    +/**
    + * Function for custom submit handler.
    + */
     function _editor_mailto_link_submit_handler(array &$form, FormStateInterface $form_state) {
    

    If that is a form submission handler, the description is not correct.
    Also, custom submit handler is vague; it is not necessary to say it is custom, which then could be said for any form submission handler added by modules that do not implement the form.

    +/**
    + * Builds query string.
    + *

    The description is missing an indefinite article.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    +/**
    + * The function for url field.
    + */
     function _editor_mailto_link_parse_url_field(?string $href) {
    +/**
    + * The function for mailto_link.
    + */
     function _editor_mailto_link_build_mailto_link(array $mailto_values) {

    The descriptions do not say what the functions do. The documentation comments do not describe the parameters nor the return values.

    +/**
    + * The function for link submit.
    + */
     function _editor_mailto_link_submit_handler(array &$form, FormStateInterface $form_state) {

    If that is a form submission handler, the description must be different.

  • First commit to issue fork.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Yashaswi18

    Ran phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml again, found that all issues are fixed and the feedback in #51 about the description is also already addressed.

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

    Also, if there is already an issue fork, please do not create patches. It avoids to start again from zero.

  • First commit to issue fork.
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Status changed to Needs review 5 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sakthi_dev

    Addressed the comment in MR. Please review.

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

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

Production build 0.69.0 2024