Issue warning when doccomments have $variables ...

Issue warning when doccomments have $variables which go unfulfilled.
Started with patch by dmharrah. Noticed expandVariables never
incremented its recursion guard and ended up rewriting it. To avoid
spurious warnings you can escape $'s, as in this comment:

  /** The decoded name of the symbol, e.g. `==` instead of `\$eq\$eq`.
  */

The above will be ignored during expansion and translated to $eq$eq for
output. Closes #4412, no review.

4 files changed, 70 insertions, 50 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/DocComments.scala b/src/compiler/scala/tools/nsc/ast/DocComments.scala
index d9a2c555e6..9af43d406f 100755
--- a/src/compiler/scala/tools/nsc/ast/DocComments.scala
+++ b/src/compiler/scala/tools/nsc/ast/DocComments.scala
@@ -7,6 +7,7 @@ package scala.tools.nsc
 package ast
 
 import symtab._
+import reporters.Reporter
 import util.{Position, NoPosition}
 import util.DocStrings._
 import util.Chars._
@@ -18,6 +19,8 @@ import scala.collection.mutable.{HashMap, ListBuffer, StringBuilder}
  */
 trait DocComments { self: SymbolTable =>
 
+  def reporter: Reporter
+
   /** The raw doc comment map */
   val docComments = new HashMap[Symbol, DocComment]
 
@@ -226,9 +229,6 @@ trait DocComments { self: SymbolTable =>
       else lookInBaseClasses
     }
 
-  private var expandCount = 0
-  private final val expandLimit = 10
-
   /** Expand variable occurrences in string `str', until a fix point is reached or
    *  a expandLimit is exceeded.
    *
@@ -237,49 +237,57 @@ trait DocComments { self: SymbolTable =>
    *  @param site  The class for which doc comments are generated
    *  @return      Expanded string
    */
-  protected def expandVariables(str: String, sym: Symbol, site: Symbol): String =
-    if (expandCount < expandLimit) {
-      try {
-        val out = new StringBuilder
-        var copied = 0
-        var idx = 0
-        while (idx < str.length) {
-          if ((str charAt idx) == '$') {
-            val vstart = idx
-            idx = skipVariable(str, idx + 1)
-            def replaceWith(repl: String) {
-              out append str.substring(copied, vstart)
-              out append repl
-              copied = idx
-            }
-            val vname = variableName(str.substring(vstart + 1, idx))
-            if (vname == "super") {
-              superComment(sym) match {
-                case Some(sc) =>
-                  val superSections = tagIndex(sc)
-                  replaceWith(sc.substring(3, startTag(sc, superSections)))
-                  for (sec @ (start, end) <- superSections)
-                    if (!isMovable(sc, sec)) out append sc.substring(start, end)
-                case None =>
+  protected def expandVariables(initialStr: String, sym: Symbol, site: Symbol): String = {
+    val expandLimit = 10
+
+    def expandInternal(str: String, depth: Int): String = {
+      if (depth >= expandLimit)
+        throw new ExpansionLimitExceeded(str)
+
+      val out         = new StringBuilder
+      var copied, idx = 0
+      // excluding variables written as \$foo so we can use them when
+      // necessary to document things like Symbol#decode
+      def isEscaped = idx > 0 && str.charAt(idx - 1) == '\\'
+      while (idx < str.length) {
+        if ((str charAt idx) != '$' || isEscaped)
+          idx += 1
+        else {
+          val vstart = idx
+          idx = skipVariable(str, idx + 1)
+          def replaceWith(repl: String) {
+            out append str.substring(copied, vstart)
+            out append repl
+            copied = idx
+          }
+          variableName(str.substring(vstart + 1, idx)) match {
+            case "super"    =>
+              superComment(sym) foreach { sc =>
+                val superSections = tagIndex(sc)
+                replaceWith(sc.substring(3, startTag(sc, superSections)))
+                for (sec @ (start, end) <- superSections)
+                  if (!isMovable(sc, sec)) out append sc.substring(start, end)
               }
-            } else if (vname.length > 0) {
+            case "" => idx += 1
+            case vname  =>
               lookupVariable(vname, site) match {
                 case Some(replacement) => replaceWith(replacement)
-                case None =>  //println("no replacement for "+vname) // DEBUG
+                case None              => reporter.warning(sym.pos, "Variable " + vname + " undefined in comment for " + sym)
               }
-            } else idx += 1
-          } else idx += 1
-        }
-        if (out.length == 0) str
-        else {
-          out append str.substring(copied)
-          expandVariables(out.toString, sym, site)
+            }
         }
-      } finally {
-        expandCount -= 1
       }
-    } else throw new ExpansionLimitExceeded(str)
+      if (out.length == 0) str
+      else {
+        out append str.substring(copied)
+        expandInternal(out.toString, depth + 1)
+      }
+    }
 
+    // We suppressed expanding \$ throughout the recursion, and now we
+    // need to replace \$ with $ so it looks as intended.
+    expandInternal(initialStr, 0).replaceAllLiterally("""\$""", "$")
+  }
 
   // !!! todo: inherit from Comment?
   case class DocComment(raw: String, pos: Position = NoPosition) {
diff --git a/src/library/scala/collection/immutable/StringLike.scala b/src/library/scala/collection/immutable/StringLike.scala
index 926fde4fe0..98f1b90866 100644
--- a/src/library/scala/collection/immutable/StringLike.scala
+++ b/src/library/scala/collection/immutable/StringLike.scala
@@ -162,6 +162,21 @@ self =>
     if (toString.endsWith(suffix)) toString.substring(0, toString.length() - suffix.length)
     else toString
 
+  /** Replace all literal occurrences of `literal` with the string `replacement`.
+   *  This is equivalent to [[java.lang.String#replaceAll]] except that both arguments
+   *  are appropriately quoted to avoid being interpreted as metacharacters.
+   *
+   *  @param    literal     the string which should be replaced everywhere it occurs
+   *  @param    replacement the replacement string
+   *  @return               the resulting string
+   */
+  def replaceAllLiterally(literal: String, replacement: String): String = {
+    val arg1 = java.util.regex.Pattern.quote(literal)
+    val arg2 = java.util.regex.Matcher.quoteReplacement(replacement)
+
+    toString.replaceAll(arg1, arg2)
+  }
+
   /**
    *    For every line in this string:
    *
diff --git a/src/library/scala/reflect/NameTransformer.scala b/src/library/scala/reflect/NameTransformer.scala
index 6cfb40cadf..7bc5a61f6c 100755
--- a/src/library/scala/reflect/NameTransformer.scala
+++ b/src/library/scala/reflect/NameTransformer.scala
@@ -6,8 +6,6 @@
 **                          |/                                          **
 \*                                                                      */
 
-
-
 package scala.reflect
 
 /**
@@ -21,7 +19,6 @@ object NameTransformer {
 
   private val op2code = new Array[String](nops)
   private val code2op = new Array[OpCodes](ncodes)
-
   private def enterOp(op: Char, code: String) = {
     op2code(op) = code
     val c = (code.charAt(1) - 'a') * 26 + code.charAt(2) - 'a'
@@ -48,10 +45,10 @@ object NameTransformer {
   enterOp('?', "$qmark")
   enterOp('@', "$at")
 
-  /** Replace operator symbols by corresponding "<code>$op_name</code>".
+  /** Replace operator symbols by corresponding `\$opname`.
    *
-   *  @param name ...
-   *  @return     ...
+   *  @param name the string to encode
+   *  @return     the string with all recognized opchars replaced with their encoding
    */
   def encode(name: String): String = {
     var buf: StringBuilder = null
@@ -82,10 +79,10 @@ object NameTransformer {
     if (buf eq null) name else buf.toString()
   }
 
-  /** Replace <code>$op_name</code> by corresponding operator symbol.
+  /** Replace `\$opname` by corresponding operator symbol.
    *
-   *  @param name0 ...
-   *  @return      ...
+   *  @param name0 the string to decode
+   *  @return      the string with all recognized operator symbol encodings replaced with their name
    */
   def decode(name0: String): String = {
     //System.out.println("decode: " + name);//DEBUG
diff --git a/src/library/scala/reflect/generic/Symbols.scala b/src/library/scala/reflect/generic/Symbols.scala
index 770c6e920c..49cf7df1ef 100755
--- a/src/library/scala/reflect/generic/Symbols.scala
+++ b/src/library/scala/reflect/generic/Symbols.scala
@@ -25,11 +25,11 @@ trait Symbols { self: Universe =>
      */
     def name: Name
 
-    /** The name of the symbol before decoding, e.g. `$eq$eq` instead of `==`.
+    /** The name of the symbol before decoding, e.g. `\$eq\$eq` instead of `==`.
      */
     def encodedName: String
 
-    /** The decoded name of the symbol, e.g. `==` instead of `$eq$eq`.
+    /** The decoded name of the symbol, e.g. `==` instead of `\$eq\$eq`.
      */
     def decodedName: String = stripLocalSuffix(NameTransformer.decode(encodedName))