Address warnings from gitlab

Tested and verified patch #2 on drupal:10.1.x, the patch applied successfully but still getting some PHPCS errors after running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml

need to fix below mentioned errors, chaning it to needs work again.

FILE: /entity_print/src/Asset/AssetCollector.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 51 | WARNING | \Drupal calls should be avoided in classes, use dependency
    |         | injection instead
 52 | WARNING | The removal-version 'entity_print:3.0.0' does not match the
    |         | lower-case machine-name standard: drupal:n.n.n or
    |         | project:n.x-n.n
--------------------------------------------------------------------------------


FILE: entity_print/src/Asset/AssetRenderer.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 71 | WARNING | Variable $build inside unset call is undefined.
--------------------------------------------------------------------------

1. +++ b/src/PrintBuilder.php
   @@ -42,11 +49,14 @@ class PrintBuilder implements PrintBuilderInterface {
        $this->stringTranslation = $string_translation;
   +    $this->fileSystem = $file_system;
   

   We need a deprecation layer here, so default this to null, and trigger_error if the value is null, then fetch from the singleton

   See \Drupal\entity_print\Asset\AssetCollector::__construct for an example

   Will need a new change record to go with that, so you can use it in the message. The link to create a change record is at the top of this page

2. +++ b/tests/modules/entity_print_test/src/Plugin/EntityPrint/PrintEngine/TestPrintEngine.php
   @@ -65,8 +65,18 @@ class TestPrintEngine extends PrintEngineBase {
   +    $this->configuration['test_engine_setting'] = $form_state->getValue(
   +      [
   +        'testprintengine',
   +        'test_engine_setting',
   +      ]
   +    );
   +    $this->configuration['test_engine_suffix'] = $form_state->getValue(
   +      [
   +        'testprintengine',
   +        'test_engine_suffix',
   +      ]
   +    );
   
   +++ b/tests/modules/entity_print_test/src/Plugin/EntityPrint/PrintEngine/TestWordPrintEngine.php
   @@ -74,7 +74,12 @@ class TestWordPrintEngine extends PrintEngineBase {
   -    $this->configuration['test_word_setting'] = $form_state->getValue(['test_word_print_engine', 'test_word_setting']);
   +    $this->configuration['test_word_setting'] = $form_state->getValue(
   +      [
   +        'test_word_print_engine',
   +        'test_word_setting',
   +      ]
   +    );
   
   +++ b/tests/src/FunctionalJavascript/EntityPrintAdminTest.php
   @@ -101,7 +101,13 @@ class EntityPrintAdminTest extends WebDriverTestBase {
   -    $this->assertEquals(['test_engine_setting' => 'testvalue', 'test_engine_suffix' => 'overridden'], $config_entity->getSettings());
   +    $this->assertEquals(
   +      [
   +        'test_engine_setting' => 'testvalue',
   +        'test_engine_suffix' => 'overridden',
   +      ],
   +      $config_entity->getSettings()
   +    );
   

   can we collapse these a bit, the [] can go on the same lines as the ()

The items at #4 are false positives and need to be ignored by adding some ignore rules per https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignorin...

@larowlan #7 point 2 have done with added phpcs ignore and recreate patch. I checked for point 1 \Drupal\entity_print\Asset\AssetCollector::__construct

object is InfoParserInterface then @trigger_error for deprecation so in PrintBuilder class which object will check null for @trigger_error.

the new object (file system)

@larowlan could you please check the below code for #7 point 1

+   * @param \Drupal\Core\File\FileSystemInterface $file_system
+   *   The file system service.
    */
-  public function __construct(RendererFactoryInterface $renderer_factory, EventDispatcherInterface $event_dispatcher, TranslationInterface $string_translation) {
+  public function __construct(RendererFactoryInterface $renderer_factory, EventDispatcherInterface $event_dispatcher, TranslationInterface $string_translation, FileSystemInterface $file_system = NULL) {
     $this->rendererFactory = $renderer_factory;
     $this->dispatcher = $event_dispatcher;
     $this->stringTranslation = $string_translation;
+    if (!$file_system) {
+      @trigger_error('Calling $file_system without argument the constructor of ' . __CLASS__ . ' is deprecated in entity_print:8.x-2.4 and is removed from entity_print:3.0.0. Pass an instance of FileSystemInterface instead. See https://www.drupal.org/node/3238430', E_USER_DEPRECATED);
+      $this->fileSystem = \Drupal::service('file_system');
+    }
+    $this->fileSystem = $file_system;

Ah sorry, that was a bad example, because it was dealing with a replaced paramater.

Can you check \Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor::__construct in core for a better example

@larowlan I checked file core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php it isn't good example for comment #11

 /**
   * Constructs an AjaxResponseAttachmentsProcessor object.
   *
   * @param \Drupal\Core\Asset\AssetResolverInterface $asset_resolver
   *   An asset resolver.
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
   *   A config factory for retrieving required config objects.
   * @param \Drupal\Core\Asset\AssetCollectionRendererInterface $css_collection_renderer
   *   The CSS asset collection renderer.
   * @param \Drupal\Core\Asset\AssetCollectionRendererInterface $js_collection_renderer
   *   The JS asset collection renderer.
   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
   *   The request stack.
   * @param \Drupal\Core\Render\RendererInterface $renderer
   *   The renderer.
   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
   *   The module handler.
   */
  public function __construct(AssetResolverInterface $asset_resolver, ConfigFactoryInterface $config_factory, AssetCollectionRendererInterface $css_collection_renderer, AssetCollectionRendererInterface $js_collection_renderer, RequestStack $request_stack, RendererInterface $renderer, ModuleHandlerInterface $module_handler) {
    $this->assetResolver = $asset_resolver;
    $this->config = $config_factory->get('system.performance');
    $this->cssCollectionRenderer = $css_collection_renderer;
    $this->jsCollectionRenderer = $js_collection_renderer;
    $this->requestStack = $request_stack;
    $this->renderer = $renderer;
    $this->moduleHandler = $module_handler;
  }

Sorry, you'll need to check it in 10.1.x

@mukesh88 any updates here? Anything I can help with - feel free to ping me on slack if you need more info

@larowlan Thanks for response. Actually i am busy on project i don't spent much time on contribution if i will free or sure will ping you on slack

I will resolve all the reported phpcs issues.

removed all the reported issues with PHPCS, and created the MR for the same.
kindly review.

larowlan → closed merge request !38

@akshaydalvi212 that merge request contains lots of changes that aren't relevant to PHPCS.

I've closed it. We have a patch above, with actionable items. Let's work on top of this patch, or use it as the basis for an MR.

bharath-kondeti → made their first commit to this issue’s fork.

https://git.drupalcode.org/project/entity_print/-/merge_requests/39 Raised a MR with the fixes. Please review. Thanks

Tests are failing here

Hii,
patch applied cleanly for #26 but still there are some errors.

\entity_print\modules\entity_print_views\src\Controller\ViewPrintController.php
----------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------

\entity_print\src\Asset\AssetCollector.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\Controller\EntityPrintController.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\Entity\PrintEngineStorageInterface.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\EntityPrintPermissions.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\Event\PrintEventBase.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\EventSubscriber\PrintEngineExceptionSubscriber.php
-----------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
-----------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------

\entity_print\src\Form\SettingsForm.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\Plugin\EntityPrint\PrintEngine\DomPdf.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\Plugin\EntityPrintPluginManager.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\Plugin\PrintEngineInterface.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\PrintBuilder.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

\entity_print\src\Renderer\RendererBase.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

I have fixed this issue in above attached patch

The last submitted patch, 28: Fixed-the-issues-reported-by-phpcs-27.patch, failed testing. View results →

i am not able to apply patch #28 , so i have applied patch #26 and its applied.
after this patch i have found some more issues so i have added a patch for the fixes. please review.

Hi, I am not able to apply patch #30 it is throwing error.

sha@LAPTOP-6VFKUABC MINGW64 ~/entity_print (8.x-2.x)
$ git apply issues-3352346-Fixed-error-reported-by-phpcs.patch
error: patch failed: src/Asset/AssetCollector.php:5
error: src/Asset/AssetCollector.php: patch does not apply
error: patch failed: src/Plugin/EntityPrint/PrintEngine/DomPdf.php:2
error: src/Plugin/EntityPrint/PrintEngine/DomPdf.php: patch does not apply

So changing the status to Needs work.
Thankyou.

Hello, I applied the patch for the version 8.x-2.x, the patch applied succesfully.

Checking patch modules/entity_print_views/src/Controller/ViewPrintController.php...
Checking patch src/Asset/AssetCollector.php...
Checking patch src/Controller/EntityPrintController.php...
Checking patch src/Entity/PrintEngineStorageInterface.php...
Checking patch src/EntityPrintPermissions.php...
Checking patch src/Event/PrintEventBase.php...
Checking patch src/EventSubscriber/PrintEngineExceptionSubscriber.php...
Checking patch src/Form/SettingsForm.php...
Checking patch src/Plugin/EntityPrint/PrintEngine/DomPdf.php...
Checking patch src/Plugin/EntityPrintPluginManager.php...
Checking patch src/Plugin/PrintEngineInterface.php...
Checking patch src/PrintBuilder.php...
Checking patch src/Renderer/RendererBase.php...
Applied patch modules/entity_print_views/src/Controller/ViewPrintController.php cleanly.
Applied patch src/Asset/AssetCollector.php cleanly.
Applied patch src/Controller/EntityPrintController.php cleanly.
Applied patch src/Entity/PrintEngineStorageInterface.php cleanly.
Applied patch src/EntityPrintPermissions.php cleanly.
Applied patch src/Event/PrintEventBase.php cleanly.
Applied patch src/EventSubscriber/PrintEngineExceptionSubscriber.php cleanly.
Applied patch src/Form/SettingsForm.php cleanly.
Applied patch src/Plugin/EntityPrint/PrintEngine/DomPdf.php cleanly.
Applied patch src/Plugin/EntityPrintPluginManager.php cleanly.
Applied patch src/Plugin/PrintEngineInterface.php cleanly.
Applied patch src/PrintBuilder.php cleanly.
Applied patch src/Renderer/RendererBase.php cleanly.

Ran the command phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml entity_print/ after applying patch, there are no phpcs errors left.

If the maintainers want to keep this type of issue then probably should be rescoped to cover the warnings from gitlab.

Personal recommendation would be to close this out and start a new one since these type of tickets come off as farming.