Make some coding standards improvements [#2960098]

Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shows the following warnings/errors, which should be fixed.

Command:
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig modules/custom/crop/

Result:

FILE: crop/tests/src/Functional/CropFunctionalTest.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 13 WARNINGS AFFECTING 14 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
  10 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Tests\BrowserTestBase.
  84 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  85 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  88 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 101 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 109 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 110 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 122 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 123 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 142 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 147 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 148 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 152 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 153 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: crop/tests/src/Kernel/CropCRUDTest.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 16 WARNINGS AFFECTING 16 LINES
------------------------------------------------------------------------------------------------------------------------------------------
  68 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  69 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  70 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  75 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  76 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  77 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 103 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 104 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 105 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 106 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 109 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 110 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 111 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 112 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 113 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 114 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------


FILE: crop/tests/src/Kernel/CropUnitTestBase.php
---------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\KernelTests\KernelTestBase.
---------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------


FILE: crop/tests/src/Kernel/CropEffectTest.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------
 95 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------


FILE: crop/src/Entity/Crop.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------
 124 | WARNING | Exceptions should not be translated
 124 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------


FILE: crop/src/CropTypeListBuilder.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
------------------------------------------------------------------------------------------------------------------------------------------
  71 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 112 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 131 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------


FILE: crop/src/Form/CropTypeDeleteForm.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
 46 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 60 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------

Comment	File	Size	Author
#27	Create Article _ Drush Site-Install.png	144.7 KB	riddhi.addweb
#27	After patch apply 2960098.png	105.78 KB	riddhi.addweb
#16	crop-2960098-16.patch	20.82 KB	silvi.addweb
#9	2960098-9.patch	5.35 KB	neslee canil pinto
#6	2960098-6.patch	7.17 KB	neslee canil pinto
#3	interdiff-2960098-3.txt	11.81 KB	woprrr
#3	code_standard-2960098-3.patch	14.64 KB	woprrr
#2	code_standard-2960098-2.patch	4.29 KB	emartoni

Issue fork crop-2960098

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

2960098-fix-phpcs-code changes, plain diff MR !6
Check out this branch for the first time

Check out existing branch, if you already have it locally

2 hidden branches

2960098-fix-phpcs-errors

changes, plain diff MR !13

Check out this branch for the first time

Check out existing branch, if you already have it locally

2960098-gitlab-ci-reports

changes, plain diff MR !14

Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

11 April 2018 at 12:15

emartoni created an issue. See original summary.

Status	File	Size
new	code_standard-2960098-2.patch	4.29 KB

Comment #3

woprrr commented 28 September 2018 at 12:18

Status	File	Size
new	code_standard-2960098-3.patch	14.64 KB
new	interdiff-2960098-3.txt	11.81 KB

+++ b/tests/src/Kernel/CropEffectTest.php
@@ -84,11 +84,15 @@ class CropEffectTest extends CropUnitTestBase {
-    /** @var \Drupal\crop\CropInterface $crop */
+    /*
+     * @var \Drupal\crop\CropInterface $crop
+     */
...
-    /** @var $effect \Drupal\crop\Plugin\ImageEffect\CropEffect */
+    /*
+     * @var $effect \Drupal\crop\Plugin\ImageEffect\CropEffect
+     */

This part does stay in inline form.

And small miss for crop.setting.yml the file will have a newline at the end. By the way we can a small typo for testCropDimenssions() method.

All the rest are OK to me :) When review are OK we can flag this RTBC.

Thank you a lot for help and contribution !!!! You ROCK !

Comment #4

idebr commented 14 August 2019 at 20:09

Status:

Needs review

» Needs work

The patch in #3 contains changes from #2830768: Automated crop integration

Comment #5

woprrr commented 30 March 2020 at 08:57

Now it does be good #2830768: Automated crop integration was merged.

In all cases we need to reroll it now :D that was a long time ago ...

Status	File	Size
new	2960098-6.patch	7.17 KB

Comment #7

woprrr commented 30 March 2020 at 09:40

Status:

Needs review

» Needs work

Hi Neslee Canil Pinto,

I'm happy to see that super fast purpose !!!! :) Thank you great :) !

small feedback :

+++ b/src/Events/AutomaticCrop.php
@@ -45,8 +45,11 @@ class AutomaticCrop extends Event {
+   *   The image.

"The image that apply image style effect `crop`."

+++ b/src/Events/AutomaticCrop.php
@@ -45,8 +45,11 @@ class AutomaticCrop extends Event {
+   *   The crop type.

The crop type entity linked to this image.

+++ b/src/Events/AutomaticCrop.php
@@ -45,8 +45,11 @@ class AutomaticCrop extends Event {
+   *   The configuartion.

Small Typo here + We can describe more (in limit of 80 characters) what configuration is here.

Other small question why you delete @deprecated notices in your patch ? That important for backward compatibility and user that use 8.1.

Comment #8

neslee canil pinto

he/him

English

commented 30 March 2020 at 09:43

Assigned:

Unassigned

» neslee canil pinto

Comment #9

neslee canil pinto

he/him

English

commented 30 March 2020 at 09:53

Status:

Needs work

» Needs review

Status	File	Size
new	2960098-9.patch	5.35 KB

Hi woprrr,
I thought its deprecated, so will remove, i reverted it

Comment #10

11 January 2024 at 07:57

Nikolay Shapovalov made their first commit to this issue’s fork.

Comment #12

11 January 2024 at 08:38

Nikolay Shapovalov opened merge request !6

Status	File	Size
hidden	code_standard-2960098-2.patch	4.29 KB
hidden	code_standard-2960098-3.patch	14.64 KB
hidden	interdiff-2960098-3.txt	11.81 KB
hidden	2960098-6.patch	7.17 KB
hidden	2960098-9.patch	5.35 KB

Comment #15

nikolay shapovalov commented 11 January 2024 at 09:27

Issue summary:

View changes

Comment #16

silvi.addweb commented 11 June 2024 at 08:09

Status	File	Size
new	crop-2960098-16.patch	20.82 KB

Re-roll the patch according to the latest code changes.

Comment #17

nayana_mvr commented 12 July 2024 at 07:03

Tried to apply the patch #16 on Drupal version 10.4.x and Crop API module version 8.x-2.x but getting the following error:-

Checking patch crop.module...
error: while searching for:
 *
 * Provides storage and API for image crops.
 */
use Drupal\Component\Utility\UrlHelper;
use Drupal\Core\Form\FormStateInterface;
use Drupal\crop\Entity\Crop;
use Drupal\image\Entity\ImageStyle;
use Drupal\media\Entity\MediaType;
use Drupal\media\MediaTypeInterface;
use Drupal\media_entity\MediaBundleInterface;
use Drupal\file\FileInterface;
use Drupal\Core\Routing\RouteMatchInterface;

/**
 * Implements hook_theme().

error: patch failed: crop.module:6
error: crop.module: patch does not apply
Checking patch src/CropTypeListBuilder.php...
error: while searching for:

namespace Drupal\crop;

use Drupal\Core\Config\Entity\ConfigEntityListBuilder;
use Drupal\Core\Entity\Query\QueryFactory;
use Drupal\Core\Entity\EntityTypeInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\Routing\UrlGeneratorInterface;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Component\Utility\Xss;
use Drupal\image\Entity\ImageStyle;

/**
 * Defines a class to build a listing of crop type entities.

error: patch failed: src/CropTypeListBuilder.php:2
error: src/CropTypeListBuilder.php: patch does not apply
Checking patch src/Entity/Crop.php...
Hunk #1 succeeded at 121 (offset 8 lines).
error: while searching for:
    }

    if (isset($effects['crop_crop']) && $image_style->getEffects()
        ->has($effects['crop_crop']['uuid'])) {
      $type = $image_style->getEffect($effects['crop_crop']['uuid'])
        ->getConfiguration()['data']['crop_type'];
      $crop = self::findCrop($uri, $type);

error: patch failed: src/Entity/Crop.php:154
error: src/Entity/Crop.php: patch does not apply
Checking patch src/Form/CropTypeDeleteForm.php...
error: while searching for:
use Drupal\Core\Entity\EntityConfirmFormBase;
use Drupal\Core\Entity\Query\QueryFactory;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\StringTranslation\TranslationInterface;
use Drupal\Core\Url;
use Symfony\Component\DependencyInjection\ContainerInterface;

error: patch failed: src/Form/CropTypeDeleteForm.php:5
error: src/Form/CropTypeDeleteForm.php: patch does not apply
Checking patch src/Plugin/ImageEffect/CropEffect.php...
Hunk #2 succeeded at 30 (offset 4 lines).
error: while searching for:

    $form['crop_type'] = [
      '#type' => 'select',
      '#title' => t('Crop type'),
      '#default_value' => $this->configuration['crop_type'],
      '#options' => $options,
      '#description' => t('Crop type to be used for the image style.'),
    ];

    return $form;

error: patch failed: src/Plugin/ImageEffect/CropEffect.php:140
error: src/Plugin/ImageEffect/CropEffect.php: patch does not apply
Checking patch tests/src/Functional/CropFunctionalTest.php...
error: while searching for:
  protected function setUp() {
    parent::setUp();

    $this->adminUser = $this->drupalCreateUser(['administer crop types', 'administer image styles']);

    // Create test image style.
    $this->testStyle = $this->container->get('entity_type.manager')->getStorage('image_style')->create([

error: patch failed: tests/src/Functional/CropFunctionalTest.php:49
error: tests/src/Functional/CropFunctionalTest.php: patch does not apply
Checking patch tests/src/Functional/UpdatePathTest.php...
error: tests/src/Functional/UpdatePathTest.php: No such file or directory
Checking patch tests/src/Kernel/CropCRUDTest.php...
Checking patch tests/src/Kernel/CropEffectTest.php...
error: while searching for:
    $crop = $this->container->get('entity_type.manager')->getStorage('crop')->create($values);
    $crop->save();

    /** @var $effect \Drupal\crop\Plugin\ImageEffect\CropEffect */
    $effect = $this->imageEffectManager->createInstance('crop_crop', ['data' => ['crop_type' => $this->cropType->id()]]);
    $effect->transformDimensions($dimensions, $file_uri);

    $this->assertEquals($crop->size(), $dimensions, t('CropEffect::transformDimensions() transform image dimensions correctly.'));
  }

}

error: patch failed: tests/src/Kernel/CropEffectTest.php:88
error: tests/src/Kernel/CropEffectTest.php: patch does not apply

So I have created a new MR for fixing the phpcs issues. Please review.

Comment #18

12 July 2024 at 07:29

nayana_mvr opened merge request !13

Comment #19

12 July 2024 at 11:03

apaderno changed the visibility of the branch 2960098-fix-phpcs-code to hidden.

Comment #20

12 July 2024 at 11:12

apaderno opened merge request !14

Comment #21

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 12 July 2024 at 11:14

MR !13 fixes all the PHP_CodeSniffer errors/warnings.

Comment #22

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 12 July 2024 at 11:18

Actually, it seems the PHP_CodeSniffer errors/warnings have been already fixed.

Comment #24

18 July 2024 at 08:00

nilesh.addweb made their first commit to this issue’s fork.

Comment #25

nilesh.addweb commented 18 July 2024 at 08:07

Status:

Needs work

» Needs review

@roberttabigue,
There is two MR, MR !13 for phpcs and MR !14 for gitlab pipe line,
Please test now I have merge this two MR code in MR !13.

Comment #26

nilesh.addweb commented 18 July 2024 at 10:22

Hi,
I have fixed the PHPStan issues, and now the PHPStan job, along with all other jobs, has passed successfully. Please review the changes.

before --> https://git.drupalcode.org/issue/crop-2960098/-/pipelines/227578
after --> https://git.drupalcode.org/issue/crop-2960098/-/pipelines/227699

Comment #27

riddhi.addweb commented 18 July 2024 at 12:34

Status:

Needs review

» Reviewed & tested by the community

Status	File	Size
new	After patch apply 2960098.png	105.78 KB
new	Create Article _ Drush Site-Install.png	144.7 KB

I have applied the MR !13 and it resolves all the errors and tested module functionality with the image crop widget and focal point crop.
Please check the Screenshot for the same.

Comment #28

18 July 2024 at 14:14

apaderno changed the visibility of the branch 2960098-gitlab-ci-reports to hidden.

Comment #29

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 18 July 2024 at 14:15

This issue is only for fixing PHP_CodeSniffer reports, not all the GitLab CI reports.

Comment #30

ambot112 commented 22 October 2024 at 04:51

The Merge request !13 is great, it solves all the phpcs and phstan errors.

Comment #31

nikolay shapovalov commented 20 November 2024 at 11:03

Title:	Fix the issues reported by phpcs	» Add DrupalPractise phpcs check
Issue summary:	View changes
Status:	Reviewed & tested by the community	» Needs review
Related issues:		+#3447620: Declare compatibility with Drupal 11

Actually, it seems the PHP_CodeSniffer errors/warnings have been already fixed.

Yes @avpaderno, you are right. Right now there are no issues found by CI.

I found that a lot of changes that were implement at MR #6 were merged at #3447620: Declare compatibility with Drupal 11.
CI checks code using only Drupal rules set and DrupalPractice is ignored.
My suggestion to add DrupalPractice as well, using custom phpcs.xml.dist.

I update IS using check on latest 8.x-2.x-dev.
MR #6 is ready for review.

Status	File	Size
hidden	crop-2960098-16.patch	20.82 KB
hidden	After patch apply 2960098.png	105.78 KB
hidden	Create Article _ Drush Site-Install.png	144.7 KB

Comment #35

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 23 November 2024 at 08:20

Title:

Add DrupalPractise phpcs check

» Add DrupalPractice phpcs check

Comment #36

nikolay shapovalov commented 30 November 2024 at 16:28

Using FormattableMarkup at tests assert is bad practice, removed from all tests.
MR updated and ready for review.

Comment #37

drdam commented 5 December 2024 at 15:02

Status:

Needs review

» Reviewed & tested by the community

Ok for me

Comment #38

6 November 2025 at 17:30

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

Comment #39

phenaproxima

he/him

English

Massachusetts

commented 6 November 2025 at 17:31

Title:

Add DrupalPractice phpcs check

» Make some coding standards improvements

Comment #40

phenaproxima

he/him

English

Massachusetts

commented 6 November 2025 at 17:33

Status:

Reviewed & tested by the community

» Fixed

Auto-merged into 8.x-2.x. Thanks everyone!

Comment #41

6 November 2025 at 17:33

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Comment #42

6 November 2025 at 17:42

phenaproxima committed adab83d0 on 8.x-2.x authored by nikolay shapovalov

[#2960098] feat: Make some coding standards improvements

By: emartoni...

Comment #43

20 November 2025 at 17:43

Status:

Fixed

» Closed (fixed)

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

Assigned:	neslee canil pinto	» Unassigned
Category:	Bug report	» Task
Priority:	Normal	» Minor
Issue summary:	View changes

Title:	Fix PHPCS (code standard) erros	» Fix the issues reported by phpcs
Issue tags:		+Coding standards

Make some coding standards improvements

Issue fork crop-2960098

Comments

Related issues

Referenced by