Problem/Motivation

FILE: /var/www/html/drupal_cms/drupal9/web/modules/contrib/token_filter/js/plugins/tokenbrowser/plugin.js
---------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
---------------------------------------------------------------------------------------------------------
 18 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "TRUE" but found "true"
 28 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "FALSE" but found "false"
 29 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "FALSE" but found "false"
 30 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "TRUE" but found "true"
---------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------


FILE: /var/www/html/drupal_cms/drupal9/web/modules/contrib/token_filter/src/Plugin/CKEditorPlugin/TokenBrowser.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 14 ERRORS AND 1 WARNING AFFECTING 11 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
  37 | ERROR   | [ ] Missing member variable doc comment
  39 | ERROR   | [ ] Parameter $configuration is not described in comment
  39 | ERROR   | [ ] Parameter $plugin_id is not described in comment
  39 | ERROR   | [ ] Parameter $plugin_definition is not described in comment
  39 | ERROR   | [ ] Parameter $token_service is not described in comment
  42 | ERROR   | [ ] Missing parameter comment
  56 | ERROR   | [x] Missing function doc comment
  77 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  90 | ERROR   | [x] Trailing punctuation for @see references is not allowed.
 108 | ERROR   | [ ] The @see reference should not contain any additional text
 184 | ERROR   | [x] Line indented incorrectly; expected at least 8 spaces, found 6
 185 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 4
 185 | ERROR   | [x] Closing brace indented incorrectly; expected 6 spaces, found 4
 201 | ERROR   | [x] Expected 1 blank line after function; 0 found
 202 | ERROR   | [x] The closing brace for the class must have an empty line before it
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/drupal_cms/drupal9/web/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
---------------------------------------------------------------------------------------------------------------------------------------

Time: 118ms; Memory: 12MB
CommentFileSizeAuthor
#19 Coding-Standard-3317495-19.patch4.81 KBrohit rana
#16 Coding-Standard-3317495-16.patch4.81 KBrohit rana
#12 Coding-Standard-3317495-12.patch4.82 KBrohit rana
#9 Coding-Standard-3317495-9.patch4.75 KBrohit rana
#7 Coding-Standard-3317495-7.patch4.74 KBrohit rana
#2 Coding-Standard-3317495-2.patch4.88 KBrohit rana

Issue fork token_filter-3317495

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

Rohit Rana created an issue. See original summary.

rohit rana’s picture

rohit rana commented
StatusFileSize
new Coding-Standard-3317495-2.patch4.88 KB
rohit rana’s picture

rohit rana commented
Assigned: rohit rana » Unassigned
Status: Active » Needs review
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Coding Standard Issues » Fix what reported by phpcs
Version: 8.x-1.4 » 8.x-1.x-dev
Status: Needs review » Needs work 
   /**
    * {@inheritdoc}
    *
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.
+   * @param mixed $plugin_definition
+   *   The plugin implementation definition.
+   * @param Drupal\token\Token $token_service
+   *   The token service.
    * @param Drupal\Core\Access\CsrfTokenGenerator $csrf_token_service
+   *   The CSRF token manager service.
    */
   public function __construct(

{@inheritdoc} isn't used for class constructors, and it won't make sense, since the parameters are documented.

-   * @see TreeBuilderInterface::buildRenderable() for option definitions.
+   * @see TreeBuilderInterface
+   *   TreeBuilderInterface::buildRenderable() for option definitions.

The correct document is @see TreeBuilderInterface::buildRenderable(), but the class name must include the full namespace.

rohit rana’s picture

rohit rana commented

Ok , I will fix it.

rohit rana’s picture

rohit rana commented
Assigned: Unassigned » rohit rana
rohit rana’s picture

rohit rana commented
Assigned: rohit rana » Unassigned
Status: Needs work » Needs review
StatusFileSize
new Coding-Standard-3317495-7.patch4.74 KB

Please Review.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Fix what reported by phpcs » Fix the issues reported by phpcs
Status: Needs review » Needs work 
+   * @see TokenTreeController::outputTree()

The class name needs to be fully qualified and include its namespace, in the same way is done with @var Drupal\token\Token.

rohit rana’s picture

rohit rana commented
StatusFileSize
new Coding-Standard-3317495-9.patch4.75 KB

@apaderno Please Review.

rohit rana’s picture

rohit rana commented
Status: Needs work » Needs review
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work 
-   * @see TreeBuilderInterface::buildRenderable() for option definitions.
+   * @see \Drupal\token\TreeBuilderInterface

-   * @see TokenTreeController::outputTree().
+   * @see \Drupal\token\Controller\TokenTreeController

It should reference the method, not the class, like the original comment did.

   /**
-   * {@inheritdoc}
+   * Constructs an TokenBrowser object.

The class name must include its full namespace.

rohit rana’s picture

rohit rana commented
Status: Needs work » Needs review
StatusFileSize
new Coding-Standard-3317495-12.patch4.82 KB
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

The patch fixes what I reported.

darvanen’s picture

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

Just a couple of nits in the latest patch:

  1. +++ b/src/Plugin/CKEditorPlugin/TokenBrowser.php
@@ -34,25 +34,43 @@ class TokenBrowser extends CKEditorPluginBase implements ContainerFactoryPluginI
+  /**
+   * The token service.
+   *
+   * @var Drupal\token\Token
+   */
+
   protected $tokenService;

    Please remove the blank line below the doc block, it should not be there. 

  2. +++ b/src/Plugin/CKEditorPlugin/TokenBrowser.php
@@ -34,25 +34,43 @@ class TokenBrowser extends CKEditorPluginBase implements ContainerFactoryPluginI
+   * Constructs an Drupal\token_filter\Plugin\CKEditorPlugin\TokenBrowser object.

    Please use "a" rather than "an" here.

rohit rana’s picture

rohit rana commented
Assigned: Unassigned » rohit rana
rohit rana’s picture

rohit rana commented
StatusFileSize
new Coding-Standard-3317495-16.patch4.81 KB
rohit rana’s picture

rohit rana commented
Assigned: rohit rana » Unassigned
Status: Needs work » Needs review
darvanen’s picture

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

@Rohit Rana in future please provide an interdiff when updating a patch.

+++ b/src/Plugin/CKEditorPlugin/TokenBrowser.php
@@ -34,25 +34,42 @@ class TokenBrowser extends CKEditorPluginBase implements ContainerFactoryPluginI
+  /**
+   * The token service.
+   * @var Drupal\token\Token
+   */
+
   protected $tokenService;

There is still a blank line here.

rohit rana’s picture

rohit rana commented
Status: Needs work » Needs review
StatusFileSize
new Coding-Standard-3317495-19.patch4.81 KB

Next time, I'll make sure to provide interdiff.

Sonal Gyanani made their first commit to this issue’s fork.

Sonal Gyanani opened merge request !4

Sonal Gyanani’s picture

Sonal Gyanani commented
Status: Needs review » Reviewed & tested by the community

Patch #19 works fine, 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

Found an issue in the MR, setting to NW.

Sonal Gyanani’s picture

Sonal Gyanani commented
Status: Needs work » Needs review

Thanks @darvanen, actually maybe it is swapped because showing following error

54 | ERROR | [ ] Doc comment for parameter $csrf_token_service does
     |       |     not match actual variable name <undefined>

But I fixed it and again swapped it as it was originally.
Please review.

darvanen’s picture

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

Looks good, great work everyone, thank you. Merged.

Status: Fixed » Closed (fixed)

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