Fix coding standards

There are three unused variables:

FILE: /var/www/html/web/modules/dev/feeds_ex/src/Feeds/Parser/QueryPathXmlParser.php
------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------
 128 | WARNING | Unused variable $parser.
------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/dev/feeds_ex/feeds_ex.theme.inc
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 29 | WARNING | Unused variable $source.
----------------------------------------------------------------------


FILE: /var/www/html/web/modules/dev/feeds_ex/tests/src/Unit/Feeds/Parser/HtmlParserTest.php
-------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------
 229 | WARNING | Unused variable $config.
-------------------------------------------------------------------------------------------

Since unused variables may be a sign of work in progress, I didn't do anything to them and defer to a maintainer.

The remaining issues:

FILE: /var/www/html/web/modules/dev/feeds_ex/src/Feeds/Parser/XmlParser.php
---------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
---------------------------------------------------------------------------
 307 | ERROR | Function libxml_disable_entity_loader() has been deprecated
 321 | ERROR | Function libxml_disable_entity_loader() has been deprecated
---------------------------------------------------------------------------

These deprecations seem like they would better be addressed in a separate issue.

FILE: /var/www/html/web/modules/dev/feeds_ex/src/Feeds/Parser/ParserBase.php
-----------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------
 245 | WARNING | Line exceeds 80 characters; contains 93 characters
 386 | WARNING | Only string literals should be passed to t() where possible
-----------------------------------------------------------------------------

Line 386:

      $this->getMessenger()->addMessage($this->t($error['message'], $error['variables']), $error['severity'] <= RfcLogLevel::ERROR ? 'error' : 'warning', FALSE);

I'm not sure why this is translated; it seems like error messages from the parser should just be displayed as is. But I am probably misunderstanding something :)

In any case, the MR fixes the majority of the coding standards issues with the exceptions stated above. So I'm setting to "Needs review" for feedback.

I think it would be a good to rebase this one and see what GitLab CI has to say about the coding standards after these changes.

And now I'm heading back to the kitchen. :)

Huh, Drupal.org doesn't test for unused variables. So according to GitLab CI, the only remaining issues are those in #4.

Hmm, now the unit tests are broken again. Since they are also broken in the PHPStan issue, it's probably not my fixes here that broke them. Maybe it's because the tests are now running on Drupal 10.2 instead of 9.5?

I'll try to look at this tomorrow.

Current status: Now that 📌 Remove deprecated libxml_disable_entity_loader() Postponed is its own issue, the only two remaining coding standards items to address are:

FILE: /var/www/html/web/modules/dev/feeds_ex/src/Feeds/Parser/ParserBase.php
-----------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------
 245 | WARNING | Line exceeds 80 characters; contains 93 characters
 386 | WARNING | Only string literals should be passed to t() where possible
-----------------------------------------------------------------------------

Line 245 is commented out code, so maybe it should just be left in place as a reminder?
For line 346, see my comment in #4.

I merged in the latest changes from 8.x-1.x into it the MR. Maybe that helps?

The tests broke because some tests methods that were disabled by adding an underscore now run. Perhaps it is a good idea to add markTestIncomplete() to these test methods instead. These tests are about things that I didn't understand when I starting porting Feeds Extensible Parsers from D7 to D8.

And ideally, it would be good to identify what the disabled tests were supposed to cover exactly and see if that would still make sense in the D8+ version.

Ok! With markTestIncomplete(), then we are back to the issues in #9.

I've just added phpcs:ignore lines to ignore the remaining phpcs issues. In some cases it is allowed to pass a PHP variable to t(). The core module 'Database Log' does it too. For example in dblog.admin.inc: t($type) and in DbLogController: $this->t($dblog->type).

The commented out code is kept for now, but in the comment above I added more information about why the commented out code is there. And what the next step is for possibly resolving it.

Merged the code!

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