Merge pull request #13910 from dotty-staging/scaladoc/backport-bugfixes-311

Kordyjan · web-flow · commit b344ee040e49 · 2021-12-14T12:12:16.000+01:00
Backport Scaladoc bugfixes to 3.1.1
diff --git a/scaladoc-testcases/src/tests/externalLocations/externalStubs.scala b/scaladoc-testcases/src/tests/externalLocations/externalStubs.scala
@@ -0,0 +1,5 @@
+package tests.externalStubs
+
+trait \/
+
+trait /\
diff --git a/scaladoc-testcases/src/tests/externalLocations/scaladoc2.scala b/scaladoc-testcases/src/tests/externalLocations/scaladoc2.scala
@@ -1,6 +1,8 @@
-package tests.externalScaladoc2
+package tests
+package externalScaladoc2
 
 import scala.util.matching.*
+import externalStubs._
 
 class Test {
   def a: String = ???
@@ -10,5 +12,7 @@ class Test {
   def c: Regex.Match = ???
 }
 
+class Test2 extends \/ with /\
+
 abstract class MySeq[T] extends scala.collection.immutable.Seq[T]
 
diff --git a/scaladoc-testcases/src/tests/externalLocations/scaladoc3.scala b/scaladoc-testcases/src/tests/externalLocations/scaladoc3.scala
@@ -1,6 +1,8 @@
-package tests.externalScaladoc3
+package tests
+package externalScaladoc3
 
 import scala.util.matching.*
+import externalStubs._
 
 class Test {
   def a: String = ???
@@ -10,3 +12,5 @@ class Test {
   def c: Regex.Match = ???
 }
 
+class Test2 extends \/ with /\
+
diff --git a/scaladoc-testcases/src/tests/givenSignatures.scala b/scaladoc-testcases/src/tests/givenSignatures.scala
@@ -4,13 +4,19 @@ package givenSignatures
 
 object Obj
 
-given Seq[String] = Nil
+given Seq[String] = Nil //expected: given given_Seq_String: Seq[String]
 
-given GivenType = GivenType()
+given GivenType = GivenType() //expected: given given_GivenType: GivenType
 
 class GivenType
 
 trait Ord[T]
 
 given listOrd[T](using ord: Ord[T]): Ord[List[T]]
   = ???
+
+trait Foo[A]
+
+given listOrd: Foo[String] with { val i: Int = 1 } //expected: given listOrd: listOrd.type
+
+trait Placeholder //expected: object listOrd extends Foo[String]
diff --git a/scaladoc-testcases/src/tests/inheritedMembers1.scala b/scaladoc-testcases/src/tests/inheritedMembers1.scala
@@ -10,7 +10,11 @@ class A
       = ???
     object X
     trait Z
-    given B with {}
+    given B with { val x = 1 }//expected: given given_B: given_B.type
+    trait Placeholder//expected: object given_B extends B
+
+    object Y extends Z
+
     type I = Int
     /*<-*/extension (a: A) /*->*/def extension: String
       = ???
diff --git a/scaladoc-testcases/src/tests/slashMembers.scala b/scaladoc-testcases/src/tests/slashMembers.scala
@@ -0,0 +1,8 @@
+package tests
+package slashMembers
+
+class A
+
+trait \/
+
+trait /\
diff --git a/scaladoc/src/dotty/tools/scaladoc/ScaladocSettings.scala b/scaladoc/src/dotty/tools/scaladoc/ScaladocSettings.scala
@@ -18,12 +18,8 @@ import dotty.tools.dotc.core.Contexts._
 
 class ScaladocSettings extends SettingGroup with AllScalaSettings:
   val unsupportedSettings = Seq(
-    // Options that we like to support
-    extdirs, javabootclasspath, encoding,
     // Needed for plugin architecture
-    plugin,disable,require, pluginsDir, pluginOptions,
-    // we need support for sourcepath and sourceroot
-    sourcepath, sourceroot
+    plugin, disable, require, pluginsDir, pluginOptions,
   )
 
 
diff --git a/scaladoc/src/dotty/tools/scaladoc/api.scala b/scaladoc/src/dotty/tools/scaladoc/api.scala
@@ -67,7 +67,7 @@ enum Kind(val name: String):
   case Exported(m: Kind.Def) extends Kind("export")
   case Type(concreate: Boolean, opaque: Boolean, typeParams: Seq[TypeParameter])
     extends Kind("type") // should we handle opaque as modifier?
-  case Given(kind: Def | Class, as: Option[Signature], conversion: Option[ImplicitConversion])
+  case Given(kind: Def | Class | Val.type, as: Option[Signature], conversion: Option[ImplicitConversion])
     extends Kind("given") with ImplicitConversionProvider
   case Implicit(kind: Kind.Def | Kind.Val.type, conversion: Option[ImplicitConversion])
     extends Kind(kind.name)  with ImplicitConversionProvider
diff --git a/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala b/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala
@@ -13,6 +13,7 @@ import java.nio.file.Path
 import java.nio.file.Files
 import java.io.File
 import scala.util.matching._
+import dotty.tools.scaladoc.util.Escape._
 
 val UnresolvedLocationLink = "#"
 
diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/ClassLikeSupport.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/ClassLikeSupport.scala
@@ -97,15 +97,16 @@ trait ClassLikeSupport:
     val baseMember = mkMember(classDef.symbol, kindForClasslike(classDef), selfSignature)(
       modifiers = modifiers,
       graph = graph,
-      deprecated = classDef.symbol.isDeprecated()
+      deprecated = classDef.symbol.isDeprecated(),
+    ).copy(
+      directParents = classDef.getParentsAsLinkToTypes,
+      parents = supertypes,
     )
 
     if summon[DocContext].args.generateInkuire then doInkuireStuff(classDef)
 
     if signatureOnly then baseMember else baseMember.copy(
         members = classDef.extractPatchedMembers.sortBy(m => (m.name, m.kind.name)),
-        directParents = classDef.getParentsAsLinkToTypes,
-        parents = supertypes,
         selfType = selfType,
         companion = classDef.getCompanion
     )
@@ -144,15 +145,6 @@ trait ClassLikeSupport:
           )
           parseMethod(c, dd.symbol,specificKind = Kind.Extension(target, _))
         }
-      // TODO check given methods?
-      case dd: DefDef if !dd.symbol.isHiddenByVisibility && dd.symbol.isGiven && !dd.symbol.isArtifact =>
-        Some(dd.symbol.owner.typeMember(dd.name))
-          .filterNot(_.exists)
-          .map { _ =>
-            parseMethod(c, dd.symbol, specificKind =
-              Kind.Given(_, getGivenInstance(dd), None)
-            )
-          }
 
       case dd: DefDef if !dd.symbol.isHiddenByVisibility && dd.symbol.isExported && !dd.symbol.isArtifact =>
         val exportedTarget = dd.rhs.collect {
@@ -171,58 +163,25 @@ trait ClassLikeSupport:
         Some(parseMethod(c, dd.symbol, specificKind = Kind.Exported(_))
           .withOrigin(Origin.ExportedFrom(s"$instanceName.$functionName", dri)))
 
-      case dd: DefDef if !dd.symbol.isHiddenByVisibility && !dd.symbol.isGiven && !dd.symbol.isSyntheticFunc && !dd.symbol.isExtensionMethod && !dd.symbol.isArtifact =>
+      case dd: DefDef if !dd.symbol.isHiddenByVisibility && !dd.symbol.isSyntheticFunc && !dd.symbol.isExtensionMethod && !dd.symbol.isArtifact =>
         Some(parseMethod(c, dd.symbol))
 
       case td: TypeDef if !td.symbol.flags.is(Flags.Synthetic) && (!td.symbol.flags.is(Flags.Case) || !td.symbol.flags.is(Flags.Enum)) =>
         Some(parseTypeDef(td))
 
-      case vd: ValDef if !isSyntheticField(vd.symbol)
-        && (!vd.symbol.flags.is(Flags.Case) || !vd.symbol.flags.is(Flags.Enum))
-        && vd.symbol.isGiven =>
-          val classDef = Some(vd.tpt.tpe).flatMap(_.classSymbol.map(_.tree.asInstanceOf[ClassDef]))
-          Some(classDef.filter(_.symbol.flags.is(Flags.Module)).fold[Member](parseValDef(c, vd))(parseGivenClasslike(_)))
-
       case vd: ValDef if !isSyntheticField(vd.symbol) && (!vd.symbol.flags.is(Flags.Case) || !vd.symbol.flags.is(Flags.Enum)) =>
         Some(parseValDef(c, vd))
 
-      case c: ClassDef if c.symbol.owner.methodMember(c.name).exists(_.flags.is(Flags.Given)) =>
-        Some(parseGivenClasslike(c))
-
-      case c: ClassDef if c.symbol.shouldDocumentClasslike &&  !c.symbol.isGiven =>
+      case c: ClassDef if c.symbol.shouldDocumentClasslike =>
         Some(parseClasslike(c))
 
       case _ => None
   }
 
-  private def parseGivenClasslike(c: ClassDef): Member = {
-    val parsedClasslike = parseClasslike(c)
-
-    val parentTpe = c.parents(0) match {
-      case t: TypeTree => Some(t.tpe)
-      case t: Term => Some(t.tpe)
-      case _ => None
-    }
-
-    val givenParents = parsedClasslike.directParents.headOption
-    val cls: Kind.Class = parsedClasslike.kind match
-      case Kind.Object => Kind.Class(Nil, Nil)
-      case Kind.Trait(tps, args) => Kind.Class(tps, args)
-      case cls: Kind.Class => cls
-      case other =>
-        report.warning("Unrecoginzed kind for given: $other", c.pos)
-        Kind.Class(Nil, Nil)
-
-    parsedClasslike.withKind(
-      Kind.Given(cls, givenParents.map(_.signature), parentTpe.flatMap(extractImplicitConversion))
-    )
-  }
-
   private def parseInheritedMember(c: ClassDef)(s: Tree): Option[Member] =
     def inheritance = Some(InheritedFrom(s.symbol.owner.normalizedName, s.symbol.dri))
     processTreeOpt(s)(s match
-      case c: ClassDef if c.symbol.shouldDocumentClasslike && !c.symbol.isGiven => Some(parseClasslike(c, signatureOnly = true))
-      case c: ClassDef if c.symbol.owner.methodMember(c.name).exists(_.flags.is(Flags.Given)) => Some(parseGivenClasslike(c))
+      case c: ClassDef if c.symbol.shouldDocumentClasslike => Some(parseClasslike(c, signatureOnly = true))
       case other => {
         val parsed = parseMember(c)(other)
         parsed.map(p =>
@@ -392,6 +351,7 @@ trait ClassLikeSupport:
           ))
         case _ =>
           Kind.Implicit(basicKind, None)
+      else if methodSymbol.flags.is(Flags.Given) then Kind.Given(basicKind, Some(method.returnTpt.tpe.asSignature), extractImplicitConversion(method.returnTpt.tpe))
       else specificKind(basicKind)
 
     val origin = if !methodSymbol.isOverridden then Origin.RegularlyDefined else
@@ -463,10 +423,10 @@ trait ClassLikeSupport:
   def parseValDef(c: ClassDef, valDef: ValDef): Member =
     def defaultKind = if valDef.symbol.flags.is(Flags.Mutable) then Kind.Var else Kind.Val
     val memberInfo = unwrapMemberInfo(c, valDef.symbol)
-    val kind = if valDef.symbol.flags.is(Flags.Implicit) then
-        Kind.Implicit(Kind.Val, extractImplicitConversion(valDef.tpt.tpe))
-        else if valDef.symbol.flags.is(Flags.Enum) then Kind.EnumCase(Kind.Val)
-        else defaultKind
+    val kind = if valDef.symbol.flags.is(Flags.Implicit) then Kind.Implicit(Kind.Val, extractImplicitConversion(valDef.tpt.tpe))
+      else if valDef.symbol.flags.is(Flags.Given) then Kind.Given(Kind.Val, Some(memberInfo.res.asSignature), extractImplicitConversion(valDef.tpt.tpe))
+      else if valDef.symbol.flags.is(Flags.Enum) then Kind.EnumCase(Kind.Val)
+      else defaultKind
 
     mkMember(valDef.symbol, kind, memberInfo.res.asSignature)(deprecated = valDef.symbol.isDeprecated())
 
diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/NameNormalizer.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/NameNormalizer.scala
@@ -9,14 +9,14 @@ import SymOps._
 
 object NameNormalizer {
 
-  extension (using Quotes)(s: reflect.Symbol) def normalizedName: String = {
-    import reflect.*
-    val withoutGivenPrefix = if s.isGiven then s.name.stripPrefix("given_") else s.name
-    val withoutObjectSuffix = if s.flags.is(Flags.Module) then withoutGivenPrefix.stripSuffix("$") else withoutGivenPrefix
-    val constructorNormalizedName = if s.isClassConstructor then "this" else withoutObjectSuffix
-    val escaped = escapedName(constructorNormalizedName)
-    escaped
-  }
+  extension (using Quotes)(s: reflect.Symbol)
+    def normalizedName: String = {
+      import reflect.*
+      val withoutObjectSuffix = if s.flags.is(Flags.Module) then s.name.stripSuffix("$") else s.name
+      val constructorNormalizedName = if s.isClassConstructor then "this" else withoutObjectSuffix
+      val escaped = escapedName(constructorNormalizedName)
+      escaped
+    }
 
   private val ignoredKeywords: Set[String] = Set("this")
 
diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala
@@ -115,7 +115,6 @@ object SymOps:
       !sym.isHiddenByVisibility
       && !sym.flags.is(Flags.Synthetic)
       && (!sym.flags.is(Flags.Case) || !sym.flags.is(Flags.Enum))
-      && !(sym.companionModule.flags.is(Flags.Given))
 
     def getCompanionSymbol: Option[reflect.Symbol] = Some(sym.companionClass).filter(_.exists)
 
@@ -249,7 +248,6 @@ class SymOpsWithLinkCache:
     def dri(using dctx: DocContext): DRI =
       import reflect.*
       if sym == Symbol.noSymbol then topLevelDri
-      else if sym.isValDef && sym.moduleClass.exists then sym.moduleClass.dri
       else
         val method =
           if (sym.isDefDef) Some(sym)
@@ -261,7 +259,7 @@ class SymOpsWithLinkCache:
         else
           (sym.className, sym.anchor)
 
-        val location = sym.packageNameSplitted ++ className
+        val location = (sym.packageNameSplitted ++ className).map(escapeFilename(_))
 
         val externalLink = {
             import reflect._
diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala
@@ -61,7 +61,7 @@ trait TypesSupport:
   extension (on: SignaturePart) def l: List[SignaturePart] = List(on)
 
   private def tpe(using Quotes)(symbol: reflect.Symbol): SSignature =
-    val suffix = if symbol.isValDef then plain(".type").l else Nil
+    val suffix = if symbol.isValDef || symbol.flags.is(reflect.Flags.Module) then plain(".type").l else Nil
     dotty.tools.scaladoc.Type(symbol.normalizedName, Some(symbol.dri)) :: suffix
 
   private def commas(lists: List[SSignature]) = lists match
diff --git a/scaladoc/src/dotty/tools/scaladoc/translators/ScalaSignatureProvider.scala b/scaladoc/src/dotty/tools/scaladoc/translators/ScalaSignatureProvider.scala
@@ -22,6 +22,8 @@ object ScalaSignatureProvider:
         givenClassSignature(documentable, cls, builder)
       case Kind.Given(d: Kind.Def, _, _) =>
         givenMethodSignature(documentable, d, builder)
+      case Kind.Given(Kind.Val, _, _) =>
+        givenValSignature(documentable, builder)
       case cls: Kind.Class =>
         classSignature(documentable, cls, builder)
       case enm: Kind.Enum =>
@@ -129,10 +131,24 @@ object ScalaSignatureProvider:
     case Kind.Given(_, Some(instance), _) =>
       builder.keyword("given ")
         .name(method.name, method.dri)
+        .generics(body.typeParams)
+        .functionParameters(body.argsLists)
         .plain(": ")
         .signature(instance)
     case _ =>
-      builder.keyword("given ").name(method.name, method.dri)
+      builder.keyword("given ")
+        .name(method.name, method.dri)
+        .generics(body.typeParams)
+        .functionParameters(body.argsLists)
+
+  private def givenValSignature(field: Member, builder: SignatureBuilder): SignatureBuilder = field.kind match
+    case Kind.Given(_, Some(instance), _) =>
+      builder.keyword("given ")
+        .name(field.name, field.dri)
+        .plain(": ")
+        .signature(instance)
+    case _ =>
+      builder.keyword("given ").name(field.name, field.dri)
 
   private def methodSignature(method: Member, cls: Kind.Def, builder: SignatureBuilder): SignatureBuilder =
     val bdr = builder
diff --git a/scaladoc/src/dotty/tools/scaladoc/util/escape.scala b/scaladoc/src/dotty/tools/scaladoc/util/escape.scala
@@ -1,4 +1,11 @@
 package dotty.tools.scaladoc.util
 
 object Escape:
-  def escapeUrl(url: String) = url.replace("#","%23")
+  def escapeUrl(url: String) = url
+    .replace("#","%23")
+
+  def escapeFilename(filename: String) =
+    val escaped = filename
+      .replace("/", "$div")
+      .replace("\\", "$bslash")
+    if escaped != filename then escaped + "$" else escaped
diff --git a/scaladoc/test/dotty/tools/scaladoc/ExternalLocationProviderIntegrationTest.scala b/scaladoc/test/dotty/tools/scaladoc/ExternalLocationProviderIntegrationTest.scala
@@ -25,23 +25,33 @@ class JavadocExternalLocationProviderIntegrationTest extends ExternalLocationPro
 
 class Scaladoc2ExternalLocationProviderIntegrationTest extends ExternalLocationProviderIntegrationTest(
   "externalScaladoc2",
-  List(".*scala.*::scaladoc2::https://www.scala-lang.org/api/current/"),
+  List(
+    ".*scala/.*::scaladoc2::https://www.scala-lang.org/api/current/",
+    ".*externalStubs.*::scaladoc2::https://external.stubs/api/"
+  ),
   List(
     "https://www.scala-lang.org/api/current/scala/util/matching/Regex$$Match.html",
     "https://www.scala-lang.org/api/current/scala/Predef$.html#String",
     "https://www.scala-lang.org/api/current/scala/collection/immutable/Map.html",
     "https://www.scala-lang.org/api/current/scala/collection/IterableOnceOps.html#addString(b:StringBuilder,start:String,sep:String,end:String):StringBuilder",
-    "https://www.scala-lang.org/api/current/scala/collection/IterableOnceOps.html#mkString(start:String,sep:String,end:String):String"
+    "https://www.scala-lang.org/api/current/scala/collection/IterableOnceOps.html#mkString(start:String,sep:String,end:String):String",
+    "https://external.stubs/api/tests/externalStubs/$div$bslash$.html",
+    "https://external.stubs/api/tests/externalStubs/$bslash$div$.html"
   )
 )
 
 class Scaladoc3ExternalLocationProviderIntegrationTest extends ExternalLocationProviderIntegrationTest(
   "externalScaladoc3",
-  List(".*scala.*::scaladoc3::https://dotty.epfl.ch/api/"),
+  List(
+    ".*scala/.*::scaladoc3::https://dotty.epfl.ch/api/",
+    ".*externalStubs.*::scaladoc3::https://external.stubs/api/"
+  ),
   List(
     "https://dotty.epfl.ch/api/scala/collection/immutable/Map.html",
     "https://dotty.epfl.ch/api/scala/Predef$.html#String-0",
-    "https://dotty.epfl.ch/api/scala/util/matching/Regex$$Match.html"
+    "https://dotty.epfl.ch/api/scala/util/matching/Regex$$Match.html",
+    "https://external.stubs/api/tests/externalStubs/$div$bslash$.html",
+    "https://external.stubs/api/tests/externalStubs/$bslash$div$.html"
   )
 )
 
diff --git a/scaladoc/test/dotty/tools/scaladoc/signatures/SignatureTest.scala b/scaladoc/test/dotty/tools/scaladoc/signatures/SignatureTest.scala
