PHPStan fails when @legacy tests call deprecated code

I suppose we could add that to the baseline short term, while we file and wait an issue upstream to prevent this being reported?

Am i misreading something, but that MR should be comitted to D9.4 which does not have PHPStan?

If reading summary correct here the point is to add deprecated code (in a test here?). This should be marked explicitly as phpstan-ignore then (or added to the baseline).

I think @catch was prepping the D10 commit that needs to go first anyway in Drupal’s workflow

I see. But then indeed PHPStan is 'correct' here. PHPStan expects when you deprecate code, you also remove usages of it. (Since using deprecated code is a level 0 notice). So in this case we add the exception by saying on that line, ignore it, or add it to the baseline (if the plan is to later remove, which is the case here, so i would say indeed add it to the baseline)

Actually if the deprecation is for D10, the D10 patch should just not have the legacy test, no?

#8, see #3217699-25: Convert select query extenders to backend-overrideable services

@catch this is not needed. Only here we're taking a bit of a 'shortcut' were we add a 'deprecated in D10' item in D10. (Which we do to be able to cherry-pick it to D9, but actually in D10 it should not be there, since it's deprecated in D10).

Normal flow would be we add a 'deprecated in D11', with deprecation test to D10 branch, which PHPStan will not flag untill we open the D11 branch, then phpstan will flag it to have it removed in D11 (which is correct).

In this specific case we have 2 options, add the usage of a deprecated function in D10 (and add it to the baseline to remove later), or only add it to D9.

In our current setup we only have deprecation rules which are included in the mglaman/phpstan-drupal package.
https://github.com/mglaman/phpstan-drupal/tree/main/src/Rules/Deprecations

Specific rule in the above context kicking in is StaticServiceDeprecatedServiceRule.php

Question is how we want phpstan to notice our deprecations. If we want all deprecations to be flagged (this might include deprecations for D11, which we might not want (yet)) we should add the https://github.com/phpstan/phpstan-deprecation-rules package.
To filter between what deprecations to flag (the ones for D10, but not the ones for D11) we should check how upgrade_status does this (if that does it)

If we don't want deprecations to be flagged we should disable the rules included in the phpstan-drupal package.

This MR disables the (partial) deprecation checking we have at the moment.
I've created #3263109: Use PHPStan for deprecation checking to further discuss how to handle deprecations with phpstan.

This looks great to me. We3 already have deprecated service testing via Symfony's PHPUnit bridge so this is duplicating and the only things it is discovering is correct usage in legacy tests.

I've had another think about this and I think we can just add a comment to the legacy test to disable the phpstan check for the next line. It means we'll still benefit from untested code that calls a deprecated service being found and it's not that much an onus to do.

Here's a new patch that does just this.

#19 is to me the best option ATM; may require some extra work to PHPStan-ignore calls in legacy tests when deprecating something but I think that's OK, PHPStan will tell you what.

Adding credit for @catch and @mondrake

great! happy to see that direction with comments has been taken

+++ b/core/tests/Drupal/FunctionalTests/Core/Container/ServiceDeprecationTest.php
@@ -34,7 +34,9 @@ class ServiceDeprecationTest extends BrowserTestBase {
+    // @phpstan-ignore-next-line
     \Drupal::service('deprecation_test.service');
+    // @phpstan-ignore-next-line
     \Drupal::service('deprecation_test.alias');

<3 I love it! It also makes it easy to grep for and find usages of ignores later on. Thanks @alexpott!

Looks good to me. For sure since we already now with the legacy tag, that those tests need to go in the next version.

See upstream https://github.com/phpstan/phpstan/issues/4452

core/modules/node/tests/src/Functional/NodeRevisionPermissionsTest.php had been deleted so easy to reroll.