Deprecated annotation best practice

Hi devs,

This tweet https://twitter.com/piotrprz/status/1343866413107310594 prompted me to check our documented practice for deprecated and I’ve noticed that we don’t have a best practice documented for @Deprecated on https://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/, so here’s a proposal:

Before we move to Java 9+

Rules:

  • Always use both @Deprecated annotation and the @deprecated javadoc tag.
  • In the @deprecated javadoc tag, always specify since WHICH version it’s deprecated, and WHAT should be used instead. Also explain WHY it’s deprecated.

Examples:

  • Good:
    • Couldn’t find any example in xwiki-platform! It should specify the 3 elements (version, what, why).
  • Partially good (missing WHY):
      /**
       * The hierarchy of Entity Types.
       * 
       * @deprecated since 10.6RC1, use {@link EntityType#getAllowedParents()} instead
       */
      @Deprecated
      Map<EntityType, List<EntityType>> PARENT_TYPES = new EnumMap<EntityType, List<EntityType>>(EntityType.class)
    
  • Bad (missing version):
     /**
      * @return the first form contained in this section.
      * @deprecated this method can be ambiguous since an administration section page might contain several forms.
      *              The method {@link #getFormContainerElement(String)} should be used instead.
      */
     @Deprecated
     public FormContainerElement getFormContainerElement()
     {
         return new FormContainerElement(this.formContainer);
     }
    
  • Bad:
    @Deprecated
    public class i18n
    

After we move to Java 9

Rules:

  • Same idea of the 3 elements that must be specified.
  • However the @Deprecated annotation now support 2 parameters and they must both be used with forRemoval always set to true and since should specify the version.
  • The javadoc tag must still be there to explain the WHY and the WHAT to use.

Good example:

public class Worker {
    /**
     * Calculate period between versions
     * @deprecated This method is no longer acceptable to compute time between versions because...
     * Use {@link Utils#calculatePeriod(Machine)} instead.
     *
     * @param machine instance
     * @return computed time
     */
    @Deprecated(since = "4.5", forRemoval = true)
    public int calculate(Machine machine) {
        return machine.exportVersions().size() * 10;
    }
}

WDYT?

Thanks

1 Like

I think this is an good solution.

Globally +1 for the deprecated with the 3 infos.

I’m not sure about this: whenever we moved a deprecated method / class in legacy we consider it as “removed” and it will stay like this forever basically with the deprecated annotation. So the semantic of the “forRemoval” seems a bit wrong there for me.

ah yes good point. So it’s the opposite, we must never use forRemoval (default is false).

Pretty much copy/paste of @surli’s message.

The rule has now been documented at Java Code Style (Community.CodeStyle.JavaCodeStyle.WebHome) - XWiki

It’s also linked from https://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HDeprecatingCode

One thing we didn’t discuss was whether to explicitly set forRemoval = false or use the default (which is false). My IDE says it’s redundant since it’s the default. OTOH if not specified, we could maybe thing that the coder didn’t think about this aspect.

Now, since in XWiki we always want this to be false, I think it’s fine to omit it.

WDYT?

+1 no reason to repeat the default IMO

FTR, I’ve also implemented [XCOMMONS-2371] Don't fail the build when using the "since" parameter of the @Deprecated annotation - XWiki.org JIRA

Example of converting some existing @Deprecated annotations: https://github.com/xwiki/xwiki-commons/commit/aa5c7e040786ad694cb0ea27a9eef15884f1f13c

What do we do when the deprecation is done in several branches at the same time?

TODO: check if it’s legit to use 2 @Deprecated(since = ...) annotations.

Answer: it’s not legit.

Proposal: use a comma-separated list as in xwiki-platform/MentionLocation.java at 99353cb06bf62de1819d1bc5d757e881a3f35f38 · xwiki/xwiki-platform · GitHub

Note: We should support having a space after the comma IMO.

Does someone know of any tool using that annotation (except the javadoc tool)?

Thx

I’ve now documented it at Java Code Style - XWiki

You mean aside from xwiki-commons/UnstableAnnotationCheck.java at master · xwiki/xwiki-commons · GitHub and xwiki-commons/SinceFormatCheck.java at master · xwiki/xwiki-commons · GitHub ?

AFAIK UnstableAnnotationCheck is not looking at the @Deprecated annotation. It’s only using the @since javadoc tags.

Right, sorry I mixed. I don’t know why I thought about it as checking deprecation versions, which does not make sense.

So no, no tool in mind.