Merge pull request #1771 from vigdorchik/si_6605

Cleanup MemberLookup. Better explain ambiguous link targets.

5 files changed, 56 insertions, 46 deletions

diff --git a/src/compiler/scala/tools/nsc/doc/base/MemberLookupBase.scala b/src/compiler/scala/tools/nsc/doc/base/MemberLookupBase.scala
index f3a5660dc4..35390adcd9 100755
--- a/src/compiler/scala/tools/nsc/doc/base/MemberLookupBase.scala
+++ b/src/compiler/scala/tools/nsc/doc/base/MemberLookupBase.scala
@@ -15,6 +15,7 @@ trait MemberLookupBase {
   import global._
   def internalLink(sym: Symbol, site: Symbol): Option[LinkTo]
   def chooseLink(links: List[LinkTo]): LinkTo
+  def toString(link: LinkTo): String
 
   import global._
   import definitions.{ NothingClass, AnyClass, AnyValClass, AnyRefClass, ListClass }
@@ -25,6 +26,25 @@ trait MemberLookupBase {
   def makeEntityLink(title: Inline, pos: Position, query: String, siteOpt: Option[Symbol]) =
     new EntityLink(title) { lazy val link = memberLookup(pos, query, siteOpt) }
 
+  private var showExplanation = true
+  private def explanation: String =
+    if (showExplanation) {
+      showExplanation = false
+      """
+      |Quick crash course on using Scaladoc links
+      |==========================================
+      |Disambiguating terms and types: Prefix terms with '$' and types with '!' in case both names are in use:
+      | - [[scala.collection.immutable.List!.apply class List's apply method]] and
+      | - [[scala.collection.immutable.List$.apply object List's apply method]]
+      |Disambiguating overloaded members: If a term is overloaded, you can indicate the first part of its signature followed by *:
+      | - [[[scala.collection.immutable.List$.fill[A](Int)(⇒A):List[A]* Fill with a single parameter]]]
+      | - [[[scala.collection.immutable.List$.fill[A](Int,Int)(⇒A):List[List[A]]* Fill with a two parameters]]]
+      |Notes:
+      | - you can use any number of matching square brackets to avoid interference with the signature
+      | - you can use \\. to escape dots in prefixes (don't forget to use * at the end to match the signature!)
+      | - you can use \\# to escape hashes, otherwise they will be considered as delimiters, like dots.""".stripMargin
+    } else ""
+
   def memberLookup(pos: Position, query: String, siteOpt: Option[Symbol]): LinkTo = {
     var members = breakMembers(query)
 
@@ -73,36 +93,23 @@ trait MemberLookupBase {
         def linkToString(link: LinkTo) = {
           val chosenInfo =
             if (link == chosen) " [chosen]" else ""
-          link.toString + chosenInfo + "\n"
+          toString(link) + chosenInfo + "\n"
         }
-        if (!settings.docNoLinkWarnings.value)
+        if (!settings.docNoLinkWarnings.value) {
+          val allLinks = links.map(linkToString).mkString
           reporter.warning(pos,
-            "The link target \"" + query + "\" is ambiguous. Several (possibly overloaded) members fit the target:\n" +
-            links.map(linkToString).mkString +
-            (if (MemberLookup.showExplanation)
-              "\n\n" +
-              "Quick crash course on using Scaladoc links\n" +
-              "==========================================\n" +
-              "Disambiguating terms and types: Prefix terms with '$' and types with '!' in case both names are in use:\n" +
-              " - [[scala.collection.immutable.List!.apply class List's apply method]] and\n" +
-              " - [[scala.collection.immutable.List$.apply object List's apply method]]\n" +
-              "Disambiguating overloaded members: If a term is overloaded, you can indicate the first part of its signature followed by *:\n" +
-              " - [[[scala.collection.immutable.List$.fill[A](Int)(⇒A):List[A]* Fill with a single parameter]]]\n" +
-              " - [[[scala.collection.immutable.List$.fill[A](Int,Int)(⇒A):List[List[A]]* Fill with a two parameters]]]\n" +
-              "Notes: \n" +
-              " - you can use any number of matching square brackets to avoid interference with the signature\n" +
-              " - you can use \\. to escape dots in prefixes (don't forget to use * at the end to match the signature!)\n" +
-              " - you can use \\# to escape hashes, otherwise they will be considered as delimiters, like dots.\n"
-            else "")
-        )
+            s"""The link target \"$query\" is ambiguous. Several members fit the target:
+            |$allLinks
+            |$explanation""".stripMargin)
+        }
         chosen
     }
   }
 
-  private abstract class SearchStrategy
-  private object BothTypeAndTerm extends SearchStrategy
-  private object OnlyType extends SearchStrategy
-  private object OnlyTerm extends SearchStrategy
+  private sealed trait SearchStrategy
+  private case object BothTypeAndTerm extends SearchStrategy
+  private case object OnlyType extends SearchStrategy
+  private case object OnlyTerm extends SearchStrategy
 
   private def lookupInRootPackage(pos: Position, members: List[String]) =
     lookupInTemplate(pos, members, EmptyPackage) ::: lookupInTemplate(pos, members, RootPackage)
@@ -156,14 +163,11 @@ trait MemberLookupBase {
     else if (member.endsWith("*"))
       cleanupBogusClasses(container.info.nonPrivateDecls) filter signatureMatch
     else
-      if (strategy == BothTypeAndTerm)
-        termSyms ::: typeSyms
-      else if (strategy == OnlyType)
-        typeSyms
-      else if (strategy == OnlyTerm)
-        termSyms
-      else
-        Nil
+      strategy match {
+        case BothTypeAndTerm => termSyms ::: typeSyms
+        case OnlyType => typeSyms
+        case OnlyTerm => termSyms
+      }
 
     //println("lookupInTemplate(" + member + ", " + container + ") => " + result)
     result
@@ -223,8 +227,3 @@ trait MemberLookupBase {
     (sym.nameString + sym.signatureString).replaceAll("\\s", "")
   }
 }
-
-object MemberLookup {
-  private[this] var _showExplanation = true
-  def showExplanation: Boolean = if (_showExplanation) { _showExplanation = false; true } else false
-}
diff --git a/src/compiler/scala/tools/nsc/doc/model/MemberLookup.scala b/src/compiler/scala/tools/nsc/doc/model/MemberLookup.scala
index b1105196b7..c7a767f992 100644
--- a/src/compiler/scala/tools/nsc/doc/model/MemberLookup.scala
+++ b/src/compiler/scala/tools/nsc/doc/model/MemberLookup.scala
@@ -28,4 +28,11 @@ trait MemberLookup extends base.MemberLookupBase {
     else
       mbrs.min(Ordering[MemberEntity].on[(MemberEntity, LinkTo)](_._1))._2
   }
+
+  override def toString(link: LinkTo) = link match {
+    case LinkToTpl(tpl: EntityImpl) => tpl.sym.toString
+    case LinkToMember(mbr: EntityImpl, inTpl: EntityImpl) =>
+      mbr.sym.signatureString + " in " + inTpl.sym.toString
+    case _ => link.toString
+  }
 }
diff --git a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
index ea074873e8..c6cfc317ea 100644
--- a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
+++ b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
@@ -1093,13 +1093,5 @@ class ModelFactory(val global: Global, val settings: doc.Settings) {
     (bSym.isAliasType || bSym.isAbstractType) &&
     { val rawComment = global.expandedDocComment(bSym, inTpl.sym)
       rawComment.contains("@template") || rawComment.contains("@documentable") }
-
-  object LinkToTpl {
-    // this makes it easier to create links
-    def apply(tpl: TemplateEntity): LinkTo = tpl match {
-      case dtpl: DocTemplateEntity => new LinkToTpl(dtpl)
-      case _ => new Tooltip(tpl.qualifiedName)
-    }
-  }
 }
 
diff --git a/src/compiler/scala/tools/nsc/doc/model/ModelFactoryTypeSupport.scala b/src/compiler/scala/tools/nsc/doc/model/ModelFactoryTypeSupport.scala
index 87dc615c8e..844a509b7e 100644
--- a/src/compiler/scala/tools/nsc/doc/model/ModelFactoryTypeSupport.scala
+++ b/src/compiler/scala/tools/nsc/doc/model/ModelFactoryTypeSupport.scala
@@ -92,7 +92,10 @@ trait ModelFactoryTypeSupport {
             findTemplateMaybe(bSym) match {
               case Some(bTpl) if owner == bSym.owner =>
                 // (0) the owner's class is linked AND has a template - lovely
-                LinkToTpl(bTpl)
+                bTpl match {
+                  case dtpl: DocTemplateEntity => new LinkToTpl(dtpl)
+                  case _ => new Tooltip(bTpl.qualifiedName)
+                }
               case _ =>
                 val oTpl = findTemplateMaybe(owner)
                 (oTpl, oTpl flatMap (findMember(bSym, _))) match {
diff --git a/src/compiler/scala/tools/nsc/interactive/Doc.scala b/src/compiler/scala/tools/nsc/interactive/Doc.scala
index 4d02de6198..ad28a28105 100755
--- a/src/compiler/scala/tools/nsc/interactive/Doc.scala
+++ b/src/compiler/scala/tools/nsc/interactive/Doc.scala
@@ -32,6 +32,15 @@ abstract class Doc(val settings: doc.Settings) extends MemberLookupBase with Com
           None
     }
 
+  override def toString(link: LinkTo) = ask { () =>
+    link match {
+      case LinkToMember(mbr: Symbol, site: Symbol) =>
+        mbr.signatureString + " in " + site.toString
+      case LinkToTpl(sym: Symbol) => sym.toString
+      case _ => link.toString
+    }
+  }
+
   def retrieve(sym: Symbol, site: Symbol): Option[DocResult] = {
     val sig = ask { () => externalSignature(sym) }
     findExternalLink(sym, sig) map { link => UrlResult(link.url) } orElse {