Problem/Motivation
Found this issues reported by phpcs:
FILE: /Users/studenttrainees/Drupal.org/drupalorg-site/docroot/modules/contrib/pdf/pdf.module
---------------------------------------------------------------------------------------------
FOUND 11 ERRORS AND 1 WARNING AFFECTING 8 LINES
---------------------------------------------------------------------------------------------
1 | ERROR | [x] Missing file doc comment
7 | ERROR | [x] Spaces must be used to indent lines; tabs are not allowed
7 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 1
8 | ERROR | [x] Spaces must be used to indent lines; tabs are not allowed
8 | ERROR | [x] Array indentation error, expected 3 spaces but found 2
9 | ERROR | [x] Spaces must be used to indent lines; tabs are not allowed
9 | ERROR | [x] Array indentation error, expected 4 spaces but found 3
10 | ERROR | [x] Spaces must be used to indent lines; tabs are not allowed
10 | WARNING | [x] A comma should follow the last multiline array item. Found: NULL
11 | ERROR | [x] Spaces must be used to indent lines; tabs are not allowed
12 | ERROR | [x] Spaces must be used to indent lines; tabs are not allowed
13 | ERROR | [x] Spaces must be used to indent lines; tabs are not allowed
---------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------
FILE: /Users/studenttrainees/Drupal.org/drupalorg-site/docroot/modules/contrib/pdf/css/pdf.css
----------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
2 | ERROR | [x] Expected 1 space after colon in style definition; 0 found
33 | ERROR | [x] Blank lines are not allowed in class definitions
----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------
FILE: /Users/studenttrainees/Drupal.org/drupalorg-site/docroot/modules/contrib/pdf/pdf.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: ...studenttrainees/Drupal.org/drupalorg-site/docroot/modules/contrib/pdf/src/Form/ConfigForm.php
---------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------
10 | WARNING | The class short comment should describe what the class does and not simply repeat
| | the class name
---------------------------------------------------------------------------------------------------
FILE: ...l.org/drupalorg-site/docroot/modules/contrib/pdf/src/Plugin/Field/FieldFormatter/PdfPages.php
---------------------------------------------------------------------------------------------------
FOUND 8 ERRORS AND 3 WARNINGS AFFECTING 11 LINES
---------------------------------------------------------------------------------------------------
8 | WARNING | [x] Unused use statement
10 | ERROR | [ ] Missing short description in doc comment
20 | ERROR | [x] Missing function doc comment
26 | ERROR | [x] Missing function doc comment
29 | WARNING | [ ] t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
35 | ERROR | [x] Missing function doc comment
42 | WARNING | [ ] t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
48 | ERROR | [x] Missing function doc comment
57 | ERROR | [x] No space found before comment text; expected "// '#value' => TODO," but found
| | "//'#value' => TODO,"
82 | ERROR | [x] Expected 1 blank line after function; 0 found
83 | ERROR | [x] The closing brace for the class must have an empty line before it
---------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------
FILE: ...g/drupalorg-site/docroot/modules/contrib/pdf/src/Plugin/Field/FieldFormatter/PdfThumbnail.php
---------------------------------------------------------------------------------------------------
FOUND 9 ERRORS AND 6 WARNINGS AFFECTING 14 LINES
---------------------------------------------------------------------------------------------------
8 | WARNING | [x] Unused use statement
10 | ERROR | [ ] Missing short description in doc comment
19 | ERROR | [x] Missing function doc comment
19 | ERROR | [x] Expected 1 blank line before function; 0 found
27 | ERROR | [x] Missing function doc comment
31 | WARNING | [ ] t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
38 | WARNING | [ ] t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
44 | WARNING | [ ] t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
49 | ERROR | [x] Missing function doc comment
59 | WARNING | [ ] t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
62 | WARNING | [x] A comma should follow the last multiline array item. Found: $height
68 | ERROR | [x] Missing function doc comment
76 | ERROR | [x] No space found before comment text; expected "// '#value' => ," but found
| | "//'#value' => ,"
103 | ERROR | [x] Expected 1 blank line after function; 0 found
104 | ERROR | [x] The closing brace for the class must have an empty line before it
---------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------
FILE: ...org/drupalorg-site/docroot/modules/contrib/pdf/src/Plugin/Field/FieldFormatter/PdfDefault.php
---------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 7 WARNINGS AFFECTING 4 LINES
---------------------------------------------------------------------------------------------------
10 | ERROR | Missing short description in doc comment
46 | WARNING | t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
139 | WARNING | t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
139 | WARNING | t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
139 | WARNING | t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
139 | WARNING | Do not concatenate strings to translatable strings, they should be part of the
| | t() argument and you should use placeholders
139 | WARNING | t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
153 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------------
Time: 277ms; Memory: 10MBSteps to reproduce
Run this command in the module folder: phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml
Proposed resolution
Remaining tasks
FILE: ...org/drupalorg-site/docroot/modules/contrib/pdf/src/Plugin/Field/FieldFormatter/PdfDefault.php
---------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------
156 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------------
Time: 214ms; Memory: 10MB
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3364326-12.patch | 21.75 KB | aimevp |
| #6 | 3364326-6.patch | 19.17 KB | pray_12 |
| #5 | After-patch.png | 101.05 KB | anjali mehta |
| #5 | before-patch.png | 178.29 KB | anjali mehta |
| #4 | phpcs-3364326-4.patch | 12.85 KB | arpitk |
Issue fork pdf-3364326
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
Comment #2
paraderojether commentedCreated a patch to fix the issues reported by phpcs except for the dependency injection.
Please review.
Thank You.
Comment #3
arpitk commentedworking on remaining issues.
Comment #4
arpitk commentedHere is the updated patch. Please review.
Thanks!
Comment #5
anjali mehta commentedHello @arpitk
Tested #3364326-4 patch on Drupal 9.5.9 and php version 8.1. Patch applied cleanly. But getting a warning and error on running the phpcs. Adding screenshot for Your references. Please have a look.
Thank You for your support.
Comment #6
pray_12 commentedHi,
Updated the patch. Please review.
Thank you!!
Comment #7
yashaswi18 commentedApplied the patch provided in #6.
The patch is applied successfully and ran the command phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml pdf/ again, no errors and warnings found.
Comment #8
silvi.addweb commentedI have tested this patch and it's working fine.
Comment #9
m.abdulqader commentedComment #11
m.abdulqader commentedComment #12
aimevpWhile working on implementing a patch from another issue I was surprised to see that only a small portion of the patch provided in #6 got merged to dev. Since I wanted to incorporate these changes on an active project I started working on a re-roll of that patch and then noticed the dependency injection fixes were done incorrectly (or maybe they were for D9 but are no longer for D10). So I combined all these changes + some code style improvements into a new patch which works and has been tested on a Drupal 10.3.6 install. However I'm not sure how this patch would behave on a Drupal 9.x install or if this requires a version bump of the pdf module then?
Comment #14
m.abdulqader commentedThank you @aimevp I merged the patch.
Comment #17
adam-vessey commentedIt does not appear that the PHPCS fixes here have made it into a tagged release: https://git.drupalcode.org/project/pdf/-/commit/b4d50d7a2e1198615b85808d...