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

Scaladoc does not show deprecation message #20118

Open
mwisnicki opened this issue Apr 7, 2024 · 8 comments · May be fixed by #21925
Open

Scaladoc does not show deprecation message #20118

mwisnicki opened this issue Apr 7, 2024 · 8 comments · May be fixed by #21925
Assignees

Comments

@mwisnicki
Copy link

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")
@mwisnicki mwisnicki added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 7, 2024
@som-snytt
Copy link
Contributor

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.

@Gedochao Gedochao added area:annotations area:doctool and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 8, 2024
@SethTisue
Copy link
Member

Scala 2 does show the deprecation message:

Screenshot 2024-11-06 at 3 14 18 PM

@SethTisue SethTisue added the Spree Suitable for a future Spree label Nov 6, 2024
@SethTisue
Copy link
Member

SethTisue commented Nov 11, 2024

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
Copy link
Member

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
Copy link
Member

SethTisue commented Nov 11, 2024

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
Copy link
Member

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.

HarrisL2 added a commit to HarrisL2/scala3 that referenced this issue Nov 11, 2024
@SethTisue
Copy link
Member

SethTisue commented Nov 11, 2024

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

HarrisL2 added a commit to HarrisL2/scala3 that referenced this issue Nov 11, 2024
@SethTisue SethTisue self-assigned this Nov 11, 2024
@HarrisL2
Copy link
Contributor

It seems that the existing test suite only tests for the generated signatures, and does not concern with annotations. We will have to add a new test type to test for the deprecated annotation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants