Problem/Motivation

Getting following error/warnings.

FILE: /var/www/html/modules/contrib/printjs/printjs.module
-----------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------
1 | ERROR | [x] Missing file doc comment
6 | ERROR | [x] Namespaced classes/interfaces/traits should be referenced with use statements
-----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/printjs/printjs.info.yml
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/printjs/src/Printjs.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------
22 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/printjs/src/Plugin/Block/PrintJsBlock.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------
37 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------

Time: 2.53 secs; Memory: 6MB

Steps to reproduce

Run following command

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml modules/contrib/printjs/

Proposed resolution

Above errors/warnings need to be fixed.

CommentFileSizeAuthor
#2 3348615-2.patch4.71 KBsamitk

Issue fork printjs-3348615

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
new4.71 KB

Above errors/warnings has been fixed.

hardikpandya’s picture

The patch fixes reported phpcs issues. RTBC +1

avpaderno’s picture

Title: Drupal Coding Standards Issues | phpcs » Fix the issues reported by phpcs
Priority: Normal » Minor
Issue tags: -Coding standards Phpcs, -Phpcs Drupal coding standard issue
avpaderno’s picture

Status: Needs review » Needs work
+/**
+ * @file
+ * Contains printjs.module.
+ */
+

That is not the description for a .module file that is normally used. It does not even make sense, as it says that printjs.module contains printjs.module.

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

The namespace is missing.

+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.

Probably the article should be different.

kkalashnikov made their first commit to this issue’s fork.

kunalgautam’s picture

Status: Needs work » Needs review
avpaderno’s picture

Status: Needs review » Needs work
+/**
+ * @file
+ * Provides hooks and alters for printjs.
+ */

The usual comment is Hook implementations for [module name].

+
+/**
+ * Implements hook_help().
+ */
+function printjs_help($route_name, RouteMatchInterface $route_match) {
+  switch ($route_name) {
+    // Main module help for the printjs module.
+    case 'help.page.printjs':
+      $output = '<h3>' . t('About') . '</h3>';
+      $output .= '<p>' . t('Print content in with id use library
+      <a href="https://printjs.crabbly.com/">https://printjs.crabbly.com/</a>
+      you can define id which one you want to print, by default its id="print"
+      <div id="print"> Print me</div>
+      How to use:') . '</p>';
+      $output .= '<ul><li>' . t('You can create your content into div id="print" add Printjs block, it will show print button it will send content in id=print + css of page to printer') . '</li>';
+      $output .= '<li>' . t('You can add print button in header or footer of views') . '</li>';
+      $output .= '<li>' . t('You can customize your button print to print pdf, image, json ... like <a href="https://printjs.crabbly.com/ ">https://printjs.crabbly.com</a>with hook_preprocess_printjs') . '</li>';
+      $output .= '</ul>';
+      return $output;
+
+    default:
+      break;
+  }
+}

The phpcs report shown in the issue summary does not say hook_help() needs to be implemented. That is a change for a different issue.

+  /**
+   * Creates a new PrintJsBlock.
+   *
+   * @param array $configuration

The class name does not include its namespace.

kunalgautam’s picture

Status: Needs work » Needs review
avpaderno’s picture

Status: Needs review » Needs work
+/**
+ * @file
+ * Hook implementation for the printjs module.
+ */

The module name is not printjs. Do not get confused between module machine name and module name, since those are different.
Also, it is Hook implementations, since a module file generally contains more than a single hook implementation; even in the case there is just a hook, the plural is always used.

+  /**
+   * Creates a PrintJs Block.
+   *
+   * @param array $configuration
+   *   An associative array containing the plugin's configuration.
+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.
+   * @param string $plugin_definition
+   *   The plugin implementation definition.
+   * @param \Drupal\printjs\Printjs $print_js
+   *   The Print Js service.
+  /**
+   * The Printjs constructor.
+   *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+   *   The config factory.
+   * @param \Drupal\Core\StringTranslation\TranslationInterface $string_translation
+   *   Injected service.
+   */
+  public function __construct(ConfigFactoryInterface $configFactory, TranslationInterface $string_translation) {
+    $this->configFactory = $configFactory;

The class name and its namespace are missing from the short description.
The usual comment is "Constructs a new [class name] object."
A description like Injected service. is too generic; it would suit every service.

shalini_jha’s picture

Assigned: Unassigned » shalini_jha

I will work on this issue.

shalini_jha’s picture

Assigned: shalini_jha » Unassigned

shalini_jha’s picture

Status: Needs work » Needs review
avpaderno’s picture

Status: Needs review » Needs work
+/**
+ * @file
+ * Contains printjs.module.
+ */

That is not the documentation comment used for modules. See comment #12 for the correct documentation comment, which the previous MR was almost providing.

+/**
+ * Implements hook_help().
+ */
+function printjs_help($route_name, RouteMatchInterface $route_match) {
+  switch ($route_name) {
+    // Main module help for the printjs module.
+    case 'help.page.printjs':
+      $output = '<h3>' . t('About') . '</h3>';
+      $output .= '<p>' . t('Print content in with id use library
+      <a href="https://printjs.crabbly.com/">https://printjs.crabbly.com/</a>
+      you can define id which one you want to print, by default its id="print"
+      <div id="print"> Print me</div>
+      How to use:') . '</p>';
+      $output .= '<ul><li>' . t('You can create your content into div id="print" add Printjs block, it will show print button it will send content in id=print + css of page to printer') . '</li>';
+      $output .= '<li>' . t('You can add print button in header or footer of views') . '</li>';
+      $output .= '<li>' . t('You can customize your button print to print pdf, image, json ... like <a href="https://printjs.crabbly.com/ ">https://printjs.crabbly.com</a>with hook_preprocess_printjs') . '</li>';
+      $output .= '</ul>';
+      return $output;
+
+    default:
+      break;
+  }
+}
+

The report in the issue summary does not say that hook must be implemented. This issue is for fixed what reported by PHP_CodeSniffer; other changes are off-topic for this issue, except in the case of lines already changed for what PHP_CodeSniffer reports.

+  /**
+   * Constructs a new PrintJs object using the provided configuration and
+   * string translation services.   *

The class name does not include its namespace.
using the provided configuration and string translation services is not necessary and it is not used for constructors documentation comments.
There are trailing spaces and an extraneous asterisk.

+  /**
+   * The Printjs constructor.
+   *

The class namespace is missing.

  • lazzyvn committed 987b35e9 on master
    Issue #3348615 by kkalashnikov, shalini_jha, samit.310@gmail.com: Fix...
lazzyvn’s picture

Please patch on dev version i updated

avpaderno’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.