Problem/Motivation

There are many places where contrib could be invoking methods on objects whose type is unknown to static analysis. Per #3096959-29: Drupal 9 deprecated code report, an example of this (from the Crop API module) is:

$scheme = \Drupal::service('file_system')->uriScheme($uri);

Because the @return of \Drupal::service() is documented simply as "mixed", PHPStan does not know what class/interface uriScheme() is being called on, and therefore can't determine whether it's deprecated.

As a result, it does not report a deprecation error, when in fact there is one. Failing to report these means contrib modules that pass Upgrade Status will still fail to work on Drupal 9.

Proposed resolution

What if we commit a file to the codebase of Upgrade Status that contains an index of all of core's method names (across all classes), and for each method name, which interfaces/classes define it. Note that this information is already determined for generating api.drupal.org, so perhaps we can leverage whatever code generates that to also generate this index file?

Then, can we add code to the PHPStan process that detects when a method is being called on an object of unknown type, and for those cases, looks up the method from this index file to determine the possible classes it might be getting called on. And, if the method is deprecated in at least one of these classes, then flag it as a potential deprecation error.

While this could lead to false positives (if it's a method name that's deprecated in one interface/class but not in another), that's preferable to false negatives, and the false positive can be fixed by adding the correct typehint to the flagged code.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

effulgentsia created an issue. See original summary.

effulgentsia’s picture

effulgentsia’s picture

Berdir’s picture

I talked about it with @mglaman before, according to him phpstan is supposed to see this and understand what service it is, but then somehow it doesn't want to report it as a deprecation.

PS: False negative/the issue title seems like a complicated way to describe this :)

effulgentsia’s picture

Title: False negatives for deprecated method invocation on objects of unknown (to static analysis) type » Deprecated method invocations on non-type-hinted objects are not detected

False negative/the issue title seems like a complicated way to describe this

Ok, here's an attempt to simplify it.

mglaman’s picture

In phpstan-drupal we try to create a service map so that we can detect calls like this properly.

That is what ServiceDynamicReturnTypeExtension is for.

HOWEVER I just noticed this only runs on direct calls to the container, not against `\Drupal` and its `service` method. So we need a matching dynamic return type for that.

phpstan-drupal issue: https://github.com/mglaman/phpstan-drupal/issues/190

mglaman’s picture

Issue tags: +phpstan-drupal
mglaman’s picture

This may have been fixed in the latest release: https://github.com/mglaman/phpstan-drupal/releases/tag/0.12.14

It includes a fix for \Drupal::getContainer()->get() or $container->get(). So \Drupal::service calls should be detected.

Gábor Hojtsy’s picture

Status: Active » Postponed (maintainer needs more info)

In that case feedback would be great from folks after they do a composer update :)

mglaman’s picture

The fix for \Drupal::service calls has been added to phpstan-drupal:0.12.15, which should provide all the required coverage for detecting the access of deprecated services.

Gábor Hojtsy’s picture

Status: Postponed (maintainer needs more info) » Fixed

Thanks @mglaman. This should fix it for Upgrade Status once people do a composer update.

Gábor Hojtsy’s picture

Status: Fixed » Closed (fixed)

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