Problem/Motivation

Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig gives the following warnings/errors.

FILE: ./quicklink_2.0.x/tests/quicklink_test/quicklink_test.info.yml
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
 7 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"

FILE: ./quicklink_2.0.x/quicklink.install
 8 | ERROR | [x] Line indented incorrectly; expected 0 spaces, found 1

FILE: ./quicklink_2.0.x/quicklink.module
 141 | ERROR | The array declaration extends to column 150 (the limit is 80). The array content should be split up over multiple
     |       | lines

FILE: ./quicklink_2.0.x/src/Form/QuicklinkConfigForm.php
  62 | ERROR   | [x] Missing function doc comment
  62 | ERROR   | [ ] Public method name "QuicklinkConfigForm::quicklink_config_form_validate" is not in lowerCamel format
  63 | ERROR   | [x] Short array syntax must be used to define arrays
  73 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
  73 | ERROR   | [x] Short array syntax must be used to define arrays
 206 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 207 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 212 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 213 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 223 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 224 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 231 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 232 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 233 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 240 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 241 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 242 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 248 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 314 | ERROR   | [x] Short array syntax must be used to define arrays
 324 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         |     $this->t() instead
 324 | ERROR   | [x] Short array syntax must be used to define arrays

FILE: ./quicklink_2.0.x/README.md
 76 | WARNING | Line exceeds 80 characters; contains 81 characters

Steps to reproduce

Proposed resolution

Those errors/warnings need to be fixed.

Remaining tasks

We can remove quicklink_config_form_validate function from src/Form/QuicklinkConfigForm.php file, looks like not used any where.

Issue fork quicklink-3329641

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

samit.310@gmail.com created an issue. See original summary.

samitk’s picture

Assigned: samitk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new7.76 KB

Error/warnings are fixed.

charchil khandelwal’s picture

Assigned: Unassigned » charchil khandelwal
Issue tags: -, -

I will review this.

charchil khandelwal’s picture

Assigned: charchil khandelwal » Unassigned

Thanks @samit.310@gmail.com, patch #2 applied cleanly, all errors and warnings are fixed.
And i also created MR for same.
Please review.

avpaderno’s picture

Title: Drupal Coding Standards Issues | phpcs » Fix the issues reported by phpcs
Category: Bug report » Task
Priority: Normal » Minor
Status: Needs review » Needs work
-`composer require oomphinc/composer-installers-extender npm-asset/quicklink:^2.0`
+`composer require oomphinc/composer-installers-extender
+ npm-asset/quicklink:^2.0`

Lines showing commands should not be split on two or more lines or be formatted to make clear the command is only one.

 /**
- * Class QuicklinkConfig.
+ * The Class QuicklinkConfig.
  */
 class QuicklinkConfigForm extends ConfigFormBase {

That description is still repeating the class name instead of describing what the class does.
Also, the report does not show any warning/error for that line, which means that either the report is not complete, or the patch is changing files that should not change. (I take it is the first, since Class QuicklinkConfig. is not describing what the class does.)

+  /**
+   * The quicklink_config_form_validate function.
+   * @codingStandardsIgnoreStart
+   */
   public function quicklink_config_form_validate($form, &$form_state) {

There is no need to use @codingStandardsIgnoreStart.

 dependencies:
-  - quicklink
+  - drupal:quicklink

The Quicklink module (this module) is not a Drupal core module.

avpaderno’s picture

Issue summary: View changes
nisha_j’s picture

StatusFileSize
new7.84 KB

bindu r made their first commit to this issue’s fork.

Anita Verma made their first commit to this issue’s fork.

anita verma’s picture

Status: Needs work » Needs review

Hello

Updated the suggested changes as per #6 .
Kindly review the changes.
Thankyou.

silvi.addweb’s picture

Status: Needs review » Needs work
StatusFileSize
new37.34 KB

Hello,
After checking, I discovered one more error.

apaderno changed the visibility of the branch 3329641-drupal-coding-standards to hidden.

avpaderno’s picture

It seems part of the changes done in this issue has been already committed. The only PHP_CodeSniffer error reported by GitLab CI is the following one.

FILE: ...9641/web/modules/custom/quicklink-3329641/src/Form/QuicklinkConfigForm.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 65 | ERROR | Public method name
    |       | "QuicklinkConfigForm::quicklink_config_form_validate" is not in
    |       | lowerCamel format
    |       | (Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps)
--------------------------------------------------------------------------------
Time: 237ms; Memory: 6MB

 
 

avpaderno’s picture

Truly, that method can be removed: It is not used, and it duplicates validateForm(). (Its code has not been fully converted to Drupal 8+ too.)

  /**
   * The validateForm method for the QuicklinkConfigForm.
   */
  public function quicklink_config_form_validate($form, &$form_state) {
    $parameterFieldsToValidate = [
      'total_request_limit',
      'concurrency_throttle_limit',
      'viewport_delay',
      'idle_wait_timeout',
    ];

    foreach ($parameterFieldsToValidate as $value) {
      $formValue = $form_state['values'][$value];
      if ($formValue !== '' && (!is_numeric($formValue) || intval($formValue) != $formValue || $formValue < 0)) {
        form_set_error($value, $this->t('%name must be a positive integer or zero.', [
          '%name' => $form['throttle_options'][$value]['#title'],
        ]));
      }
    }
  }
  /**
   * {@inheritdoc}
   */
  public function validateForm(array &$form, FormStateInterface $form_state) {
    $parameterFieldsToValidate = [
      'total_request_limit',
      'concurrency_throttle_limit',
      'viewport_delay',
      'idle_wait_timeout',
    ];

    foreach ($parameterFieldsToValidate as $value) {
      $formValue = $form_state->getValues()[$value];
      if ($formValue !== '' && (!is_numeric($formValue) || intval($formValue) != $formValue || $formValue < 0 || $formValue < '')) {
        $form_state->setErrorByName($value, $this->t('%name must be a positive integer or zero.', [
          '%name' => $form['throttle_options'][$value]['#title'],
        ]));
      }
    }
    parent::validateForm($form, $form_state);
  }
avpaderno’s picture

Status: Needs work » Needs review
clarkssquared’s picture

Hi

I applied the updated MR !9 and I can confirm that it fixes PHPCS issues

➜  quicklink git:(2.0.x) curl https://git.drupalcode.org/project/quicklink/-/merge_requests/9.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 10100    0 10100    0     0  15314      0 --:--:-- --:--:-- --:--:-- 15467
patching file .gitlab-ci.yml
patching file README.md
patching file quicklink.install
patching file quicklink.module
patching file 'src/Form/QuicklinkConfigForm.php'
patching file 'tests/quicklink_test/quicklink_test.info.yml'
➜  quicklink git:(2.0.x) ✗ ..
➜  contrib git:(master) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml quicklink 
➜  contrib git:(master) ✗ 

retaining the status to needs review for other's feedback