Problem/Motivation

Phpstan pipeline: https://git.drupalcode.org/project/soembed/-/jobs/2707120
Line src/Plugin/Filter/SoEmbedFilter.php
------ ----------------------------------------------------------------------
176 \Drupal calls should be avoided in classes, use dependency injection
instead
182 \Drupal calls should be avoided in classes, use dependency injection
instead
341 \Drupal calls should be avoided in classes, use dependency injection
instead
343 \Drupal calls should be avoided in classes, use dependency injection
instead

Proposed resolution

Use D.I to avoid these warnings or use @phpstan-ignore-next-line if Drupal calls are used on purpose.

CommentFileSizeAuthor
#13 soembed-MR7.patch2.83 KBloze
#3 finalss.png74.15 KBgirish_lnweb

Issue fork soembed-3473083

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

ankitv18 created an issue. See original summary.

girish_lnweb’s picture

girish_lnweb commented
Assigned: Unassigned » girish_lnweb

I will be working on it.

girish_lnweb’s picture

girish_lnweb commented
StatusFileSize
new finalss.png74.15 KB

@ankitv18 I have use same version of the module and trying to replicate the issue but I have not get any error as you mentioned above, I use this command to check for the errors
./vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml web/modules/custom/soembed/
and got some errors, mention in the attached screenshot.

Kindly give me the steps to replicate.

Thank you

rajdip_755’s picture

rajdip_755
Primary language Bengali
Location kolkata
commented

Hi @girish_lnweb, I have also run the phpcs but can't replicating the issue as you mentioned in #3 .

Along with this I have checked lines mentioned by @ankitv18 in the codebase of the module and the \Drupal is used in those lines which is not good as per the Drupal Coding Standard, we should use the DI for those lines.

Thanks !

ankitv18’s picture

ankitv18 commented
Assigned: girish_lnweb » ankitv18
Issue summary: View changes

ankitv18 opened merge request !6

ankitv18’s picture

ankitv18 commented
Assigned: ankitv18 » Unassigned
Status: Active » Needs review

  • ki committed 4002aa79 on 8.x-2.x authored by ankitv18 
    Resolve #3473083 "Avoid direct drupal calls"
ki’s picture

ki commented
Status: Needs review » Fixed
loze’s picture

loze commented
Status: Fixed » Needs work

this latest change gives me this error

Error: Call to a member function moduleExists() on null in Drupal\soembed\Plugin\Filter\SoEmbedFilter->getEnabledProviders()

loze opened merge request !7

loze’s picture

loze commented
Status: Needs work » Needs review

MR7 gets this working for me.

loze’s picture

loze commented
StatusFileSize
new soembed-MR7.patch2.83 KB

here is a patch for composer

  • ki committed e5129999 on 8.x-2.x authored by loze 
    Resolve #3473083 "Avoid direct drupal calls"
ki’s picture

ki commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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