mglaman / phpstan-drupal

Extension for PHPStan to allow analysis of Drupal code.
https://phpstan-drupal.mglaman.dev/
MIT License
195 stars 76 forks source link

PHPStan Drupal rules should emit messages consistent with the runtime deprecation message #468

Open goba opened 2 years ago

goba commented 2 years ago

Existing messages are missing information for humans (docs link) and processing code (upgrade_status, deprecation_status, Project Update Bot) alike.

Eg.

Missing explicit access check on entity query. (missing link, drupal version deprecated, removed, etc). Drupal​\​Tests​\​BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use. (missing drupal version deprecated, removed) Configuration entity must define aconfig_exportkey. See https://www.drupal.org/node/2481909 (missing drupal version deprecated, removed).

goba commented 2 years ago

The defaultTheme message in core was

Drupal\Tests\BrowserTestBase::$defaultTheme is required in drupal:9.0.0 when using an install profile that does not set a default theme. See https://www.drupal.org/node/2352949, which includes recommendations on which theme to use. per the patch in. https://www.drupal.org/project/drupal/issues/3082655, which is also not very standard, it needs to be updated from that too :D

goba commented 2 years ago

Again for the defaultTheme message, the link currently in phpstan-drupal is actually better than the one in the message from core (duh).

Also, I tried to dig up the config export change, a recent update to the message was https://git.drupalcode.org/project/drupal/-/commit/3003e5d81866e88093ed735f1dba1dfcf48fc811 but https://git.drupalcode.org/project/drupal/-/commit/16efa3bea6e7dd87111102dc71e2a6ed8395ff21 introduced it let's say. So https://www.drupal.org/project/drupal/issues/2986901 in 8.6.x, but that is a general schema check. I did not find where the config_export key itself started to be required.

goba commented 2 years ago

Ah found the config_export one in https://www.drupal.org/project/drupal/issues/2949021

Entity type "%s" is using config schema as a fallback for a missingconfig_exportdefinition is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. See https://www.drupal.org/node/2949023.', $this->id()

mglaman commented 1 year ago

This has to be a policy moving forward. As I have accidentally found out before, changing messages is a backward compatibility break as it breaks all baselines.

The action here should be creating a pull request template that has a series of to-dos, verifying the error message matches deprecation messages is one of them.