Scaladoc regression in 2.13.4 related to relative entity link

[BalmungSan]

Reproduction steps

Using Scala 2.13.4 having the following in a file called test.scala

object foo {
  trait Bar {
    /** Returns a new [[Bar]]. */
    def f: Bar = ???
  }
}

trait Baz extends foo.Bar

Then running scaladoc test.scala produces the following warning:

test.scala:3: warning: Could not find any member to link for "Bar". /** Returns a new [[Bar]] by applying a function. */ ^

Problem

I would expect it to properly handle the relative entity link to Bar without producing any warnings; as it does when using 2.13.3, 2.12.13, 2.12.12, 2.11.12 Also, note that everything in that snippet is required to reproduce the error. If you remove the additional trait Baz or you remove the enclosing object foo then it works.

[SethTisue]

A likely next step here would be to bisect it to determine the PR responsible for the regression. That's almost always more efficient than trying to debug it from scratch.

[dwijnand]

Looks like https://github.com/scala/scala/pull/9060 is responsible.

[liang3zy22]

I have debugged this issue. This issue still can be reproduced without the commit scala/scala#9060, just use following codes:

object foo {
  trait Bar {
    /** Returns a new [[Bar]]. */
    def f: Bar = ???
  }
}

trait Baz extends foo.Bar {
  override def f: foo.Bar = super.f
}

If using [[foo.Bar]], the warning will disappear.

[SethTisue]

@liang3zy22 thanks, that's very helpful information! and quite odd.

I'm looking at this with @timshel at the Open Source Spree in Paris

~We're digging into it, but it seems strange that the presence or absence of Baz would make any difference at all.~ see below

[som-snytt]

insert Baz Luhrmann joking reference because it's Paris Spree

[SethTisue]

Duh... as Jacques understood well before I did, it's not Baz's mere presence that is causing a problem. Baz#f inherits the doc comment from Bar#f, and then Bar isn't found because Bar isn't in scope inside Baz!

It makes sense that this might be sensitive to whether override def f is present, because then the f in Baz has a new symbol with a different owner. In that case we have our own implementation of f but not our own doc comment.

And it makes perfect sense that object foo is necessary, because foo is why Bar isn't in scope in Baz.

It seems the logic in NonTemplateMemberImpl still isn't quite right. The code for inRealTpl attempts to decide what owner to use to look up symbols in, but it isn't taking into account that the doc comment itself is inherited (regardless of whether f is overridden or not).

[SethTisue]

We are handicapped by lacking an understanding of the mechanism by which doc comments are inherited. It isn't easy to find in the code.

Ah, but look! Some of it isn't tools.nsc.doc, it's in tools.nst.ast, e.g. in DocComments.scala.

[SethTisue]

We do not see an easy way to make the case identified by @liang3zy22 work. The need for #9785 to also stay fixed at the time makes it even harder.

As background, note that when a doc comment is inherited, it is inherited literally, as a string, and without a record being kept of where it came from. In DocComments.scala, we see:

   superComment(sym) match {
      ...
      case Some(sc) =>
        if (ownComment == "") sc
        else ... ownComment ...
    }
  })

No record is kept that this was done. (But perhaps we could still figure it out again for ourselves, later, during doc generation?)

The conflict with #9785 is that for that bug, it's desirable to be looking up symbols in the current template. But for this bug, it's desirable to be looking up symbols in the template where the doc comment was inherited from.

[SethTisue]

One possible way of looking at this ticket is that things are already worked as designed, and it's only by accident that Luis's case used to work. #9785 is more important to have fixed — in that one, the generated documentation was actually incorrect.

Doc comments are inherited as strings, before link expansion has occurred. It shouldn't really be considered too surprising that when a doc comment refers to an identifier that isn't in scope anymore, the link doesn't work.

So I've removed the "good first issue" label because it isn't easy, and I'm also removing the "help wanted" label, because although we'd be happy if someone figured out a way to resolve it, the bug isn't so bad: a warning is emitted (it doesn't fail silently), and a workaround exists (using the full name).

[som-snytt]

"They're all good first issues, Seth."

[SethTisue]

We tried it in Scaladoc 3 and even the latest nightly chokes on the version with the def f override:

11 |  override def f: foo.Bar = super.f
   |               ^
   |               No DRI found for query: Bar

and both versions (with and without the override) generate Scaladoc with broken links to Bar.

🤷

We don't have a "NOT fixed in Scala 3" label in this repo, only a "fixed in Scala 3" label

[liang3zy22]

Just saw the comments from @SethTisue and @som-snytt. I think when doc comment is inherited, the link should be already created. But this may break current codes logic.