From c4561c1d4945a38febc41436ed333569d0e9a063 Mon Sep 17 00:00:00 2001
From: Jason Zaugg <jzaugg@gmail.com>
Date: Mon, 7 Apr 2014 10:41:31 +0200
Subject: SI-8479 Fix constructor default args under scaladoc

The `DocDef` node hid the `DefDef` constructor from the scrutinee
of the namer when determining if the class had constructor defaults
or not.

The current pattern for fixing these bugs is to delegate the check
to `TreeInfo`, and account for the wrapper `DocDef` node. I've
followed that pattern, but expressed my feelings about this approach
in a TODO comment.

Before this patch, the enclosed test failed with:

     error: not enough arguments for constructor SparkContext: (master: String, appName: String)SparkContext
---
 src/compiler/scala/tools/nsc/ast/TreeInfo.scala    | 10 +++++++
 .../scala/tools/nsc/typechecker/Namers.scala       |  5 +---
 src/reflect/scala/reflect/internal/TreeInfo.scala  |  5 ++++
 test/scaladoc/run/SI-8479.check                    |  1 +
 test/scaladoc/run/SI-8479.scala                    | 32 ++++++++++++++++++++++
 5 files changed, 49 insertions(+), 4 deletions(-)
 create mode 100644 test/scaladoc/run/SI-8479.check
 create mode 100755 test/scaladoc/run/SI-8479.scala

diff --git a/src/compiler/scala/tools/nsc/ast/TreeInfo.scala b/src/compiler/scala/tools/nsc/ast/TreeInfo.scala
index cbbb4c8ba8..1005cd1ccf 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeInfo.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeInfo.scala
@@ -21,6 +21,11 @@ abstract class TreeInfo extends scala.reflect.internal.TreeInfo {
 
   import definitions.ThrowableClass
 
+  // TODO these overrides, and the slow trickle of bugs that they solve (e.g. SI-8479),
+  //      suggest that we should pursue an alternative design in which the DocDef nodes
+  //      are eliminated from the tree before typer, and instead are modelled as tree
+  //      attachments.
+
   /** Is tree legal as a member definition of an interface?
    */
   override def isInterfaceMember(tree: Tree): Boolean = tree match {
@@ -28,6 +33,11 @@ abstract class TreeInfo extends scala.reflect.internal.TreeInfo {
     case _ => super.isInterfaceMember(tree)
   }
 
+  override def isConstructorWithDefault(t: Tree) = t match {
+    case DocDef(_, definition) => isConstructorWithDefault(definition)
+    case _ => super.isConstructorWithDefault(t)
+  }
+
   /** Is tree a pure (i.e. non-side-effecting) definition?
    */
   override def isPureDef(tree: Tree): Boolean = tree match {
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index bb938074cb..45da6d80d6 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -663,10 +663,7 @@ trait Namers extends MethodSynthesis {
         val m = ensureCompanionObject(tree, caseModuleDef)
         m.moduleClass.updateAttachment(new ClassForCaseCompanionAttachment(tree))
       }
-      val hasDefault = impl.body exists {
-        case DefDef(_, nme.CONSTRUCTOR, _, vparamss, _, _)  => mexists(vparamss)(_.mods.hasDefault)
-        case _                                              => false
-      }
+      val hasDefault = impl.body exists treeInfo.isConstructorWithDefault
       if (hasDefault) {
         val m = ensureCompanionObject(tree)
         m.updateAttachment(new ConstructorDefaultsAttachment(tree, null))
diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala
index fa4441e513..a6556fc22d 100644
--- a/src/reflect/scala/reflect/internal/TreeInfo.scala
+++ b/src/reflect/scala/reflect/internal/TreeInfo.scala
@@ -50,6 +50,11 @@ abstract class TreeInfo {
     case _ => false
   }
 
+  def isConstructorWithDefault(t: Tree) = t match {
+    case DefDef(_, nme.CONSTRUCTOR, _, vparamss, _, _)  => mexists(vparamss)(_.mods.hasDefault)
+    case _                                              => false
+  }
+
   /** Is tree a pure (i.e. non-side-effecting) definition?
    */
   def isPureDef(tree: Tree): Boolean = tree match {
diff --git a/test/scaladoc/run/SI-8479.check b/test/scaladoc/run/SI-8479.check
new file mode 100644
index 0000000000..619c56180b
--- /dev/null
+++ b/test/scaladoc/run/SI-8479.check
@@ -0,0 +1 @@
+Done.
diff --git a/test/scaladoc/run/SI-8479.scala b/test/scaladoc/run/SI-8479.scala
new file mode 100755
index 0000000000..3c91395025
--- /dev/null
+++ b/test/scaladoc/run/SI-8479.scala
@@ -0,0 +1,32 @@
+import scala.tools.nsc.doc.model._
+import scala.tools.nsc.doc.base._
+import scala.tools.nsc.doc.base.comment._
+import scala.tools.partest.ScaladocModelTest
+import java.net.{URI, URL}
+import java.io.File
+
+object Test extends ScaladocModelTest {
+
+  override def code =
+    """
+      |object Test {
+      |  val x = new SparkContext(master = "")
+      |}
+      |
+      |class SparkContext(config: Any) {
+      |
+      |  /** Scaladoc comment */
+      |  def this(
+      |      master: String,
+      |      appName: String = "") = this(null)
+      |}
+      |
+      |
+    """.stripMargin
+
+  override def scaladocSettings = ""
+
+  def testModel(rootPackage: Package) {
+    // it didn't crash
+  }
+}
-- 
cgit v1.2.3