Use \Drupal consistently in tests

The IS mentions both Kernel and Functional tests should use \Drupal::service, but according to alexpott in #2699565-27: Replace \Drupal:: with $this->container->get() in test classes that allow dependency injection of Basic Auth module → this is only true for Functional tests.

I've created an MR with a phpstan rule to identify functional tests that access $this->container. See https://git.drupalcode.org/issue/drupal-2066993/-/jobs/4841841 for results. We could land this and add the existing violations to the baseline. Then we can open follow up issues to cleanup the baseline. I suspect it should be simple enough to write a rector rule to replace $this->container->get() with \Drupal::service().

Removing the original report here as it's just confusing as it was originally the opposite.

IIRC, using \Drupal::service() instead of $this->container->get() addresses issues that surfaced while trying to fix 📌 Memory leak in DrupalKernel when installing modules Active in any test that has a container rebuild. Some of the relevant discussion might've been in Slack, though.

Managed to fix 405 files and only had one fail. Have added a @phpstan-ignore to NodeAccessGrantsCacheContextTest for now. Changing from a Plan to a Task and setting Needs Review.

Some comments.

1. General topic: adding new PHPStan rules to Drupal core. As already found in 📌 Introduce a @return-type-will-be-added annotation to indicate return type addition in next major; enforce with a PHPStan rule Active , new Drupal-defined rules added to Drupal phpstan.neon will not be visible to custom/contrib unless they also add the rule to their projects' neon files. mglaman/phpstan-drupal in that sense is different because in that case all the rules are added as part of the extension installation via composer. Unless we do something, there's high risk that Drupal core own rules will not be checked. The existing rule is different because it only checks Drupal component code that is not present in modules/themes/etc.
2. As discussed in ✨ Add a trait for autowiring properties in tests Needs work , it seems to me that only preventing usage of $this->container->get() is not enough. I understand the problem is that any service should always be accessed from the singleton. Therefore we should also discourage contructs that store a service in a test property or a local variable like
   

     $cache = \Drupal::service('cache');
     ...
     $cache->get(...);
   

   no? That would be tricky.

3. The new PHPStan rule needs tests. Actually see how test coverage instruments in PHPStan rules testing are adding the red line in the MR where the rule is introduced.

Thanks @mondrake, some great points there. My initial thoughts on those:

1. I think automatically applying this rule everywhere in contrib and custom code would be quite disruptive, so it's almost a nice side effect that it would be opt-in. Ultimately I don't really care where it lives. It could probably even be a phpcs sniff that can be auto-fixed with phpcbf.

2. I think that could be handled separately in a follow up? Better to make progress than to get it perfect first go. Unless doing this makes things worse than the current state.

3. Absolutely. Keen to get more feedback first though.