Problem/Motivation

Getting Following errors/warnings

FILE: .../contrib/token_filter/src/Plugin/CKEditorPlugin/TokenBrowser.php
----------------------------------------------------------------------
FOUND 20 ERRORS AND 2 WARNINGS AFFECTING 16 LINES
----------------------------------------------------------------------
7 | WARNING | [x] Unused use statement
61 | ERROR | [ ] Parameter $configuration is not described in
| | comment
61 | ERROR | [ ] Parameter $plugin_id is not described in comment
61 | ERROR | [ ] Parameter $plugin_definition is not described in
| | comment
61 | ERROR | [ ] Parameter $token_service is not described in
| | comment
61 | ERROR | [ ] Parameter $file_url_generator is not described
| | in comment
61 | ERROR | [ ] Parameter $module_extension_list is not
| | described in comment
64 | ERROR | [ ] Missing parameter comment
67 | ERROR | [x] Expected 1 space between type hint and argument
| | "$configuration"; 21 found
70 | ERROR | [x] Expected 1 space between type hint and argument
| | "$csrf_token_service"; 8 found
71 | ERROR | [x] Expected 1 space between type hint and argument
| | "$token_service"; 21 found
73 | ERROR | [x] Expected 1 space between type hint and argument
| | "$module_extension_list"; 7 found
82 | ERROR | [x] Missing function doc comment
105 | WARNING | [ ] t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait
| | and $this->t() instead
115 | ERROR | [ ] Return type "\Drupal\Core\Url The URL." must not
| | contain spaces
118 | ERROR | [x] Trailing punctuation for @see references is not
| | allowed.
136 | ERROR | [ ] The @see reference should not contain any
| | additional text
212 | ERROR | [x] Line indented incorrectly; expected at least 8
| | spaces, found 6
213 | ERROR | [x] Line indented incorrectly; expected 6 spaces,
| | found 4
213 | ERROR | [x] Closing brace indented incorrectly; expected 6
| | spaces, found 4
229 | ERROR | [x] Expected 1 blank line after function; 0 found
230 | ERROR | [x] The closing brace for the class must have an
| | empty line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...p/modules/contrib/token_filter/src/Plugin/Filter/TokenFilter.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
130 | ERROR | The array declaration extends to column 100 (the limit
| | is 80). The array content should be split up over
| | multiple lines
----------------------------------------------------------------------

FILE: /app/modules/contrib/token_filter/token_filter.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
----------------------------------------------------------------------

Time: 1.61 secs; Memory: 10MB

Steps to reproduce

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

Proposed resolution

Fix the errors and warnings.

CommentFileSizeAuthor
#7 No phpcs issues reported.png89.73 KBhardikpandya
#7 interdiff_4-7.txt1.26 KBhardikpandya
#7 3333841-7.patch5.41 KBhardikpandya
#4 3333841-4.patch4.43 KBprem suthar
#2 3333841-2.patch2.58 KBshivam-kumar

Issue fork token_filter-3333841

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:

About issue forks

Comments

shivam-kumar created an issue. See original summary.

shivam-kumar’s picture

shivam-kumar commented
Assigned: shivam-kumar » Unassigned
Status: Active » Needs review
StatusFileSize
new 3333841-2.patch2.58 KB

Fixed the issue's errors and warnings. Needs review.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Version: 2.0.0-rc1 » 2.x-dev
Status: Needs review » Needs work

Thanks for this attempt! However, the patch does not address all of the issues raised by phpcs.

On visual inspection of the patch I found this:

+++ b/token_filter/src/Plugin/CKEditorPlugin/TokenBrowser.php
@@ -79,6 +78,9 @@ public function __construct(
+  /**
+   *
+   */

Empty doc blocks will fail phpcs.

It looks like you have run phpcbf and submitted that as a patch, this is not enough to satisfy the requirements of this issue. Please continue to manually address all of the phpcs issues until you can run phpcs without it returning any results.

prem suthar’s picture

prem suthar commented
StatusFileSize
new 3333841-4.patch4.43 KB

Fix All Phpcs Errors plz Review it.

prem suthar’s picture

prem suthar commented
Status: Needs work » Needs review
darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Status: Needs review » Needs work

Thanks @Prem Suthar, you appear to have missed the requirements for token_filter.info.yml. See the issue summary for the full list of issues, and do run phpcs to check your work please.

Also please always provide an interdiff when working with patches :)

hardikpandya’s picture

hardikpandya commented
Status: Needs work » Needs review
Issue tags: -
StatusFileSize
new 3333841-7.patch5.41 KB
new interdiff_4-7.txt1.26 KB
new No phpcs issues reported.png89.73 KB

The issues mentioned in the description related to info.yml file are the details drupal.org adds and you won't see them when you git clone a project.

I found one phpcs issue related to using t() in class and I have fixed that. Added interdiff and updated patch files.

All phpcs issues are fixed now.

Charchil Khandelwal made their first commit to this issue’s fork.

Charchil Khandelwal opened merge request !6

Charchil Khandelwal’s picture

Charchil Khandelwal commented
Status: Needs review » Reviewed & tested by the community

Created MR!
Thanks @hardikpandya, Patch #7 applied cleanly, and all errors and warnings are fixed.
So moving it to RTBC.
Thanks

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Status: Reviewed & tested by the community » Needs work

Made one comment on the MR, hiding patches now we're using MRs

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Assigned: Unassigned » darvanen

Can't seem to use the MR locally due to the branch name, I'm going to make a few edits and commit this so I can launch a full version.

  • dc9743bb committed on 2.x 
    Issue #3333841 by Charchil Khandelwal, hardikpandya, shivam-kumar, Prem...
darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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