Skip to content
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

Show deprecation text in the error message #2

Open
wenzhengjiang opened this issue Nov 15, 2018 · 11 comments

Comments

Projects
None yet
5 participants
@wenzhengjiang
Copy link

commented Nov 15, 2018

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

This comment has been minimized.

Copy link
Author

commented Nov 15, 2018

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

@ondrejmirtes

This comment has been minimized.

Copy link
Member

commented Nov 15, 2018

@joshuaspence

This comment has been minimized.

Copy link

commented Nov 15, 2018

+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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

@mglaman

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Member

commented Mar 9, 2019

@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

This comment has been minimized.

Copy link

commented Mar 11, 2019

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

@ondrejmirtes

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

This is now getting closer to being ready :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.