From 0fde95e6a4c0659a59519399573ece0e684be660 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 15 Apr 2013 16:27:12 -0700 Subject: 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. --- test/scaladoc/run/t5527.check | 22 ++++++++++++++-------- test/scaladoc/run/t5527.scala | 23 +++++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) (limited to 'test/scaladoc') diff --git a/test/scaladoc/run/t5527.check b/test/scaladoc/run/t5527.check index ab2aeb2d67..54595652cd 100644 --- a/test/scaladoc/run/t5527.check +++ b/test/scaladoc/run/t5527.check @@ -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 { @@ -48,6 +42,18 @@ package { 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 */ diff --git a/test/scaladoc/run/t5527.scala b/test/scaladoc/run/t5527.scala index 2449ff60c3..af0c1f8d36 100644 --- a/test/scaladoc/run/t5527.scala +++ b/test/scaladoc/run/t5527.scala @@ -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 */ -- cgit v1.2.3 From 3f0a90bd53cd1fc85891cf2673318c3a91f713bc Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 17 Apr 2013 13:11:18 -0700 Subject: SI-7376 Unmoored doc has correct position The unmoored DocComment is created more eagerly so that its position is correct despite subsequent line comments. (Previously, skipComment would advance docPos.) It looks like the error caret is still off by one when a doc comment shows up in the middle of an operator, and who doesn't scaladoc the interior of expressions? Another bug fixed by Paul's refactor is that additional comments between the doc and the entity no longer breaks the scaladoc. Test added. --- .../scala/tools/nsc/doc/ScaladocAnalyzer.scala | 41 +++++++++------------- test/scaladoc/run/t5527.check | 20 ++++++++++- test/scaladoc/run/t5527.scala | 25 +++++++++++++ 3 files changed, 61 insertions(+), 25 deletions(-) (limited to 'test/scaladoc') diff --git a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala index 99a6ab9e3e..003c439f65 100644 --- a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala +++ b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala @@ -149,9 +149,9 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax class ScaladocUnitScanner(unit0: CompilationUnit, patches0: List[BracePatch]) extends UnitScanner(unit0, patches0) { - private var docBuffer: StringBuilder = null // buffer for comments - private var docPos: Position = NoPosition // last doc comment position - private var inDocComment = false + private var docBuffer: StringBuilder = null // buffer for comments (non-null while scanning) + private var inDocComment = false // if buffer contains double-star doc comment + private var lastDoc: DocComment = null // last comment if it was double-star doc private lazy val unmooredParser = { // minimalist comment parser import scala.tools.nsc.doc.base.{comment => _, _} @@ -180,7 +180,7 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax * 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. + * By omission, tags for `see`, `todo`, `note` and `example` are ignored. */ override def discardDocBuffer() = { import scala.tools.nsc.doc.base.comment.Comment @@ -193,13 +193,10 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax } def isDirty = unclean(unmooredParser parseComment doc) if ((doc ne null) && (settings.lint || isDirty)) - unit.warning(docPos, "discarding unmoored doc comment") + unit.warning(doc.pos, "discarding unmoored doc comment") } - override def flushDoc(): DocComment = { - if (docBuffer eq null) null - else try DocComment(docBuffer.toString, docPos) finally docBuffer = null - } + override def flushDoc(): DocComment = (try lastDoc finally lastDoc = null) override protected def putCommentChar() { if (inDocComment) @@ -218,23 +215,19 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax super.skipBlockComment() } override def skipComment(): Boolean = { - super.skipComment() && { - if (docBuffer ne null) { - if (inDocComment) - foundDocComment(docBuffer.toString, offset, charOffset - 2) - else - try foundComment(docBuffer.toString, offset, charOffset - 2) finally docBuffer = null - } + // emit a block comment; if it's double-star, make Doc at this pos + def foundStarComment(start: Int, end: Int) = try { + val str = docBuffer.toString + val pos = new RangePosition(unit.source, start, start, end) + unit.comment(pos, str) + if (inDocComment) + lastDoc = DocComment(str, pos) true + } finally { + docBuffer = null + inDocComment = false } - } - def foundComment(value: String, start: Int, end: Int) { - val pos = new RangePosition(unit.source, start, start, end) - unit.comment(pos, value) - } - def foundDocComment(value: String, start: Int, end: Int) { - docPos = new RangePosition(unit.source, start, start, end) - unit.comment(docPos, value) + super.skipComment() && ((docBuffer eq null) || foundStarComment(offset, charOffset - 2)) } } class ScaladocUnitParser(unit: CompilationUnit, patches: List[BracePatch]) extends UnitParser(unit, patches) { diff --git a/test/scaladoc/run/t5527.check b/test/scaladoc/run/t5527.check index 54595652cd..235e73b5c8 100644 --- a/test/scaladoc/run/t5527.check +++ b/test/scaladoc/run/t5527.check @@ -1,6 +1,12 @@ newSource1:47: warning: discarding unmoored doc comment /** Document this crucial constant for posterity. ^ +newSource1:64: warning: discarding unmoored doc comment + /*************************\ + ^ +newSource1:73: warning: discarding unmoored doc comment + val i = 10 */** Important! + ^ [[syntax trees at end of parser]] // newSource1 package { object UselessComments extends scala.AnyRef { @@ -54,6 +60,14 @@ package { def test7 = { val u = 4; 0.to(u).foreach(((i) => println(i))) + }; + def test8 = { + val z = "fancy"; + z.replace("fanc", "arts") + }; + def test9 = { + val i = 10.$times(10); + assert(i.$eq$eq(100)) } }; /** comments that we should keep */ @@ -108,7 +122,11 @@ package { super.(); () } - } + }; + /** Get the simple value. + * @return the default value + */ + def value: Int = 7 } } diff --git a/test/scaladoc/run/t5527.scala b/test/scaladoc/run/t5527.scala index af0c1f8d36..60ae23c1a7 100644 --- a/test/scaladoc/run/t5527.scala +++ b/test/scaladoc/run/t5527.scala @@ -72,6 +72,24 @@ object Test extends DirectTest { for (i <- 0 to u) println(i) } + def test8 = { + /*************************\ + * Fancy ASCII Art Block * + * @author som-snytt * + \*************************/ + // this is just a local + val z = "fancy" + z replace ("fanc", "arts") + } + def test9 = { + val i = 10 */** Important! + * We have to multiply here! + * @author community + * @see SI-1234 + */ + 10 + assert(i == 100) + } } /** comments that we should keep */ @@ -108,6 +126,13 @@ object Test extends DirectTest { /** class D */ @deprecated("use ... instead", "2.10.0") class D + + /** Get the simple value. + * @return the default value + */ + // an intervening line comment + /* I had more to say, but didn't want to pollute the scaladoc. */ + def value: Int = 7 } """.trim -- cgit v1.2.3