From 386d5068471809d906d3db3aa56ed5f9352250c2 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Wed, 30 Mar 2011 20:40:46 +0000 Subject: 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. --- src/compiler/scala/tools/nsc/ast/DocComments.scala | 86 ++++++++++++---------- .../scala/collection/immutable/StringLike.scala | 15 ++++ src/library/scala/reflect/NameTransformer.scala | 15 ++-- src/library/scala/reflect/generic/Symbols.scala | 4 +- 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 "$op_name". + /** 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 $op_name 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)) -- cgit v1.2.3