Problem/Motivation

Getting following error/warnings

FILE: /app/modules/contrib/workbench_email/src/EventSubscriber/ContentModerationStateChangedEvent.php
-----------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------
32 | ERROR | [x] Expected "string|false" but found "string|FALSE" for @var tag in member variable comment
-----------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/EventSubscriber/WorkbenchTransitionEventSubscriber.php
-----------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
57 | ERROR | Type hint "\Drupal\content_moderation\Event\ContentModerationStateChangedEvent" missing for $event
-----------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/Form/TemplateForm.php
------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 3 WARNINGS AFFECTING 5 LINES
------------------------------------------------------------------------------------------------------------------------
18 | WARNING | [x] Unused use statement
26 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
71 | ERROR | [ ] Parameter $module_handler is not described in comment
71 | ERROR | [ ] Parameter $messenger is not described in comment
83 | ERROR | [ ] Type hint "\Drupal\content_moderation\ModerationInformationInterface" missing for $moderation_info
285 | WARNING | [ ] Workflow::loadMultiple calls should be avoided in classes, use dependency injection instead
------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/Plugin/Derivative/WorkbenchEmailDeriver.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------
28 | ERROR | [x] Do not append variable name "$moderation_information" to the type declaration in a member variable comment
40 | ERROR | [ ] Type hint "\Drupal\content_moderation\ModerationInformationInterface" missing for $moderation_info
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/Plugin/QueueWorker/WorkbenchEmailProcessor.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------
126 | WARNING | [x] Inline @var declarations should use the /** */ delimiters
---------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/Plugin/RecipientType/EmailField.php
----------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------------------------------------------------------
69 | ERROR | [ ] Type hint "\Drupal\content_moderation\ModerationInformationInterface" missing for $moderation_info
141 | ERROR | [x] list(...) is forbidden, use [...] instead.
145 | ERROR | [x] list(...) is forbidden, use [...] instead.
168 | ERROR | [x] list(...) is forbidden, use [...] instead.
----------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/Plugin/RecipientType/FixedEmail.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AND 5 WARNINGS AFFECTING 6 LINES
----------------------------------------------------------------------------------
5 | WARNING | [x] Unused use statement
7 | WARNING | [x] Unused use statement
9 | WARNING | [x] Unused use statement
10 | WARNING | [x] Unused use statement
13 | WARNING | [x] Unused use statement
16 | ERROR | [x] Doc comment short description must end with a full stop
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/Plugin/RecipientType/RolesWithAccess.php
----------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 3 WARNINGS AFFECTING 7 LINES
----------------------------------------------------------------------------------------
6 | WARNING | [x] Unused use statement
8 | WARNING | [x] Unused use statement
9 | WARNING | [x] Unused use statement
10 | ERROR | [x] There must be one blank line after the last USE statement; 2 found;
24 | ERROR | [x] There must be exactly one newline after the class comment
53 | ERROR | [x] Expected 1 blank line after function; 0 found
54 | 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: /app/modules/contrib/workbench_email/src/Plugin/RecipientTypeBase.php
---------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------
38 | WARNING | Line exceeds 80 characters; contains 84 characters
---------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/Plugin/RecipientTypeInterface.php
-------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------
17 | WARNING | [x] 'TODO Docs.' should match the format '@todo Fix problem X here.'
-------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/RecipientTypePluginCollection.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
26 | WARNING | Possible useless method overriding detected
--------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/src/TemplateInterface.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
99 | WARNING | Line exceeds 80 characters; contains 104 characters
----------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/tests/src/Functional/WorkbenchEmailTest.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
------------------------------------------------------------------------------------------------------------------------------------------
302 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
336 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
348 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
370 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/tests/src/Kernel/ConfigDependenciesTest.php
--------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------
67 | WARNING | Unused variable $node_type.
--------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/tests/src/Kernel/RecipientTypePluginsTest.php
----------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------
85 | WARNING | Unused variable $node_type.
----------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/workbench_email.module
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 2 WARNINGS AFFECTING 5 LINES
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
73 | WARNING | [ ] Format should be "* Implements hook_foo().", "* Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "* Implements hook_foo_BAR_ID_bar() for
| | xyz-bar.html.twig.", "* Implements hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.", or "* Implements hook_foo_BAR_ID_bar() for block templates."
82 | WARNING | [ ] Format should be "* Implements hook_foo().", "* Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "* Implements hook_foo_BAR_ID_bar() for
| | xyz-bar.html.twig.", "* Implements hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.", or "* Implements hook_foo_BAR_ID_bar() for block templates."
100 | ERROR | [x] Additional blank lines found at end of doc comment
101 | ERROR | [ ] Expected type hint "EntityInterface"; found "ContentModerationState" for $entity
113 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/workbench_email/workbench_email.post_update.php
--------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------
41 | ERROR | [x] There should be no white space after an opening "{"
41 | ERROR | [x] There should be no white space before a closing "}"
41 | ERROR | [x] Closing brace must be on a line by itself
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

Time: 7.6 secs; Memory: 14MB

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/workbench_email/

Proposed resolution

Above error/warnings need to be fixed

Note:

I also removed public function &get($instance_id) function from
workbench_email/src/RecipientTypePluginCollection.php file as it is only called it's parent get function.

CommentFileSizeAuthor
#12 3328490-drupal-coding-standards-12.patch14.85 KBarpitk
#5 3328490-drupal-coding-standards-5.patch10.16 KBanoopsingh92
#2 3328490-2.patch19.74 KBsamitk

Issue fork workbench_email-3328490

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

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

samitk’s picture

samitk commented
Assigned: samitk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3328490-2.patch19.74 KB

Error/warnings are fixed.

mstrelan’s picture

mstrelan commented

Above error/warnings need to be fixed

Contrib modules are free to opt-in to their own coding standards. Nevertheless here's a review.

  1. +++ b/src/EventSubscriber/WorkbenchTransitionEventSubscriber.php
@@ -51,10 +51,10 @@ class WorkbenchTransitionEventSubscriber implements EventSubscriberInterface {
-  public function onContentModerationTransition($event) {
+  public function onContentModerationTransition(ContentModerationStateChangedEvent $event) {

    We may need to be careful to not break downstream modules extending this class, although I think that is unlikely. 

  2. +++ b/src/Form/TemplateForm.php
@@ -13,14 +13,13 @@ use Drupal\Core\Extension\ModuleHandlerInterface;
- * Class TemplateForm.
+ * The Class TemplateForm.

    🤷 

  3. +++ b/src/Form/TemplateForm.php
@@ -79,8 +78,12 @@ class TemplateForm extends EntityForm {
+   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+   *   The module handler.
+   * @param \Drupal\Core\Messenger\MessengerInterface $messenger
+   *   The messenger.

    I'd rather remove these annotations entirely. The parameters are already typed and the description gives no further information about them.

    Once we can drop PHP 7 support we can convert to constructor property promotion as well. 

  4. +++ b/src/Plugin/RecipientType/EmailField.php
@@ -66,7 +67,7 @@ class EmailField extends RecipientTypeBase implements ContainerFactoryPluginInte
-  public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, $moderation_info) {
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, ModerationInformationInterface $moderation_info) {

    As per previous comment on backwards compatibility. 

  5. +++ b/src/Plugin/RecipientTypeBase.php
@@ -35,7 +35,10 @@ abstract class RecipientTypeBase extends PluginBase implements RecipientTypeInte
-   * An associative array containing the configured settings of this recipient type.
+   * The settings var.
+   *
+   * An associative array containing the configured settings of this
+   * recipient type.

    How about reducing it to just "An associative array of configured settings for this recipient type". 

  6. +++ b/src/TemplateInterface.php
@@ -96,7 +96,10 @@ interface TemplateInterface extends ConfigEntityInterface, EntityWithPluginColle
+   * The recipientTypes function.

    This is superfluous. 

  7. +++ b/workbench_email.module
@@ -70,7 +70,9 @@ function _workbench_email_process_if_moderated(EntityInterface $entity) {
- * Implements hook_ENTITY_TYPE_insert for content_moderation_state.
+ * Implements hook_ENTITY_TYPE_insert().
+ *
+ * It is for content_moderation_state.

    Not sure what was wrong with this one. 

  8. +++ b/workbench_email.module
@@ -79,7 +81,9 @@ function workbench_email_content_moderation_state_insert(ContentModerationState
- * Implements hook_ENTITY_TYPE_update for content_moderation_state.
+ * Implements hook_ENTITY_TYPE_update().
+ *
+ * It is  for content_moderation_state.

    As above

anoopsingh92’s picture

anoopsingh92
Location Rajasthan, India
commented
Assigned: Unassigned » anoopsingh92
anoopsingh92’s picture

anoopsingh92
Location Rajasthan, India
commented
Assigned: anoopsingh92 » Unassigned
StatusFileSize
new 3328490-drupal-coding-standards-5.patch10.16 KB

Please review the patch.

anoopsingh92’s picture

anoopsingh92
Location Rajasthan, India
commented

I have made all the necessary changes manually, I didn't run phpcbf for these coding standards fixed.

gaurav-mathur’s picture

gaurav-mathur commented
Assigned: Unassigned » gaurav-mathur
gaurav-mathur’s picture

gaurav-mathur commented
Assigned: gaurav-mathur » Unassigned

I tested patch #5 and it applied cleanly for drupal 10.0.0-dev.
Patch did not work properly ,still seeing some errors/warnings in above files.
Please re-roll the patch.
Thank you.

anoopsingh92’s picture

anoopsingh92
Location Rajasthan, India
commented

@gaurav-mathur, Thanks for the review the patch.
I have made my changes as per the standard. Other issues are automatically fixed when you will run phpcbf and those issues will not be reflected in the patch also. those changes are only for /n and /r/n nothing else.

I have mentioned in #6 also I have made manually all these issues and didn't run phpcbf for fixing coding standard issues. If you are still seeing any issues please mention them in the comment.

Thank you.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Drupal Coding Standards Issues | phpcs » Fix the issues reported by phpcs
Version: 3.0.0 » 3.x-dev
Category: Bug report » Task
Priority: Normal » Minor
Status: Needs review » Needs work
Issue tags: -, - +Coding standards 
  /**
-   * An associative array containing the configured settings of this recipient type.
+   * An associative array containing the configured settings of this
+   * recipient type.
    *
    * @var array

Short descriptions must be only a line.

- * TODO Docs.
+ * @todo Docs.

What follows @todo is a sentence. Docs. does not say much about what needs to be done.

  /**
    * Test node type.
+   *
+   * @var nodeType
    */
   protected NodeTypeInterface $nodeType;

nodeType is not a valid PHP type and it cannot be used with @var.

I am not sure the fact phpcbf can fix some warnings/errors is a valid reason not to change those lines in the patch. Maintainers are not supposed to apply the patch and then run phpcbf; they are supposed to apply the patch, or merge the MR.

arpitk’s picture

arpitk commented
Assigned: Unassigned » arpitk

Updating patch with reference to #10

arpitk’s picture

arpitk commented
Status: Needs work » Needs review
StatusFileSize
new 3328490-drupal-coding-standards-12.patch14.85 KB

Tried the patch #5 it failed with 3.x-dev. However it works with 3.0.0. Updated the patch with reference from #10. Please review.

Thanks!

nikolay shapovalov’s picture

nikolay shapovalov commented
Assigned: arpitk » Unassigned

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

ankitv18 opened merge request !24

ankitv18’s picture

ankitv18 commented

PHPCS job is fixed : https://git.drupalcode.org/issue/workbench_email-3328490/-/jobs/1715887
Please review the MR!24

cleavinjosh’s picture

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

Hi @ankitv18,

I applied MR!24, it was applied smoothly and fixed all phpcs issues.

➜  workbench_email git:(3.x) curl https://git.drupalcode.org/project/workbench_email/-/merge_requests/24.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  6122    0  6122    0     0   9177      0 --:--:-- --:--:-- --:--:--  9178
patching file src/Form/TemplateForm.php
patching file src/Plugin/RecipientTypeBase.php
patching file src/RecipientTypePluginCollection.php
patching file src/TemplateInterface.php
patching file tests/src/Kernel/ConfigDependenciesTest.php
patching file tests/src/Kernel/RecipientTypePluginsTest.php
patching file workbench_email.module
➜  workbench_email git:(3.x) ✗ ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml workbench_email
➜  contrib git:(main) ✗

Thank you.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Thanks will look at this later in the week

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work

Thanks folks, left a review on the MR

ankitv18’s picture

ankitv18 commented
Assigned: Unassigned » ankitv18
ankitv18’s picture

ankitv18 commented

Will rebase with dev branch and check for further phpcs report & do a suggested fix.

  • larowlan committed 28166865 on 3.x authored by ankitv18 
    Issue #3328490 by ankitv18, anoopsingh92, larowlan, samit.310@gmail.com...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Fixed
ankitv18’s picture

ankitv18 commented
Assigned: ankitv18 » Unassigned

Status: Fixed » Closed (fixed)

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