I found in my project that I had a couple of t() statements that were not being detected by the DrupalPractice standard.

I did some digging and it turns out that the detection of global function calls currently only considers services, and ignores any classes (such as controllers and forms) that are not services but still have access to the container since they implement DependencyInjectionInterface.

Also, t() is actually a special case, since this is no longer reliant on injecting a service, but can be accessed in any class through the StringTranslationTrait, regardless of whether the class has access to the container or not.

CommentFileSizeAuthor
#10 Selection_161.png125.92 KBamitaibu

Comments

pfrenssen created an issue. See original summary.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Title: In addition to services, also check for global function calls in controllers and forms » In addition to services, also check for global function calls in classes that implement DependencyInjectionInterface
pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Active » Needs review

https://github.com/pfrenssen/coder/pull/46

  • klausi committed 6f6b654 on 8.x-3.x 
    fix(GlobalFunction): Detect use of global functions in classes that have...
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Fixed

Committed, thanks!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Sorry messed up the commit author, but at least I can give you credit here.

pfrenssen’s picture

pfrenssen
Location Sofia
commented

No problem about the commit credit, thanks for the quick action!

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

I'm not 100% sure if this is this PR , but suddenly I get this warning on my Drupal 7 (not 8)

t() calls should be avoided in classes, use
    |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
    |         | $this->t() instead
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

I think this code should be guarded by project version detection and not trigger on Drupal 7. Any chance you can show a bit of code that triggers the false positive?

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
StatusFileSize
new Selection_161.png125.92 KB

Sure, here's the code - https://gist.github.com/amitaibu/6ee81811090fb2007b7e9d689799be82 (class, and the info file of the module)

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Fixed » Active

Marking as active to check back when I have time.

  • klausi authored 38328fc on 8.x-3.x 
    fix(GlobalFunction): Fix Drupal 7 false positives (#3056538)
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Active » Fixed

Implemented a fix to exclude Drupal 7 and lower from this sniff.

You can also just disable this sniff for your Drupal 7 projects anyway because it does not make sense there.

Status: Fixed » Closed (fixed)

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