Skip to content

Show deprecation text in the error message #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wenzhengjiang opened this issue Nov 15, 2018 · 16 comments
Closed

Show deprecation text in the error message #2

wenzhengjiang opened this issue Nov 15, 2018 · 16 comments

Comments

@wenzhengjiang
Copy link

When we deprecate a function or method, we also document the replacement to use. For example,

/**
 * @deprecated Use bar instead
 */
function foo() {}

Currently when foo is used in the code, this rule only says foo is deprecated.

Call to deprecated function foo()

It would be very useful if it shows the deprecation text Use bar instead in the message as well.

Call to deprecated function foo(). Use bar instead.
@wenzhengjiang
Copy link
Author

I took a look at the code. It seems we need to change PHPStan\PhpDoc\PhpDocNodeResolver to let it store the deprecation text.

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Nov 15, 2018 via email

@joshuaspence
Copy link

+1, this would be very useful. It is documented as part of the FIG standards (https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc-tags.md#55-deprecated)

@GodLesZ
Copy link

GodLesZ commented Dec 13, 2018

+1 for display deprecation info.
It's a widly used practise to also write some notes about the deprecation itself.

Thanks @joshuaspence for pointing to PHPFIG.
According to them it's recommended to use @see in combination with @deprecated.

At least this should be added to the error message.

@mglaman
Copy link
Contributor

mglaman commented Mar 7, 2019

We're using this for the Drupal 9 clean up of deprecated code. Having the deprecation message available would be great. Often times the message says when a method will be removed. So you could ignore certain deprecations (ie not Symfony 4.4 but anything else)

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Mar 7, 2019 via email

@mglaman
Copy link
Contributor

mglaman commented Mar 7, 2019

Filtering out Drupal 10 versus a bunch of specific error messages would be much easier. It'd also be better DX to explain why it is deprecated and how to fix it. It also makes reporting easier.

Here's what I have setup to help triage deprecations for Drupal core using my phpstan-junit formatter.

Example report: https://deprecationbot.glamanate.com/job/core_aggregator/lastBuild/testReport/

It would be nice to include the deprecation message which includes how to fix the problem. This is really helpful at a code sprint.

PHPStan error:


Call to deprecated method url() of class Drupal. on line 36
--



Message:

   * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0.
   *   Instead create a \Drupal\Core\Url object directly, for example using
   *   Url::fromRoute().

@mglaman
Copy link
Contributor

mglaman commented Mar 8, 2019

@ondrejmirtes for what it is worth, I am totally willing to own and contribute this. I just want to have your 👍 before venturing down the road.

@ondrejmirtes
Copy link
Member

@mglaman Of course :) I need to give you a little bit of guidance on how to do it.

Check out PhpDocNodeResolver:.resolveIsDeprecated in phpstan/phpstan. It currently returns only boolean for whether the tag exists, but you should be able to inspect what $deprecatedTags actually contains. If it has description, you just need to rewrite the method similarly to resolveReturnTag or resolveThrowsTags. It it doesn't have description, we will have to modify phpstan/phpdoc-parser (but I won't go into that now).

You have to create DeprecatedTag class similar to ReturnTag or ThrowsTag.

ResolvedPhpDocBlock needs to be updated to contain not just boolean for isDeprecated, but the DeprecatedTag with the description. (isDeprecated needs to stay for backwards compatibility but you can add getDeprecatedTag).

DeprecatableReflection interface needs to have getDescription(): ?string added. Once you add it, you have to implement the method in all the interface implementations - usually by adding ?string $deprecatedDescription to the constructor.

Send a PR with this change to phpstan/phpstan. Once accepted, you can modify all the rules here in phpstan-deprecation-rules to output the description where available.

Let me know if you have any questions.

@mglaman
Copy link
Contributor

mglaman commented Mar 11, 2019

@ondrejmirtes thank you for the instructions! I will give this some time over the next week.

@ondrejmirtes
Copy link
Member

This is now getting closer to being ready :)

@mglaman
Copy link
Contributor

mglaman commented May 28, 2019

The parent PR has merged :) Now this can be worked

@ondrejmirtes
Copy link
Member

Already doing that :)

@mglaman
Copy link
Contributor

mglaman commented May 28, 2019

😮 beating me to the punch 😄

@ondrejmirtes
Copy link
Member

ondrejmirtes commented May 29, 2019

Implemented in 5685fe4, released as 0.11.2.

@github-actions
Copy link

github-actions bot commented May 1, 2021

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants