Problem/Motivation

It would be useful to have a way to mark functions/classes/etc. as "deprecated". Two use cases (from Crell):

1) For intra-version devs (read, core devs) to know that the API they're about to use is slated for execution, so they should use the new one instead. Eg, if you're writing new arg()-using code for Drupal 8 right now, all you're doing is making more work for you or someone else to remove it soon after, *and* depriving us of an opportunity to see an arg() use case that we need to make sure is supported by whatever the new mechanism is (or decide that we want to consciously remove it, which is also legit).

2) To allow *gasp* some level of backward compatibility, by allowing one API to stick around for a version or so as a limited shell around some other new-fangled approach. Naturally this only sometimes works, but it is a way for us to, in some cases, help ease the transition by keeping around at least a few D7 utilities until D9, a few D8 methods until D10, etc. That makes module porting a bit easier. (Naturally this only works for cases that have for-reals APIs, not just naked data objects.)

Proposed resolution

---- For node/1354 (docs standards) ----

@deprecated [VERSION]
  [DESCRIPTION]

Use this tag to indicate that a function, method, or class has been deprecated and should not be used, but has not yet been removed.

A version SHOULD be specified, and should indicate the stable version in which this function was first deprecated (or, if added in a dev version, the next expected stable tag).

A prose description SHOULD be provided for additional context.  At minimum, it should indicate what new mechanism to use to achieve the previous functionality or note that the functionality is slated for outright removal.  An explanation of why the function was deprecated MAY be included.  If known, the description MAY specify the version in which the function is expected to be removed outright.

A @see directive to a direct replacement MAY be included as well.

--- Proposal for api.drupal.org ----
On api.drupal.org, we can use this to make a prominent section looking something like this (using a class "deprecated" on the div containing all of this so it can be styled with red background like on the php.net example cited above? http://php.net/manual/en/function.split.php)

Deprecated

Deprecated as of Drupal 7.3, will be removed in Drupal 8.0. Use other_function() if you need to do this action, and use another_function() if you need to do this other action.

---- End of proposal

Remaining tasks

1. Adopt standards.
2. Add standards to node/1354.
3. Patch API module.
4. Start using the tag in documentation.

User interface changes

api.drupal.org will display tagged functions with some prominent markup.

API changes

None.

Original report by tim.plunkett

In #1869566: Allow any collection of plugins to be lazily instantiated, the topic of using @deprecated came up once again.
We're introducing some methods that are only for backwards compatibility, and they shouldn't be used by new code.

Since we're not using it elsewhere, and I wasn't sure how api.d.o would deal with it, let's discuss that here.

Comments

Crell’s picture

I fully support using @deprecated to indicate either "this is going away by the end of this dev version but we haven't gotten rid of it yet, please don't use it" or "this will go away in the next major version, so don't get too used to it". Or both. :-)

dawehner’s picture

One big point is that there is a lot of support for @deprecated in all kind of different tools like IDE's
and it would probably also a good way for new contributors to show, hey here you can help drupal by fixing old code.

jhodgdon’s picture

api.drupal.org will not currently do anything with @deprecated tags, but as the maintainer of the API module, I can make it do whatever we think would be appropriate. So let's discuss it here.

If we adopt a standard, let's first see what other projects are doing:
- Where do they put the @deprecated tag
- Is there any other information on the line, or just the @deprecated tag?
- How is the information displayed on their API documentation site?

aspilicious’s picture

http://docs.oracle.com/javase/1.5.0/docs/guide/javadoc/deprecation/depre...
http://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmddeprecated

Lots of projects/languages use it. It can be used with or without a description but a description is advised because it can contain version number and suggestions. See this example http://php.net/manual/en/function.split.php

tim.plunkett’s picture

Symfony uses @deprecated:

From Symfony\Component\HttpFoundation\Session\Session:

    /** 
     * @return array
     *
     * @deprecated since 2.1, will be removed from 2.3
     */
    public function getFlashes()
    {   
        $all = $this->getBag($this->flashName)->all();

        $return = array();
        if ($all) {
            foreach ($all as $name => $array) {
                if (is_numeric(key($array))) {
                    $return[$name] = reset($array);
                } else {
                    $return[$name] = $array;
                }   
            }   
        }   

        return $return;
    }   

However, it's not displayed on their API site at all: http://api.symfony.com/2.1/Symfony/Component/HttpFoundation/Session/Sess...

dawehner’s picture

phpdoc also has some description when to use @deprecated: http://www.phpdoc.org/docs/latest/for-users/phpdoc/tags/deprecated.html

The @deprecated tag declares that the associated Structural elements will be removed in a future version as it has become obsolete or its usage is otherwise not recommended.

In additional they recommend to include a description for the reason of the deprecation.

jhodgdon’s picture

Status: Active » Needs review

OK. Here's a proposal:

----- For node/1354 (docs standards) ----

Indicating deprecated code

Use the @deprecated tag near the bottom of a documentation block (before the @see and @ingroup lines, but after everything else) to indicate that the item (function, class, etc.) has been deprecated (leave blank lines before and after the @deprecated tag).

Immediately after the tag (on the same line), describe which version the item was deprecated in, when it will be removed, and an explanation of reasons and/or what to use instead as appropriate. If you need more than one line, indent subsequent lines.

Example:

 *
 * @deprecated as of Drupal 7.3, will be removed in Drupal 8.0. Use other_function() if you need to do
 *   this action, and use another_function() if you need to do this other action.
 *

--- Proposal for api.drupal.org ----
On api.drupal.org, we can use this to make a prominent section looking something like this (using a class "deprecated" on the div containing all of this so it can be styled with red background like on the php.net example cited above? http://php.net/manual/en/function.split.php)

Deprecated

Deprecated as of Drupal 7.3, will be removed in Drupal 8.0. Use other_function() if you need to do this action, and use another_function() if you need to do this other action.

---- End of proposal

Thoughts?

tim.plunkett’s picture

describe which version the item was deprecated in, when it will be removed

This won't always be known, for example if it's only for backwards compatibility, and we can't know when everything will be converted. Can the wording make this less required-seeming?

Otherwise, that's perfect.

jhodgdon’s picture

Good idea. How about:

Immediately after the tag (on the same line), tell which version the item was deprecated in, when it will be removed (if known), and an explanation of reasons and/or what to use instead (as appropriate). If you need more than one line, indent subsequent lines.

dawehner’s picture

This sounds good for me!

Crell’s picture

+1 to #7/#9.

Note that many uses of @deprecated will be intra-version. Eg, I want to mark arg() as @deprecated now, on the expectation that it will be exterminated by the time D8 ships, even though it was never @deprecated in Drupal 7.

jhodgdon’s picture

In the case of #11, I would expect we would also go back to the 7.x docs and put the @deprecated there too, wouldn't we? That way at least going forward, there would be a record on api.drupal.org for D7 users of the function that it's expected to go away?

Crell’s picture

You mean retroactively going back to add @deprecated to Drupal X once it's been removed from (or slated for removal from) Drupal X+1? I am not sure about that. That would mean anyone using arg(), for instance, in Drupal 7 (which you still have to do in a lot of places, sadly), would have a @deprecated marker on their function, but no alternative because the alternatives are all Drupal 8 code.

I guess since we're just talking about Docblocks it wouldn't actually impact running code, but it seems like yet-more-work every time we close an issue, since then we need to file another follow-up issue against D7 in addition to the change notices we already need to write.

jhodgdon’s picture

OK, I'll drop that proposal. But without the intention of going back to previous vresions, it really seems useless to me to have @deprecated at all, if it's just early in the dev cycle for a new release to say "we intend to drop this before the full release". What's the point?

Crell’s picture

I see two uses for @deprecated.

1) For intra-version devs (read, core devs) to know that the API they're about to use is slated for execution, so they should use the new one instead. Eg, if you're writing new arg()-using code for Drupal 8 right now, all you're doing is making more work for you or someone else to remove it soon after, *and* depriving us of an opportunity to see an arg() use case that we need to make sure is supported by whatever the new mechanism is (or decide that we want to consciously remove it, which is also legit).

2) To allow *gasp* some level of backward compatibility, by allowing one API to stick around for a version or so as a limited shell around some other new-fangled approach. Naturally this only sometimes works, but it is a way for us to, in some cases, help ease the transition by keeping around at least a few D7 utilities until D9, a few D8 methods until D10, etc. That makes module porting a bit easier. (Naturally this only works for cases that have for-reals APIs, not just naked data objects.)

When we want to do which of those is a case-by-case question, but both are still valuable.

jhodgdon’s picture

Issue tags: +Coding standards

OK. I've just created an issue summary, with Crell's use cases from #15 and my concrete proposal from #7/#9.

All in favor? opposed? I think we have a few +1s already... Oh, just realized this issue did not have the coding standards tag, doh!

Lars Toomre’s picture

The proposed resolution in the summary makes sense to me. +1

Crell’s picture

+1 from me.

sun’s picture

Sylvain Lecoy’s picture

+1 from me as well.

Crell’s picture

Related, this is where @deprecated would be very useful: #1862398-14: [meta] Replace drupal_http_request() with Guzzle :-)

jhodgdon’s picture

Also being discussed here:
#1848980: Mark variable functions as deprecated until they are removed
Has the @deprecated tag already come into use? Is its usage matching this proposal or something else? Can we adopt this?

sun’s picture

For an official/formal coding standard and usage pattern, I'd prefer and suggest to:

1) properly split the description from the phpDoc directive

2) ideally make the phpDoc directive be followed by an identifier that allows machine-based classification.

To get more concrete, I'd suggest this:

 * @deprecated [version]
 *   [description]

@deprecated phpDoc directive parameters:

  • [version]: Optional. SHOULD specify the version in which the API resource has been marked deprecated.
    • SHOULD refer to a stable release version; e.g., '8.0', '7.3', or '7.x-2.1'.
    • Exception: Resources that are scheduled for removal within a current in-development release MAY specify e.g. '8.x'.
  • [description]: Optional. SHOULD educate developers about what to use instead.
    • SHOULD be very short and concise; e.g.: "Use other_function()."
    • MAY refer to multiple alternatives; e.g.: "Use other_function() to do X or another_function() to do Y.
    • SHOULD refer to the base implementation for DI container services; e.g.: "Use \Drupal\Core\Foo\FooManager::method()."
    • MAY be omitted if there simply is no replacement.

Multiple, valid examples:

  • Just simply deprecated:
     * @deprecated
    
  • Deprecated, but still shipped with 8.0:
     * @deprecated 8.0
    
  • Deprecated and scheduled for removal prior to stable 8.0 release:
     * @deprecated 8.x
    
  • Deprecated in contrib:
     * @deprecated 7.x-2.1
    
  • Procedural alternative:
     * @deprecated 7.3
     *   Use other_function().
    
  • Procedural alternatives:
     * @deprecated 7.3
     *   Use other_function() to save, use another_function() to read.
    
  • DI container service alternative:
     * @deprecated 8.0
     *   Use \Drupal\Core\Extension\ModuleHandler::invoke().
    
jhodgdon’s picture

IMO we should endeavor to match the PHPDoc or Doxygen official syntax as much as possible:
http://www.phpdoc.org/docs/latest/for-users/phpdoc/tags/deprecated.html
http://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmddeprecated

All of the examples in #23 would be OK in that regard. But ... to me, just putting a version there is not telling me what I would want to know, which is:
- When was this deprecated (what version)
- When can I expect it to go away (what version)
Without an explanation, just putting a version number in there is... which one?

Crell’s picture

We're already starting to do this in places, I think, so we should probably finalize what we're doing here.

@deprecated [version if applicable]
  Description that tells me what to use instead and why.

Seems a reasonable and flexible standard to me.

sun’s picture

@jhodgdon: Drupal is a FOSS project, so the only valid answers to your questions are bound to FOSS rules and by design:

The only version that is known and can be specified is the version in which a resource has been marked as deprecated.

Whether, and when, deprecated garbage will actually be cleaned up, is impossible to predict and impossible document.

If we have had this documentation standard since Drupal's existence, then the Drupal 8 code base would still be littered with @deprecated 4.7.x docs today. That's guaranteed.

We didn't have this facility in the past though. That's why the complete list in #1813760: [meta] Clean up @todos and deprecated code is surprisingly small. :)

#23 still makes sense to me today. I'd recommend to move forward with that.

The only possible concern I'm able to see is that we might want to use separate @see directives instead of manually crafted descriptions in most cases. It could be reasonably argued that the description for @deprecated should only be used to state something important that cannot be expressed through simple @see directives. But if that's not the case, human wording is always NOT preferred.

jhodgdon’s picture

Can someone write up the standard you actually want to adopt in a concise way that could be added to node/1354? #23 is quite wordy.

Crell’s picture

@deprecated [VERSION]
  [DESCRIPTION]

Use this tag to indicate that a function, method, or class has been deprecated and should not be used, but has not yet been removed.

A version SHOULD be specified, and should indicate the stable version in which this function was first deprecated (or, if added in a dev version, the next expected stable tag).

A prose description SHOULD be provided for additional context.  At minimum, it should indicate what new mechanism to use to achieve the previous functionality or note that the functionality is slated for outright removal.  An explanation of why the function was deprecated MAY be included.  If known, the description MAY specify the version in which the function is expected to be removed outright.

A @see directive to a direct replacement MAY be included as well.

For a lot of the stuff in core that's marked @deprecated, the expected version for removal would be 8.0 as well. That's fine.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

#28 looks good, very clear. I think there's been more or less consensus that we need this, so let's mark RTBC for a few days and then consider it to be adopted. I'll update the issue summary.

jhodgdon’s picture

Issue summary: View changes

Added issue summary

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs review

I would just like to note that core is actually using all kinds of "standards" for @deprecated... I just did a quick grep and found, for the @deprecated line itself:

* @deprecated as of Drupal 8.0.
* @deprecated as of Drupal 8.0. Use $field->save().
* @deprecated as of Drupal 8.0. Use
* @deprecated This function has been replaced by the \Drupal class. Use that
* @deprecated Use \Drupal\Core\Form\ConfirmFormBase instead.
* @deprecated since 1.12 (to be removed in 2.0) 
[preceding line is from Twig]
* @deprecated Deprecated since version 2.0, to be removed in 2.3.
[preceding line is from Symfony]
* @deprecated This method is only useful for the testing environment, and as

I only found one instance where the suggested format here was actually being followed.

So... Should we:
a) revise the proposed standard to be more in line with what we, Twig, and/or Symfony are actually doing?
b) keep the proposed standard and plan to patch the existing @deprecated uses after we adopt it?

Crell’s picture

*sigh*

Let's make our lives easier and follow the Twig/Symfony example.

jhodgdon’s picture

Twig and Symfony do not use the same standards. One says "@deprecated since 1.12 (to be removed in 2.0)" and the other says "@deprecated Deprecated since version 2.0, to be removed in 2.3.", and there may not even be uniformity within Twig or Symfony (I did not look).

And people have been using all kinds of "standards" in core.

All this illustrates is that there's not a lot of uniformity in @deprecated... but I'm definitely not seeing any adoption of the standard proposed here.

To me, repeating the word "Deprecated" after "@deprecated" seems a bit silly, but other than that I don't have any idea what standard we should adopt. Or maybe we don't even need a standard?

webchick’s picture

IMO trying to come up with rules that granular is a waste of valuable brain power on the part of patch creators, patch reviewers, and core committers alike. The key thing is "When do we/don't we use @deprecated?" IMO. As long as we know that, let the English follow in all of its myriad of interesting ways. :D

webchick’s picture

...and I guess "what information do we make sure to include" is another important one. For example, I think "deprecated since when" is really useful info. As well as "to be removed when," if known.

jhodgdon’s picture

I'm with webchick on not being overly specific about grammar etc. on this one, but yes let's decide what information should and shouldn't be included, and whether the information goes on the same line (like @see) or on the next line in a paragraph-type thing (like @return). I'm not all that fond of the mixed-up line/paragraph style that is currently being used, since it's not the same as any other tags we have:

 * @deprecated blah blah blah blah
 *    blah blah blah.

but whatever.

And what the API module should do with it (which is probably to make a Deprecated header, with the rest of the line/paragraph in a P tag below it?).

Crell’s picture

@deprecated DESCRIPTION

Use this tag to indicate that a function, method, or class has been deprecated and should not be used, but has not yet been removed.

The prose description SHOULD be one or more complete sentences, and SHOULD include:
- The version in which the function was first deprecated (or, if added in a dev version, the next expected stable tag).
- When the function will be removed outright, if known.
- An explanation of why the function was deprecated.
- What new mechanism to use to achieve the previous functionality or note that the functionality is slated for outright removal.

A @see directive to a direct replacement MAY be included as well.

That allows for anything from "@deprecated Will be removed in 8.0. Use the foo event instead." to long-form essays, as appropriate.

jhodgdon’s picture

I'd be OK with that. Any other opinions?

cweagans’s picture

Status: Needs review » Reviewed & tested by the community

+1 from me. That looks great.

catch’s picture

Looks good to me as well.

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

OK, I think we can call this "adopted", since there's been ample time for comment (on the previous version -- and this one is not that different aside from being more flexible), and no dissent that I can see.

So, I went ahead and added it to:
http://drupal.org/coding-standards/docs#deprecated

In addition, I put it in the "order of sections" as coming before @see:
http://drupal.org/coding-standards/docs#order
Hopefully that is OK with everyone.

Thanks!!

Now I need to get this working in the API module. Follow-up issue for that:
#1990042: Support @deprecated tag

plach’s picture

@jhodgdon:

Can we expand the example there? From just reading the description it's not clear to me where the @deprecated tag should be placed (top or bottom of the docblock). Moreover a complete example with versions and stuff could be useful :)

jhodgdon’s picture

platch: see http://drupal.org/coding-standards/docs#order for order of tags.

I don't really want to put lengthy examples in each section... the page is long enough as it is, and in this case I just took what had been provided here (with very minor edits) and put it on the standards page.

Someone did start a child page for more detailed examples, so feel free to add to that!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Add proposal from Crell for specific node/1354 language

sun’s picture

I don't really understand what happened here. We had a sensible proposal in #23. #30 listed a few examples in existing code (which obviously do not and cannot follow any standard if there is none yet). And out of a sudden, we made "no standard" the standard.

That's not appropriate for defining coding standards.

Anyway. FYI, after getting positive feedback, I'm working to get a vastly enhanced version of #23 into the upcoming PSR-5 PHPDoc standard that is in design phase currently:

https://github.com/phpDocumentor/fig-standards/pull/50