Disallow @deprecated without message/version

[scabug]

We really should just outlaw deprecated annotations without message/version. Nothing good comes from it, because it seems the warning already emitted is too easy to ignore.

This is pretty annoying because it means we will be stuck with those elements for another few years. Example: https://github.com/scala/scala/blob/2.11.x/src/library/scala/collection/Iterator.scala#L169

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-9079?orig=1
Reporter: @soc
Affected Versions: 2.10.4, 2.11.4

[eed3si9n]

Would a PR on this accepted?
Remove the default parameter, or add overload constructors and deprecate them as softer landing?

https://github.com/scala/scala/blob/v2.13.0-M3/src/library/scala/deprecated.scala

[SethTisue]

the "softer landing" option sgtm.

(it's always tempting to cut out the deprecation step, but in general I think we should only consider that if there's some special advantage to it, like if keeping the deprecated thing has some real extra cost. I don't see that applying here)

[mghildiy]

How is second approach better than first here?

[som-snytt]

I think you have to feel strongly about this issue to force the extra args. Probably only soc felt that strongly.

Consider that deprecations sit around forever. What does the since mean IRL?

The existing warning is already obnoxious. Folks know to use -Xfatal-warnings if they want to rub salt in the wound. (Listening to Parsifal ATM.)

scala> @deprecated(since="1.0") def f = 42
<console>:11: warning: @deprecated now takes two arguments; see the scaladoc.
       @deprecated(since="1.0") def f = 42
        ^
<console>:11: warning: Usage of named or default arguments transformed this annotation
constructor call into a block. The corresponding AnnotationInfo
will contain references to local values and default getters instead
of the actual argument trees
       @deprecated(since="1.0") def f = 42
        ^
f: Int

scala> @deprecated(message="Get out!") def f = 42
<console>:11: warning: @deprecated now takes two arguments; see the scaladoc.
       @deprecated(message="Get out!") def f = 42
        ^
f: Int

I want @deprecated to use my current project version to say, "deprecated since 0.5".

Also, annotations that don't complain with constant args out of order. That's another ticket.

[NthPortal]

@som-snytt since has a few uses:

• To let maintainers know how long it's been deprecated, so they know when it can be removed (without it, Remove items deprecated in 2.12 or earlier #10033 would have been impossible to deal with).
• To let users know how long it has been deprecated. This is useful both to reduce confusion ("how long ago was I supposed to have stopped using this?"), and to allow users to estimate how long they have to fix their code which uses deprecated APIs.

Consider that deprecations sit around forever.

In the Scala library, they do not (see scala/scala#6319).

Leaving out message is even worse. When deprecating something, it is imperative to tell people what to use instead.

[NthPortal]

@mghildiy Are you still working on this?

[som-snytt]

@NthPortal I'm not arguing to remove since but to consider with extra care before requiring it.

If the Scala library wants to lint its usage of deprecated, that would help coordinate removal of API.

But it's not necessary to make the rest of us follow a narrow use case for deprecated.

Java now distinguishes between "ordinarily deprecated" and "terminally deprecated" (the language in 9.6.4.6 of the spec), based on forRemoval, which defaults to false.

Deprecations since 2.10.0 or 2.9.0 have sat around forever, by any reasonable standard.

But other improvements are possible besides "require two strings". The Scaladoc also offers dubious advice about the format of the strings. Why should I need to transcribe "mylib 0.5"? The compiler and sbt know what I'm building. I had a plugin to make sure the second string looks like a version and that I didn't mix them up.

A reasonable project standard is to require an empty or minimal message with a full explanation in Scaladoc. That minimizes noise in build output.

To require @deprecated("", "") in lieu of @deprecated is boilerplate.

Another possible tooling approach is to -Xlint:deprecation-policy, to enforce both fields are set. (A built-in linter instead of a scalafix.)

Also consider a field like forRemoval, such as until=1.0, which if present means terminally deprecated and if non-empty indicates expected removal.

[som-snytt]

There is -Xlint:deprecation, since=2.13.0, for emitting the warning about an underspecified deprecation, and now also -Wconf for making it an error. So there are knobs for requiring pristine deprecations.

val Deprecation            = LintWarning("deprecation",               "Enable -deprecation and also check @deprecated annotations.")

I don't have a strong personal-technical opinion about deprecation, so I don't know why I spilled so much virtual ink.

My previous usage of deprecation is to add the annotation to tests that test deprecated API, in order to suppress the deprecation warning; that was before the nowarn annotation.

I see that the PR included forRemoval on a trial basis. Adriaan makes the interesting suggestion to encode it in the message as "#forRemoval" which could be used by -Wconf.

How is deprecation like an erhu? They both "require two strings."