Merge pull request #5469 from adriaanm/java-scan-tailrec

No StackOverflowError in Java doc comment scanning

Fixes SI-10020 SI-10027 

3 files changed, 70 insertions, 104 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala b/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala
index 891858ba7b..3a659fd0f0 100644
--- a/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala
@@ -38,6 +38,27 @@ trait ScannersCommon {
     def deprecationWarning(off: Offset, msg: String, since: String): Unit
   }
 
+  // Hooks for ScaladocUnitScanner and ScaladocJavaUnitScanner
+  trait DocScanner {
+    protected def beginDocComment(prefix: String): Unit = {}
+    protected def processCommentChar(): Unit = {}
+    protected def finishDocComment(): Unit = {}
+
+    private var lastDoc: DocComment = null
+    // get last doc comment
+    def flushDoc(): DocComment = try lastDoc finally lastDoc = null
+    def registerDocComment(raw: String, pos: Position) = {
+      lastDoc = DocComment(raw, pos)
+      signalParsedDocComment(raw, pos)
+    }
+
+    /** To prevent doc comments attached to expressions from leaking out of scope
+      *  onto the next documentable entity, they are discarded upon passing a right
+      *  brace, bracket, or parenthesis.
+      */
+    def discardDocBuffer(): Unit = {}
+  }
+
   def createKeywordArray(keywords: Seq[(Name, Token)], defaultToken: Token): (Token, Array[Token]) = {
     val names = keywords sortBy (_._1.start) map { case (k, v) => (k.start, v) }
     val low   = names.head._1
@@ -103,11 +124,11 @@ trait Scanners extends ScannersCommon {
     }
   }
 
-  abstract class Scanner extends CharArrayReader with TokenData with ScannerData with ScannerCommon {
+  abstract class Scanner extends CharArrayReader with TokenData with ScannerData with ScannerCommon with DocScanner {
     private def isDigit(c: Char) = java.lang.Character isDigit c
 
     private var openComments = 0
-    protected def putCommentChar(): Unit = nextChar()
+    final protected def putCommentChar(): Unit = { processCommentChar(); nextChar() }
 
     @tailrec private def skipLineComment(): Unit = ch match {
       case SU | CR | LF =>
@@ -134,8 +155,6 @@ trait Scanners extends ScannersCommon {
       case SU  => incompleteInputError("unclosed comment")
       case _   => putCommentChar() ; skipNestedComments()
     }
-    def skipDocComment(): Unit = skipNestedComments()
-    def skipBlockComment(): Unit = skipNestedComments()
 
     private def skipToCommentEnd(isLineComment: Boolean): Unit = {
       nextChar()
@@ -147,27 +166,23 @@ trait Scanners extends ScannersCommon {
           // Check for the amazing corner case of /**/
           if (ch == '/')
             nextChar()
-          else
-            skipDocComment()
+          else {
+            beginDocComment("/**")
+            skipNestedComments()
+          }
         }
-        else skipBlockComment()
+        else skipNestedComments()
       }
     }
 
     /** @pre ch == '/'
      *  Returns true if a comment was skipped.
      */
-    def skipComment(): Boolean = ch match {
-      case '/' | '*' => skipToCommentEnd(isLineComment = ch == '/') ; true
+    final def skipComment(): Boolean = ch match {
+      case '/' | '*' => skipToCommentEnd(isLineComment = ch == '/') ; finishDocComment(); true
       case _         => false
     }
-    def flushDoc(): DocComment = null
 
-    /** To prevent doc comments attached to expressions from leaking out of scope
-     *  onto the next documentable entity, they are discarded upon passing a right
-     *  brace, bracket, or parenthesis.
-     */
-    def discardDocBuffer(): Unit = ()
 
     def isAtEnd = charOffset >= buf.length
 
diff --git a/src/compiler/scala/tools/nsc/javac/JavaScanners.scala b/src/compiler/scala/tools/nsc/javac/JavaScanners.scala
index e11ac94041..f77e53c54b 100644
--- a/src/compiler/scala/tools/nsc/javac/JavaScanners.scala
+++ b/src/compiler/scala/tools/nsc/javac/JavaScanners.scala
@@ -215,7 +215,7 @@ trait JavaScanners extends ast.parser.ScannersCommon {
    *
    *  @author     Martin Odersky
    */
-  abstract class JavaScanner extends AbstractJavaScanner with JavaTokenData with Cloneable with ScannerCommon {
+  abstract class JavaScanner extends AbstractJavaScanner with JavaTokenData with Cloneable with ScannerCommon with DocScanner {
     override def intVal = super.intVal// todo: needed?
     override def floatVal = super.floatVal
     def currentPos: Position = g2p(pos - 1)
@@ -577,27 +577,32 @@ trait JavaScanners extends ast.parser.ScannersCommon {
       }
     }
 
-    protected def putCommentChar(): Unit = in.next()
+    final protected def putCommentChar(): Unit = { processCommentChar(); in.next() }
 
-    protected def skipBlockComment(isDoc: Boolean): Unit = in.ch match {
-      case SU  => incompleteInputError("unclosed comment")
-      case '*' => putCommentChar() ; if (in.ch == '/') putCommentChar() else skipBlockComment(isDoc)
-      case _   => putCommentChar() ; skipBlockComment(isDoc)
+    @tailrec final protected def skipBlockComment(isDoc: Boolean): Unit = {
+      if (isDoc) beginDocComment("/*") // the second '*' is the current character
+
+      in.ch match {
+        case SU  => incompleteInputError("unclosed comment")
+        case '*' => putCommentChar() ; if (in.ch == '/') putCommentChar() else skipBlockComment(isDoc)
+        case _   => putCommentChar() ; skipBlockComment(isDoc)
+      }
     }
 
-    protected def skipLineComment(): Unit = in.ch match {
+    @tailrec final protected def skipLineComment(): Unit = in.ch match {
       case CR | LF | SU =>
       case _            => putCommentChar() ; skipLineComment()
     }
 
-    protected def skipComment(): Boolean = in.ch match {
-      case '/' => putCommentChar() ; skipLineComment() ; true
+    final protected def skipComment(): Boolean = in.ch match {
+      case '/' => putCommentChar() ; skipLineComment() ; finishDocComment() ; true
       case '*' =>
         putCommentChar()
         in.ch match {
           case '*' => skipBlockComment(isDoc = true)
           case _ => skipBlockComment(isDoc = false)
         }
+        finishDocComment()
         true
       case _   => false
     }
diff --git a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala
index d8ec7b18fd..4e99434051 100644
--- a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala
+++ b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala
@@ -101,12 +101,26 @@ trait ScaladocAnalyzer extends Analyzer {
 abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends SyntaxAnalyzer {
   import global._
 
-  class ScaladocUnitScanner(unit0: CompilationUnit, patches0: List[BracePatch]) extends UnitScanner(unit0, patches0) {
-
-    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
+  trait ScaladocScanner extends DocScanner {
+    // When `docBuffer == null`, we're not in a doc comment.
+    private var docBuffer: StringBuilder = null
+
+    override protected def beginDocComment(prefix: String): Unit =
+      if (docBuffer == null) docBuffer = new StringBuilder(prefix)
+
+    protected def ch: Char
+    override protected def processCommentChar(): Unit =
+      if (docBuffer != null) docBuffer append ch
+
+    protected def docPosition: Position
+    override protected def finishDocComment(): Unit =
+      if (docBuffer != null) {
+        registerDocComment(docBuffer.toString, docPosition)
+        docBuffer = null
+      }
+  }
 
+  class ScaladocUnitScanner(unit0: CompilationUnit, patches0: List[BracePatch]) extends UnitScanner(unit0, patches0) with ScaladocScanner {
     private object unmooredParser extends {                // minimalist comment parser
       val global: Global = ScaladocSyntaxAnalyzer.this.global
     }
@@ -148,40 +162,7 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax
         reporter.warning(doc.pos, "discarding unmoored doc comment")
     }
 
-    override def flushDoc(): DocComment = (try lastDoc finally lastDoc = null)
-
-    override protected def putCommentChar() {
-      if (inDocComment)
-        docBuffer append ch
-
-      nextChar()
-    }
-    override def skipDocComment(): Unit = {
-      inDocComment = true
-      docBuffer = new StringBuilder("/**")
-      super.skipDocComment()
-    }
-    override def skipBlockComment(): Unit = {
-      inDocComment = false // ??? this means docBuffer won't receive contents of this comment???
-      docBuffer = new StringBuilder("/*")
-      super.skipBlockComment()
-    }
-    override def skipComment(): Boolean = {
-      // 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 = Position.range(unit.source, start, start, end)
-        if (inDocComment) {
-          signalParsedDocComment(str, pos)
-          lastDoc = DocComment(str, pos)
-        }
-        true
-      } finally {
-        docBuffer    = null
-        inDocComment = false
-      }
-      super.skipComment() && ((docBuffer eq null) || foundStarComment(offset, charOffset - 2))
-    }
+    protected def docPosition: Position = Position.range(unit.source, offset, offset, charOffset - 2)
   }
   class ScaladocUnitParser(unit: CompilationUnit, patches: List[BracePatch]) extends UnitParser(unit, patches) {
     override def newScanner() = new ScaladocUnitScanner(unit, patches)
@@ -214,52 +195,17 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax
     }
   }
 
-  class ScaladocJavaUnitScanner(unit: CompilationUnit) extends JavaUnitScanner(unit) {
-
-    private var docBuffer: StringBuilder = _
-    private var inDocComment = false
+  class ScaladocJavaUnitScanner(unit: CompilationUnit) extends JavaUnitScanner(unit) with ScaladocScanner {
     private var docStart: Int = 0
-    private var lastDoc: DocComment = null
-
-    override def init() = {
-      docBuffer = new StringBuilder
-      super.init()
-    }
-
-    // get last doc comment
-    def flushDoc(): DocComment = try lastDoc finally lastDoc = null
-
-    override protected def putCommentChar(): Unit = {
-      if (inDocComment) docBuffer append in.ch
-      in.next
-    }
 
-    override protected def skipBlockComment(isDoc: Boolean): Unit = {
-      // condition is true when comment is entered the first time,
-      // i.e. immediately after "/*" and when current character is "*"
-      if (!inDocComment && isDoc) {
-        docBuffer append "/*"
-        docStart = currentPos.start
-        inDocComment = true
-      }
-      super.skipBlockComment(isDoc)
+    override protected def beginDocComment(prefix: String): Unit = {
+      super.beginDocComment(prefix)
+      docStart = currentPos.start
     }
 
-    override protected def skipComment(): Boolean = {
-      val skipped = super.skipComment()
-      if (skipped && inDocComment) {
-        val raw = docBuffer.toString
-        val position = Position.range(unit.source, docStart, docStart, in.cpos)
-        lastDoc = DocComment(raw, position)
-        signalParsedDocComment(raw, position)
-        docBuffer.setLength(0) // clear buffer
-        inDocComment = false
-        true
-      } else {
-        skipped
-      }
-    }
+    protected def ch = in.ch
 
+    override protected def docPosition = Position.range(unit.source, docStart, docStart, in.cpos)
   }
 
   class ScaladocJavaUnitParser(unit: CompilationUnit) extends {