phpstan/phpstan-deprecation-rules

Allow ignoring "@group legacy" annotated test methods when Symfony PHPUnit bridge is in use

mondrake opened this issue · 12 comments

mondrake commented

It would be useful to allow skipping analysis of @legacy annotated PHPUnit test methods test in PHPUnit.

Drupal runs deprecation tests via PHPUnit and the Symfony's PHPUnit bridge. Current policy for runtime deprecated code is to write a @legacy annotated test that explicitly contains calls to deprecated code. PHPStan would now report these calls as errors, which is a duplicated check vs. existing tools. We cannot ignore entire files since it may well be that normal and deprecation tests are part of the same test class.

See also:

ondrejmirtes commented

This PHPDoc tag is specific to Drupal, it's very non-standard.

You have two options here:

  1. Switch @legacy to @deprecated - phpstan-deprecation-rules don't report deprecated calls from already deprecated scope.
  2. Baseline the current errors: https://phpstan.org/user-guide/baseline
ondrejmirtes commented

Doesn't matter - it's still PHPUnit/Symfony specific.

alexpott commented

@ondrejmirtes is there anyway that you'd consider making this somehow configurable? Like maybe a solution would be to allow projects to configure which annotations mean deprecated so projects that use make use of @legacy could make phpstan-deprecation-rules consider that the same as @deprecated.

alexpott commented

The suggestion:

allow projects to configure which annotations mean deprecated
does not work :( it's not @legacy it's @group legacy (this was pointed out to me by @catch56)

So maybe this is a won't fix and our only choice is to mark the tests @deprecated too.

ondrejmirtes commented

I'd accept a PR for a new extension type - it can be an interface similar to this one: https://github.com/phpstan/phpstan-src/blob/master/src/Rules/Exceptions/ExceptionTypeResolver.php

It would allow users to have a custom logic called to decide if a scope is deprecated or not.

mglaman commented

Coming back to this:

So we'd need a new extension type for deprecations. Like DeprecationTypeResolver which has isDeprecated and this would allow adding support for checking the @legacy tag in phpDoc alongside the existing @deprecated tag check. Which would also help answer/solve #73.

And this all goes into phpstan-src, not here.

Edit: I read more, so the extension would go here. Would we add configuration to this extension then, allowing to specify additional deprecated tag strings?

deprecatedTags: listOf(string()),

Or something which received the type, so more thorough inspection could be done?

mglaman commented

@ondrejmirtes I know you're super busy chasing down some performance items in PHPStan, but I wanted to ping and see if you had more guidance on how you'd like to see the extension. I'll have work time available to add this.

I'd like to support @group legacy and a custom node visitor to flag nodes as being in a deprecated scope based on if/else for backward compatibility support.

bbrala commented

Still quite relevant this.

mglaman commented

I gave this a shot in #99. It's quite a bit of changes but adds scope resolvers to extend what is considered a deprecated scope.

github-actions commented

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.