Problem/Motivation

There are quite PHPCS warnings and notices during the development of this module.

Steps to reproduce

/var/www/html/vendor/bin/phpcs --standard=Drupal,DrupalPractice /var/www/html/web/modules/contrib/symfony_mailer/

FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Address.php
-----------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\user\Entity\User.
-----------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Controller/SymfonyMailerController.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------------------------------------------
  81 | ERROR | The array declaration extends to column 145 (the limit is 80). The array content should be split up over multiple lines
 106 | ERROR | The array declaration extends to column 104 (the limit is 80). The array content should be split up over multiple lines
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Email.php
-------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------
 12 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Render\RendererInterface.
-------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Mailer.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------
 13 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Theme\ThemeInitializationInterface.
-----------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/MailerHelper.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------------------
 212 | ERROR | The array declaration extends to column 96 (the limit is 80). The array content should be split up over multiple lines
 213 | ERROR | The array declaration extends to column 88 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/AbsoluteUrlEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/BodyEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/DefaultsEmailAdjuster.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\Entity\MailerTransport.
--------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/HooksEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/PlainEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/PlainOnlyEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/PriorityEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/SkipSendingEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/SubjectEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/ThemeEmailAdjuster.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\EmailInterface.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailBuilder/CommerceOrderEmailBuilder.php
------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------
 11 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\Entity\MailerPolicy.
------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailBuilder/ContactPageEmailBuilder.php
--------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------
 81 | WARNING | [x] '@todo: This should also be moved to mailer policy with an import.' should match the format '@todo Fix problem X here.'
--------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailBuilder/SimplenewsNewsletterEmailBuilder.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\simplenews\Entity\Newsletter.
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Plugin/EmailBuilder/UpdateEmailBuilder.php
------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer\Entity\MailerPolicy.
------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Processor/EmailAdjusterManager.php
---------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Cache\CacheBackendInterface.
---------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/src/Processor/OverrideManager.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------------------------------------------
 335 | ERROR | The array declaration extends to column 118 (the limit is 80). The array content should be split up over multiple lines
 357 | ERROR | The array declaration extends to column 116 (the limit is 80). The array content should be split up over multiple lines
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/tests/src/Functional/OverrideTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
 44 | ERROR | The array declaration extends to column 167 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/tests/src/Functional/SymfonyMailerTestBase.php
------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\symfony_mailer_test\MailerTestTrait.
------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/symfony_mailer/tests/src/Kernel/SymfonyMailerKernelTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 22 | ERROR | The array declaration extends to column 99 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------

Proposed resolution

Fix all all PHPCS warnings and notices.

CommentFileSizeAuthor
#11 Screenshot from 2023-10-19 13-57-54.png237.03 KBchetan 11

Issue fork symfony_mailer-3394193

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

bohart created an issue. See original summary.

bohart opened merge request !70

adamps’s picture

adamps commented
Status: Active » Needs review
adamps’s picture

adamps commented
Status: Needs review » Needs work

Thanks.

This module has a phpcs.xml that excludes Drupal.Arrays.Array.LongLineDeclaration. In my view that warning is a mistake for 2023 when screens are large, and anyway other lines of code can be much longer than 80 characters.

bohart’s picture

bohart
Location Lutsk, Ukraine
commented
Status: Needs work » Needs review

The proposed MR fixes all available coding standards warnings and notices and removes custom `phpcs.xml`.
So, our IDE will not mark and try to fix all those lines according to the required standards.

Regarding Drupal.Arrays.Array.LongLineDeclaration. This is a standard for all Drupal core and all Drupal contrib modules.
There is no visible need to customize PHPCS for a single module. It is always better to have and follow the same standards everywhere.
Even more, it's just a few lines of code to fix.

Looking forward to being merged,
thanks!

adamps’s picture

adamps commented
Status: Needs review » Needs work

Many other contrib modules are customising the rules - just search for phpcs.xml on one of your sites.

I do want to help everyone get what they need from this module, but also my time is limited. You make more work for me as maintainer if you argue with absolutely everything😃.

bohart’s picture

bohart
Location Lutsk, Ukraine
commented
Status: Needs work » Needs review

There are at least three drupal-org issues related to the topic "Symfony Mailer and Drupal Core":
- #1803948: [META] Adopt the symfony mailer component
- #3380470: Migration strategy for moving to Symfony Mailer
- #3380476: Change core modules to use Symfony Mailer

If/when/once some parts of this module become a part of the core, it will be required to follow all PHPCS.
So, there will be a need to update those parts of the code to follow all PHPCS (as Drupal core does). It will not committed otherwise.
It's even simpler to do this right now (when we have just a few places to change) instead of changing PHPCS rules for the module (and the number of such places will be increased).

Adam, I would kindly ask you (as the maintainer) to rethink this policy about adding exceptions for some PHPCS rules to the module.
Thanks!

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Category: Bug report » Task
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Priority: Normal » Minor
Status: Needs review » Needs work

chetan 11 made their first commit to this issue’s fork.

chetan 11’s picture

chetan 11 commented
StatusFileSize
new Screenshot from 2023-10-19 13-57-54.png237.03 KB

Hi @bohart
I have fixed all the phpcs issues in symfony_mailer module.
Please check the raised MR & attached screenshot.

chetan 11 opened merge request !72

adamps’s picture

adamps commented
  • This coding standards messages for this module are shown at https://www.drupal.org/pift-ci-job/2787629. Currently there are 9.
  • This module has chosen not to fix long array declarations.
  • Sorting of use statements is not required by the official test run. I'm willing to do it as a one-off, but they will gradually become less sorted.
chetan 11’s picture

chetan 11 commented
Status: Needs work » Needs review
bohart’s picture

bohart
Location Lutsk, Ukraine
commented
Issue tags: +Coding standards
Parent issue: » #3393405: Replace outdated DrupalCI testing in favor to new GitLab CI pipelines

Hi Adam,

1) you mentioned https://www.drupal.org/pift-ci-job/2787629 in your comments on Gitlab.
DrupalCI is outdated; it used outdated `drupal/coder` and will have no updates anymore (as it's deprecated).
The valid link to see PHPCS notices/warnings is on GitlabCI (which uses the latest `drupal/coder` version):
https://git.drupalcode.org/project/symfony_mailer/-/jobs/185760

2) Okay, let's skip the discussion of long array declaration out of this issue. MR updated:
- the source branch rebased and added new commits from 1/x dev branch, so it's mergeable now;
- everything related to long array declaration changes are removed;

Please proceed and commit to the 1.x dev branch: MR !70.
Thanks!

  • AdamPS committed f977eb9f on 1.x authored by bohart 
    Issue #3394193: Fix all PHPCS warnings and notices
adamps’s picture

adamps commented
Status: Needs review » Needs work

Great thanks for the MR and for showing me about GitLab. There are still some CSS errors if anyone feels like fixing those.

bohart opened merge request !73

bohart’s picture

bohart
Location Lutsk, Ukraine
commented
Status: Needs work » Needs review

`stylelint` rules applied and `stylelint` job is now green.
Was: https://git.drupalcode.org/project/symfony_mailer/-/jobs/194358
Now: https://git.drupalcode.org/issue/symfony_mailer-3394193/-/jobs/195618

The only tricky thing was about @import 'inline.day.css'; as import can be a part of CSS files (not SCSS or other).
codingStandardsIgnoreLine doesn't work, so codingStandardsIgnoreStart/codingStandardsIgnoreEnd to make all jobs green:
https://git.drupalcode.org/issue/symfony_mailer-3394193/-/pipelines/34328

@Adam, please review MR !73 and merge. Thanks!

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

bohart’s picture

bohart
Location Lutsk, Ukraine
commented

@Yashaswi18, please do not commit to other users' branches / MR.

reverted/removed commit 6740e70e by @Yashaswi18 from MR !73, so MR can be reviewed/merged again.

  • AdamPS committed e2ebc729 on 1.x authored by bohart 
    Issue #3394193 by bohart, AdamPS: Fix all PHPCS warnings and notices
adamps’s picture

adamps commented
Status: Needs review » Fixed

Great thanks. It seems rather picky for it to complain about the order of lines. Anyway, it's good, the job is done.

bohart’s picture

bohart
Location Lutsk, Ukraine
commented

< just talking mode on >
When a lot of people contribute to some project, that's not picky but quite useful :)
Suppose IDE is appropriately configured (for example, PHPStorm is configured to handle this by default). In that case, all lines will be ordered automatically according to the specified stylelint rules in the project (in our case, they come from Drupal core).
That's why the latest MR was a really quick one; IDE is just fixing everything according to Drupal core stylelint by itself.
< just talking mode off >

Adam, MR !72 can be / should be closed (I have no rights to do so on this module). So, we will not collect a pull of non-needed and opened MRs on the project (which definitely will never be merged).
Thanks!

AdamPS closed merge request !72

adamps’s picture

adamps commented

Aha I didn't realise I could close them, thanks for pointing it out. I tidied up a few others too.

Status: Fixed » Closed (fixed)

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