SI-5235 Correct usecase variable expansion

The bug is related to a couple of other annoyances, also fixed:
 - usecases without type params were crashing scaladoc due to a change
in the PolyTypes class (not allowing empty tparams list)
 - properly getting rid of backticks (even if the link is not valid)
 - correct linking for usecases with $Coll = `immutable.Seq`
(the symbol searching algorithm was too of restrictive, now we search
the entire ownerchain - and the empty package at the end)
 - give a warning if the type lookup fails
 - finally, added a $Coll variable to List, for some reason it wasn't
there and we were getting immutable.Seq as the result of use cases.

8 files changed, 140 insertions, 22 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/DocComments.scala b/src/compiler/scala/tools/nsc/ast/DocComments.scala
index b545140c4a..b2d6800ebb 100755
--- a/src/compiler/scala/tools/nsc/ast/DocComments.scala
+++ b/src/compiler/scala/tools/nsc/ast/DocComments.scala
@@ -457,22 +457,16 @@ trait DocComments { self: Global =>
           case List() => NoType
           case site :: sites1 => select(site.thisType, name, findIn(sites1))
         }
-        val (classes, pkgs) = site.ownerChain.span(!_.isPackageClass)
-        findIn(classes ::: List(pkgs.head, rootMirror.RootClass))
+        // Previously, searching was taking place *only* in the current package and in the root package
+        // now we're looking for it everywhere in the hierarchy, so we'll be able to link variable expansions like
+        // immutable.Seq in package immutable
+        //val (classes, pkgs) = site.ownerChain.span(!_.isPackageClass)
+        //val sites = (classes ::: List(pkgs.head, rootMirror.RootClass)))
+        //findIn(sites)
+        findIn(site.ownerChain ::: List(definitions.EmptyPackage))
       }
 
-      def getType(_str: String, variable: String): Type = {
-        /*
-         * work around the backticks issue suggested by Simon in
-         * https://groups.google.com/forum/?hl=en&fromgroups#!topic/scala-internals/z7s1CCRCz74
-         * ideally, we'd have a removeWikiSyntax method in the CommentFactory to completely eliminate the wiki markup
-         */
-        val str =
-          if (_str.length >= 2 && _str.startsWith("`") && _str.endsWith("`"))
-            _str.substring(1, _str.length - 2)
-          else
-            _str
-
+      def getType(str: String, variable: String): Type = {
         def getParts(start: Int): List[String] = {
           val end = skipIdent(str, start)
           if (end == start) List()
@@ -484,7 +478,7 @@ trait DocComments { self: Global =>
         val parts = getParts(0)
         if (parts.isEmpty) {
           reporter.error(comment.codePos, "Incorrect variable expansion for " + variable + " in use case. Does the " +
-                             "variable expand to wiki syntax when documenting " + site + "?")
+                                          "variable expand to wiki syntax when documenting " + site + "?")
           return ErrorType
         }
         val partnames = (parts.init map newTermName) :+ newTypeName(parts.last)
@@ -498,17 +492,36 @@ trait DocComments { self: Global =>
           case _ =>
             (getSite(partnames.head), partnames.tail)
         }
-        (start /: rest)(select(_, _, NoType))
+        val result = (start /: rest)(select(_, _, NoType))
+        if (result == NoType)
+          reporter.warning(comment.codePos, "Could not find the type " + variable + " points to while expanding it " +
+                                            "for the usecase signature of " + sym + " in " + site + "." +
+                                            "In this context, " + variable + " = \"" + str + "\".")
+        result
+      }
+
+      /**
+       * work around the backticks issue suggested by Simon in
+       * https://groups.google.com/forum/?hl=en&fromgroups#!topic/scala-internals/z7s1CCRCz74
+       * ideally, we'd have a removeWikiSyntax method in the CommentFactory to completely eliminate the wiki markup
+       */
+      def cleanupVariable(str: String) = {
+        val tstr = str.trim
+        if (tstr.length >= 2 && tstr.startsWith("`") && tstr.endsWith("`"))
+          tstr.substring(1, tstr.length - 1)
+        else
+          tstr
       }
 
       val aliasExpansions: List[Type] =
         for (alias <- aliases) yield
           lookupVariable(alias.name.toString.substring(1), site) match {
             case Some(repl) =>
-              val tpe = getType(repl.trim, alias.name.toString)
+              val repl2 = cleanupVariable(repl)
+              val tpe = getType(repl2, alias.name.toString)
               if (tpe != NoType) tpe
               else {
-                val alias1 = alias.cloneSymbol(rootMirror.RootClass, alias.rawflags, newTypeName(repl))
+                val alias1 = alias.cloneSymbol(rootMirror.RootClass, alias.rawflags, newTypeName(repl2))
                 typeRef(NoPrefix, alias1, Nil)
               }
             case None =>
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index a570cd74d6..2d277603ee 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1972,7 +1972,10 @@ trait Typers extends Modes with Adaptations with Tags {
                 case SilentResultValue(tpt) =>
                   val alias = enclClass.newAliasType(name.toTypeName, useCase.pos)
                   val tparams = cloneSymbolsAtOwner(tpt.tpe.typeSymbol.typeParams, alias)
-                  alias setInfo typeFun(tparams, appliedType(tpt.tpe, tparams map (_.tpe)))
+                  /* Unless we treat no-tparams usecases differently they blow up in typeFun
+                   * def typeFun = PolyType(tparams, tpe) // <- which asserts (!tparams.isEmpty) */
+                  val newInfo = if (tparams.isEmpty) tpt.tpe else typeFun(tparams, appliedType(tpt.tpe, tparams map (_.tpe)))
+                  alias setInfo newInfo
                   context.scope.enter(alias)
                 case _ =>
               }
diff --git a/src/library/scala/collection/immutable/List.scala b/src/library/scala/collection/immutable/List.scala
index 6fd8d143ee..74dc385f99 100644
--- a/src/library/scala/collection/immutable/List.scala
+++ b/src/library/scala/collection/immutable/List.scala
@@ -62,6 +62,7 @@ import java.io._
  *  section on `Lists` for more information.
  *
  *  @define coll list
+ *  @define Coll `List`
  *  @define thatinfo the class of the returned collection. In the standard library configuration,
  *    `That` is always `List[B]` because an implicit of type `CanBuildFrom[List, B, That]`
  *    is defined in object `List`.
@@ -96,7 +97,7 @@ sealed abstract class List[+A] extends AbstractSeq[A]
    *
    *  @usecase def ::(x: A): List[A]
    *    @inheritdoc
-   *    
+   *
    *    Example:
    *    {{{1 :: List(2, 3) = List(2, 3).::(1) = List(1, 2, 3)}}}
    */
diff --git a/src/library/scala/collection/immutable/StringOps.scala b/src/library/scala/collection/immutable/StringOps.scala
index 633821ecea..7e60cc7195 100644
--- a/src/library/scala/collection/immutable/StringOps.scala
+++ b/src/library/scala/collection/immutable/StringOps.scala
@@ -25,7 +25,7 @@ import mutable.StringBuilder
  *  @param repr     the actual representation of this string operations object.
  *
  *  @since 2.8
- *  @define Coll `StringOps`
+ *  @define Coll `String`
  *  @define coll string
  */
 final class StringOps(override val repr: String) extends AnyVal with StringLike[String] {
diff --git a/src/library/scala/collection/mutable/ArrayOps.scala b/src/library/scala/collection/mutable/ArrayOps.scala
index 7a595f211d..21c2aaaec7 100644
--- a/src/library/scala/collection/mutable/ArrayOps.scala
+++ b/src/library/scala/collection/mutable/ArrayOps.scala
@@ -30,7 +30,7 @@ import parallel.mutable.ParArray
  *
  *  @tparam T   type of the elements contained in this array.
  *
- *  @define Coll `ArrayOps`
+ *  @define Coll `Array`
  *  @define orderDependent
  *  @define orderDependentFold
  *  @define mayNotTerminateInf
diff --git a/src/partest/scala/tools/partest/ScaladocModelTest.scala b/src/partest/scala/tools/partest/ScaladocModelTest.scala
index de5354d4a0..be70a91e14 100644
--- a/src/partest/scala/tools/partest/ScaladocModelTest.scala
+++ b/src/partest/scala/tools/partest/ScaladocModelTest.scala
@@ -12,6 +12,7 @@ import scala.tools.nsc.util.CommandLineParser
 import scala.tools.nsc.doc.{Settings, DocFactory, Universe}
 import scala.tools.nsc.doc.model._
 import scala.tools.nsc.reporters.ConsoleReporter
+import scala.tools.nsc.doc.model.comment.Comment
 
 /** A class for testing scaladoc model generation
  *   - you need to specify the code in the `code` method
@@ -142,5 +143,14 @@ abstract class ScaladocModelTest extends DirectTest {
       case _ => sys.error("Error getting " + expl + ": " + list.length + " elements with this name. " +
                   "All elements in list: [" + list.mkString(", ") + "]")
     }
+
+    def extractCommentText(c: Comment) = {
+      def extractText(body: Any): String = body match {
+        case s: String  => s
+        case p: Product => p.productIterator.toList.map(extractText(_)).mkString
+        case _          => ""
+      }
+      extractText(c.body)
+    }
   }
 }
diff --git a/test/scaladoc/run/SI-5235.check b/test/scaladoc/run/SI-5235.check
new file mode 100644
index 0000000000..d9acfd063b
--- /dev/null
+++ b/test/scaladoc/run/SI-5235.check
@@ -0,0 +1,4 @@
+newSource:10: warning: Could not find the type $Coll points to while expanding it for the usecase signature of method reverse in trait SpecificColl.In this context, $Coll = "BullSh".
+             * @usecase def reverse(): $Coll
+                        ^
+Done.
diff --git a/test/scaladoc/run/SI-5235.scala b/test/scaladoc/run/SI-5235.scala
new file mode 100644
index 0000000000..cae70fd0a5
--- /dev/null
+++ b/test/scaladoc/run/SI-5235.scala
@@ -0,0 +1,87 @@
+import scala.tools.nsc.doc.model._
+import scala.tools.nsc.doc.model.diagram._
+import scala.tools.partest.ScaladocModelTest
+
+object Test extends ScaladocModelTest {
+
+  override def code = """
+        package scala.test.scaladoc.SI5235 {
+          trait Builder[From, To]
+
+          /**
+           * @define Coll `GenericColl`
+           */
+          class GenericColl {
+            /**
+             * @usecase def reverse(): $Coll
+             * Returns the reversed $Coll.
+             */
+            def reverse[T](implicit something: Builder[GenericColl, T]): T
+            def foo1: GenericColl = ???
+          }
+
+          /** Nooo, don't point to this */
+          trait MyCollection
+
+          package specific {
+            /**
+             * @define Coll `BullSh`
+             */
+            trait SpecificColl extends GenericColl {
+              def foo2: SpecificColl = ???
+            }
+          }
+
+          package mycoll {
+            /**
+             * @define Coll `mycoll.MyCollection`
+             */
+            class MyCollection extends specific.SpecificColl {
+              def foo3: MyCollection = ???
+            }
+          }
+        }
+    """
+
+  // diagrams must be started. In case there's an error with dot, it should not report anything
+  def scaladocSettings = ""
+
+  def testModel(rootPackage: Package) = {
+    // get the quick access implicit defs in scope (_package(s), _class(es), _trait(s), object(s) _method(s), _value(s))
+    import access._
+
+    val base = rootPackage._package("scala")._package("test")._package("scaladoc")._package("SI5235")
+
+    val GenericColl = base._class("GenericColl")
+    val SpecificColl = base._package("specific")._trait("SpecificColl")
+    val MyCollection = base._package("mycoll")._class("MyCollection")
+
+    // check comment text
+    val gcComment = extractCommentText(GenericColl._method("reverse").comment.get)
+    val scComment = extractCommentText(SpecificColl._method("reverse").comment.get)
+    val mcComment = extractCommentText(MyCollection._method("reverse").comment.get)
+    assert(gcComment.contains("Returns the reversed GenericColl."),
+           gcComment + ".contains(\"Returns the reversed GenericColl.\")")
+    assert(scComment.contains("Returns the reversed BullSh."),
+           scComment + ".contains(\"Returns the reversed BullSh.\")")
+    assert(mcComment.contains("Returns the reversed mycoll.MyCollection."),
+           mcComment + ".contains(\"Returns the reversed mycoll.MyCollection.\")")
+
+    // check signatures
+    val gcReverse = GenericColl._method("reverse")
+    val scReverse = SpecificColl._method("reverse")
+    val mcReverse = MyCollection._method("reverse")
+    val gcReverseType = gcReverse.resultType
+    val scReverseType = scReverse.resultType
+    val mcReverseType = mcReverse.resultType
+    assert(gcReverseType.name == "GenericColl", gcReverseType.name + " == GenericColl")
+    assert(scReverseType.name == "BullSh",      scReverseType.name + " == BullSh")
+    assert(mcReverseType.name == "MyCollection",mcReverseType.name + " == MyCollection")
+    assert(gcReverseType.refEntity(0)._1 == GenericColl,
+           gcReverse.qualifiedName + "'s return type has a link to " + GenericColl.qualifiedName)
+    assert(scReverseType.refEntity.isEmpty,
+           scReverse.qualifiedName + "'s return type does not have links")
+    assert(mcReverseType.refEntity(0)._1 == MyCollection,
+           mcReverse.qualifiedName + "'s return type has a link to " + MyCollection.qualifiedName)
+  }
+}
\ No newline at end of file