scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.88k stars 1.06k forks source link

Scaladoc does not show deprecation message #20118

Open mwisnicki opened 7 months ago

mwisnicki commented 7 months ago

https://www.scala-lang.org/api/3.4.1/scala/reflect/ClassManifestDeprecatedApis.html shows:

Deprecated true

https://github.com/scala/scala/blob/80514f73a6c7db32df9887d9a5ca9ae921e25118/src/library/scala/reflect/ClassManifestDeprecatedApis.scala#L21

@deprecated("use scala.reflect.ClassTag instead", "2.10.0")
som-snytt commented 7 months ago

Probably there is a design discussion somewhere, but just yesterday I was appreciating the obvious ~strike-thru~ to indicate deprecation in Scaladoc 2. I would actually prefer the international symbol 🔴 I mean the red circle with strike-thru, here is the emoji via twitter: image

Plus the message, of course.

SethTisue commented 6 days ago

Scala 2 does show the deprecation message:

Screenshot 2024-11-06 at 3 14 18 PM
SethTisue commented 1 day ago

there is code in MemberRenderer.scala that appears to attempt to implement this!

    val message = named.find(_.name.get == "message")
    val since = named.find(_.name.get == "since")
    ...

we'll have to figure out why it doesn't work

SethTisue commented 1 day ago

Ahh, so @HarrisL2 figured out that it sometimes does work! it works if the deprecation was done with named arguments, like this:

@deprecated(message = "foo", since = "2.10.0")

but it doesn't work if the names on the named arguments are missing:

@deprecated("foo", "2.10.0")
SethTisue commented 1 day ago

We don't know if it's safe to just access the parameters by position, but we don't seem to have any choice. We'll need to make sure it still works if the order at the call site is swapped, like @deprecated(since = "2.10.0", message = "foo")

SethTisue commented 1 day ago

If it doesn't work when the order is swapped, then we'll need to use the names if they are present, and only fall back to positional if we must.

SethTisue commented 1 day ago

We never finished figuring out the correct way to add a unit test for this. We'll need to figure that out before we un-draft the PR. Do we need to write a JSoup-based test?

The testing methodology I used instead was to publishLocal a modified compiler, then use scala-cli to run it on a test source file with scala doc -S 3.6.3-RC1-bin-SNAPSHOT .

The test source file:

class C:
  /** one */
  @deprecated("without names", "2.10.0")
  def foo = 0
  /** two */
  @deprecated(message = "with names", since = "2.10.0")
  def bar = 1
  /** three */
  @deprecated(since = "2.10.0", message = "backwards names")
  def baz = 2

the code change:

--- scaladoc/src/dotty/tools/scaladoc/renderers/MemberRenderer.scala
+++ scaladoc/src/dotty/tools/scaladoc/renderers/MemberRenderer.scala
-    val (named, unnamed) = a.params.partition(_.name.nonEmpty)
-    val message = named.find(_.name.get == "message")
-    val since = named.find(_.name.get == "since")
+    val message = a.params.lift(0)
+    val since = a.params.lift(1)

rendered:

Screenshot 2024-11-11 at 6 48 16 PM