Permalink
Browse files

SI-7376 Scaladoc warns when discarding local doc comments with API tags

Double-star doc comments in non-dockable positions at the end of a block
will emit a warning only if API tags like @author are present, or under
-Xlint.

A real comment parser is applied early to probe for tags, to minimize
ad hoc testing or duplication, but warnings are suppressed. Residual
ad hockiness lies in precisely which tags to warn on.  Ad hoc or ad doc.

This fix is a stop gap; a richer solution would also report about other
doc locations that won't be processed.
  • Loading branch information...
1 parent f6323d8 commit 0fde95e6a4c0659a59519399573ece0e684be660 @som-snytt som-snytt committed Apr 15, 2013
@@ -9,7 +9,6 @@ package doc
import scala.tools.nsc.ast.parser.{ SyntaxAnalyzer, BracePatch }
import scala.reflect.internal.Chars._
import symtab._
-import reporters.Reporter
import typechecker.Analyzer
import scala.reflect.internal.util.{ BatchSourceFile, RangePosition }
@@ -154,9 +153,46 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax
private var docPos: Position = NoPosition // last doc comment position
private var inDocComment = false
+ private lazy val unmooredParser = { // minimalist comment parser
+ import scala.tools.nsc.doc.base.{comment => _, _}
+ new {
+ val global: Global = ScaladocSyntaxAnalyzer.this.global
+ } with CommentFactoryBase with MemberLookupBase {
+ import global.{ settings, Symbol }
+ def parseComment(comment: DocComment) = {
+ val nowarnings = settings.nowarn.value
+ settings.nowarn.value = true
+ try parseAtSymbol(comment.raw, comment.raw, comment.pos)
+ finally settings.nowarn.value = nowarnings
+ }
+
+ override def internalLink(sym: Symbol, site: Symbol): Option[LinkTo] = None
+ override def chooseLink(links: List[LinkTo]): LinkTo = links.headOption orNull
+ override def toString(link: LinkTo): String = "No link"
+ override def findExternalLink(sym: Symbol, name: String): Option[LinkToExternal] = None
+ override def warnNoLink: Boolean = false
+ }
+ }
+
+ /**
+ * Warn when discarding buffered doc at the end of a block.
+ * This mechanism doesn't warn about arbitrary unmoored doc.
+ * Also warn under -Xlint, but otherwise only warn in the presence of suspicious
+ * tags that appear to be documenting API. Warnings are suppressed while parsing
+ * the local comment so that comments of the form `[at] Martin` will not trigger a warning.
+ * In addition, tags for `todo`, `note` and `example` are ignored.
+ */
override def discardDocBuffer() = {
+ import scala.tools.nsc.doc.base.comment.Comment
val doc = flushDoc
- if (doc ne null)
+ // tags that make a local double-star comment look unclean, as though it were API
+ def unclean(comment: Comment): Boolean = {
+ import comment._
+ authors.nonEmpty || result.nonEmpty || throws.nonEmpty || valueParams.nonEmpty ||
+ typeParams.nonEmpty || version.nonEmpty || since.nonEmpty
+ }
+ def isDirty = unclean(unmooredParser parseComment doc)
+ if ((doc ne null) && (settings.lint || isDirty))
unit.warning(docPos, "discarding unmoored doc comment")
}
@@ -1,11 +1,5 @@
-newSource1:17: warning: discarding unmoored doc comment
- /** Testing 123 */
- ^
-newSource1:27: warning: discarding unmoored doc comment
- /** Calculate this result. */
- ^
-newSource1:34: warning: discarding unmoored doc comment
- /** Another digit is a giveaway. */
+newSource1:47: warning: discarding unmoored doc comment
+ /** Document this crucial constant for posterity.
^
[[syntax trees at end of parser]] // newSource1
package <empty> {
@@ -48,6 +42,18 @@ package <empty> {
val test4 = 'a' match {
case ('0'| '1'| '2'| '3'| '4'| '5'| '6'| '7'| '8'| '9') => true
case _ => false
+ };
+ def test5: scala.Unit = if (true)
+ $qmark$qmark$qmark
+ else
+ ();
+ def test6 = {
+ val u = 4;
+ 0.to(u).foreach(((i) => println(i)))
+ };
+ def test7 = {
+ val u = 4;
+ 0.to(u).foreach(((i) => println(i)))
}
};
/** comments that we should keep */
@@ -49,6 +49,29 @@ object Test extends DirectTest {
case _ =>
false
}
+
+ def test5 {
+ /** @martin is this right? It shouldn't flag me as scaladoc. */
+ if (true) ???
+ }
+
+ def test6 = {
+ /** Document this crucial constant for posterity.
+ * Don't forget to dedoc this comment if you refactor to a local.
+ * @author Paul Phillips
+ */
+ val u = 4
+ for (i <- 0 to u)
+ println(i)
+ }
+ def test7 = {
+ /** Some standard tags are tolerated locally and shouldn't trigger a warning.
+ * @note Don't change this unless you know what you're doing. This means you.
+ */
+ val u = 4
+ for (i <- 0 to u)
+ println(i)
+ }
}
/** comments that we should keep */

0 comments on commit 0fde95e

Please sign in to comment.