Merge pull request scala#5332 from retronym/review/5304

Fixes to Java source support in Scaladoc

Author: adriaanm

src/compiler/scala/tools/nsc/ast/TreeInfo.scala

@@ -97,4 +97,12 @@ abstract class TreeInfo extends scala.reflect.internal.TreeInfo {
     case DocDef(_, definition) => isPureDef(definition)
     case _ => super.isPureDef(tree)
   }
+
+  override def firstConstructor(stats: List[Tree]): Tree = {
+    def unwrap(stat: Tree): Tree = stat match {
+      case DocDef(_, defn) => unwrap(defn)
+      case tree => tree
+    }
+    super.firstConstructor(stats map unwrap)
+  }
 }

src/compiler/scala/tools/nsc/javac/JavaParsers.scala

@@ -117,11 +117,8 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
       atPos(pkg.pos) {  PackageDef(pkg, stats) }
 
     def makeTemplate(parents: List[Tree], stats: List[Tree]) =
-      Template(
-        parents,
-        noSelfType,
-        if (treeInfo.firstConstructor(stats) == EmptyTree) makeConstructor(List()) :: stats
-        else stats)
+      Template(parents, noSelfType, if (treeInfo.firstConstructor(stats) == EmptyTree)
+        makeConstructor(Nil) :: stats else stats)
 
     def makeSyntheticParam(count: Int, tpt: Tree): ValDef =
       makeParam(nme.syntheticParamName(count), tpt)
@@ -586,7 +583,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
       case CLASS | ENUM | INTERFACE | AT =>
         typeDecl(if (definesInterface(parentToken)) mods | Flags.STATIC else mods)
       case _ =>
-        joinComment(termDecl(mods, parentToken))
+        termDecl(mods, parentToken)
     }
 
     def makeCompanionObject(cdef: ClassDef, statics: List[Tree]): Tree =
@@ -600,26 +597,8 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
         Import(Ident(cdef.name.toTermName), ImportSelector.wildList)
       }
 
-    // Importing the companion object members cannot be done uncritically: see
-    // ticket #2377 wherein a class contains two static inner classes, each of which
-    // has a static inner class called "Builder" - this results in an ambiguity error
-    // when each performs the import in the enclosing class's scope.
-    //
-    // To address this I moved the import Companion._ inside the class, as the first
-    // statement.  This should work without compromising the enclosing scope, but may (?)
-    // end up suffering from the same issues it does in scala - specifically that this
-    // leaves auxiliary constructors unable to access members of the companion object
-    // as unqualified identifiers.
-    def addCompanionObject(statics: List[Tree], cdef: ClassDef): List[Tree] = {
-      def implWithImport(importStmt: Tree) = deriveTemplate(cdef.impl)(importStmt :: _)
-      // if there are no statics we can use the original cdef, but we always
-      // create the companion so import A._ is not an error (see ticket #1700)
-      val cdefNew =
-        if (statics.isEmpty) cdef
-        else deriveClassDef(cdef)(_ => implWithImport(importCompanionObject(cdef)))
-
-      List(makeCompanionObject(cdefNew, statics), cdefNew)
-    }
+    def addCompanionObject(statics: List[Tree], cdef: ClassDef): List[Tree] =
+      List(makeCompanionObject(cdef, statics), cdef)
 
     def importDecl(): List[Tree] = {
       accept(IMPORT)
@@ -726,8 +705,15 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
           in.nextToken()
         } else {
           if (in.token == ENUM || definesInterface(in.token)) mods |= Flags.STATIC
-          val decls = memberDecl(mods, parentToken)
-          (if (mods.hasStaticFlag || inInterface && !(decls exists (_.isInstanceOf[DefDef])))
+          val decls = joinComment(memberDecl(mods, parentToken))
+
+          def isDefDef(tree: Tree): Boolean = tree match {
+            case _: DefDef => true
+            case DocDef(_, defn) => isDefDef(defn)
+            case _ => false
+          }
+
+          (if (mods.hasStaticFlag || inInterface && !(decls exists isDefDef))
              statics
            else
              members) ++= decls

src/compiler/scala/tools/nsc/typechecker/Contexts.scala

@@ -1016,7 +1016,16 @@ trait Contexts { self: Analyzer =>
           || unit.exists && s.sourceFile != unit.source.file
         )
       )
-      def lookupInPrefix(name: Name)    = pre member name filter qualifies
+      def lookupInPrefix(name: Name)    = {
+        val sym = pre.member(name).filter(qualifies)
+        def isNonPackageNoModuleClass(sym: Symbol) =
+          sym.isClass && !sym.isModuleClass && !sym.isPackageClass
+        if (!sym.exists && unit.isJava && isNonPackageNoModuleClass(pre.typeSymbol)) {
+          // TODO factor out duplication with Typer::inCompanionForJavaStatic
+          val pre1 = companionSymbolOf(pre.typeSymbol, this).typeOfThis
+          pre1.member(name).filter(qualifies).andAlso(_ => pre = pre1)
+        } else sym
+      }
       def accessibleInPrefix(s: Symbol) = isAccessible(s, pre, superAccess = false)
 
       def searchPrefix = {

src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala

@@ -201,7 +201,7 @@ trait MethodSynthesis {
 
     import AnnotationInfo.{mkFilter => annotationFilter}
     def addDerivedTrees(typer: Typer, stat: Tree): List[Tree] = stat match {
-      case vd @ ValDef(mods, name, tpt, rhs) if deriveAccessors(vd) && !vd.symbol.isModuleVar =>
+      case vd @ ValDef(mods, name, tpt, rhs) if deriveAccessors(vd) && !vd.symbol.isModuleVar && !vd.symbol.isJava =>
         stat.symbol.initialize // needed!
 
         val getter = Getter(vd)

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -4719,6 +4719,16 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             if (isStableContext(tree, mode, pt)) tree setType clazz.thisType else tree
         }
 
+
+      // For Java, instance and static members are in the same scope, but we put the static ones in the companion object
+      // so, when we can't find a member in the class scope, check the companion
+      def inCompanionForJavaStatic(pre: Type, cls: Symbol, name: Name): Symbol =
+        if (!(context.unit.isJava && cls.isClass && !cls.isModuleClass)) NoSymbol else {
+          val companion = companionSymbolOf(cls, context)
+          if (!companion.exists) NoSymbol
+          else member(gen.mkAttributedRef(pre, companion), name) // assert(res.isStatic, s"inCompanionJavaStatic($pre, $cls, $name) = $res ${res.debugFlagString}")
+        }
+
       /* Attribute a selection where `tree` is `qual.name`.
        * `qual` is already attributed.
        */
@@ -4745,7 +4755,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           dyna.wrapErrors(t, (_.typed1(t, mode, pt)))
         }
 
-        val sym = tree.symbol orElse member(qual, name) orElse {
+        val sym = tree.symbol orElse member(qual, name) orElse inCompanionForJavaStatic(qual.tpe.prefix, qual.symbol, name) orElse {
           // symbol not found? --> try to convert implicitly to a type that does have the required
           // member.  Added `| PATTERNmode` to allow enrichment in patterns (so we can add e.g., an
           // xml member to StringContext, which in turn has an unapply[Seq] method)

src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala

@@ -216,11 +216,16 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax
 
   class ScaladocJavaUnitScanner(unit: CompilationUnit) extends JavaUnitScanner(unit) {
 
-    private val docBuffer: StringBuilder = new StringBuilder
+    private var docBuffer: StringBuilder = _
     private var inDocComment = false
     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
 

src/scaladoc/scala/tools/nsc/doc/ScaladocGlobal.scala

@@ -13,7 +13,11 @@ trait ScaladocGlobalTrait extends Global {
 
   override val useOffsetPositions = false
   override def newUnitParser(unit: CompilationUnit) = new syntaxAnalyzer.ScaladocUnitParser(unit, Nil)
-  override def newJavaUnitParser(unit: CompilationUnit) = new syntaxAnalyzer.ScaladocJavaUnitParser(unit)
+  override def newJavaUnitParser(unit: CompilationUnit) = if (createJavadoc) {
+    new syntaxAnalyzer.ScaladocJavaUnitParser(unit)
+  } else {
+    super.newJavaUnitParser(unit)
+  }
 
   override lazy val syntaxAnalyzer = new ScaladocSyntaxAnalyzer[outer.type](outer) {
     val runsAfter = List[String]()
@@ -41,7 +45,7 @@ class ScaladocGlobal(settings: doc.Settings, reporter: Reporter) extends Global(
     phasesSet += analyzer.typerFactory
   }
   override def forScaladoc = true
-  override def createJavadoc = true
+  override def createJavadoc = if (settings.docNoJavaComments.value) false else true
 
   override lazy val analyzer = new {
     val global: ScaladocGlobal.this.type = ScaladocGlobal.this

src/scaladoc/scala/tools/nsc/doc/Settings.scala

@@ -213,6 +213,11 @@ class Settings(error: String => Unit, val printMsg: String => Unit = println(_))
     "Group similar functions together (based on the @group annotation)"
   )
 
+  val docNoJavaComments = BooleanSetting (
+    "-no-java-comments",
+    "Prevents parsing and inclusion of comments from java sources."
+  )
+
   // For improved help output.
   def scaladocSpecific = Set[Settings#Setting](
     docformat, doctitle, docfooter, docversion, docUncompilable, docsourceurl, docgenerator, docRootContent, useStupidTypes,
@@ -222,7 +227,7 @@ class Settings(error: String => Unit, val printMsg: String => Unit = println(_))
     docImplicits, docImplicitsDebug, docImplicitsShowAll, docImplicitsHide, docImplicitsSoundShadowing,
     docDiagramsMaxNormalClasses, docDiagramsMaxImplicitClasses,
     docNoPrefixes, docNoLinkWarnings, docRawOutput, docSkipPackages,
-    docExpandAllTypes, docGroups
+    docExpandAllTypes, docGroups, docNoJavaComments
   )
   val isScaladocSpecific: String => Boolean = scaladocSpecific map (_.name)
 

src/scaladoc/scala/tools/partest/ScaladocJavaModelTest.scala

@@ -0,0 +1,15 @@
+package scala.tools.partest
+
+import scala.tools.nsc.doc.Universe
+
+/** A class for testing scaladoc model generation on java sources. */
+abstract class ScaladocJavaModelTest extends ScaladocModelTest {
+
+  // overridden to pass explicit files to newDocFactory.makeUniverse (rather than code strings)
+  // since the .java file extension is required
+  override def model: Option[Universe] = {
+    val path = resourcePath + "/" + resourceFile
+    newDocFactory.makeUniverse(Left(List(path)))
+  }
+
+}

test/files/pos/t2377b/Q.java

@@ -0,0 +1,13 @@
+public class Q {
+
+  public static class Builder {}
+
+  public static class Inner {
+    public static class Builder { public void innerMethod() {} }
+    public Builder foo() { return new Builder(); } // this line gives an error, that Builder is ambiguous
+
+    public Inner.Builder viaSelect() { return new Builder(); } // this line gives an error, that Builder is ambiguous
+  }
+
+}
+