Skip to content

Commit c4561c1

Browse files
committed
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
1 parent 5720e97 commit c4561c1

File tree

5 files changed

+49
-4
lines changed

5 files changed

+49
-4
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,23 @@ abstract class TreeInfo extends scala.reflect.internal.TreeInfo {
2121

2222
import definitions.ThrowableClass
2323

24+
// TODO these overrides, and the slow trickle of bugs that they solve (e.g. SI-8479),
25+
// suggest that we should pursue an alternative design in which the DocDef nodes
26+
// are eliminated from the tree before typer, and instead are modelled as tree
27+
// attachments.
28+
2429
/** Is tree legal as a member definition of an interface?
2530
*/
2631
override def isInterfaceMember(tree: Tree): Boolean = tree match {
2732
case DocDef(_, definition) => isInterfaceMember(definition)
2833
case _ => super.isInterfaceMember(tree)
2934
}
3035

36+
override def isConstructorWithDefault(t: Tree) = t match {
37+
case DocDef(_, definition) => isConstructorWithDefault(definition)
38+
case _ => super.isConstructorWithDefault(t)
39+
}
40+
3141
/** Is tree a pure (i.e. non-side-effecting) definition?
3242
*/
3343
override def isPureDef(tree: Tree): Boolean = tree match {

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -663,10 +663,7 @@ trait Namers extends MethodSynthesis {
663663
val m = ensureCompanionObject(tree, caseModuleDef)
664664
m.moduleClass.updateAttachment(new ClassForCaseCompanionAttachment(tree))
665665
}
666-
val hasDefault = impl.body exists {
667-
case DefDef(_, nme.CONSTRUCTOR, _, vparamss, _, _) => mexists(vparamss)(_.mods.hasDefault)
668-
case _ => false
669-
}
666+
val hasDefault = impl.body exists treeInfo.isConstructorWithDefault
670667
if (hasDefault) {
671668
val m = ensureCompanionObject(tree)
672669
m.updateAttachment(new ConstructorDefaultsAttachment(tree, null))

src/reflect/scala/reflect/internal/TreeInfo.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ abstract class TreeInfo {
5050
case _ => false
5151
}
5252

53+
def isConstructorWithDefault(t: Tree) = t match {
54+
case DefDef(_, nme.CONSTRUCTOR, _, vparamss, _, _) => mexists(vparamss)(_.mods.hasDefault)
55+
case _ => false
56+
}
57+
5358
/** Is tree a pure (i.e. non-side-effecting) definition?
5459
*/
5560
def isPureDef(tree: Tree): Boolean = tree match {

test/scaladoc/run/SI-8479.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Done.

test/scaladoc/run/SI-8479.scala

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import scala.tools.nsc.doc.model._
2+
import scala.tools.nsc.doc.base._
3+
import scala.tools.nsc.doc.base.comment._
4+
import scala.tools.partest.ScaladocModelTest
5+
import java.net.{URI, URL}
6+
import java.io.File
7+
8+
object Test extends ScaladocModelTest {
9+
10+
override def code =
11+
"""
12+
|object Test {
13+
| val x = new SparkContext(master = "")
14+
|}
15+
|
16+
|class SparkContext(config: Any) {
17+
|
18+
| /** Scaladoc comment */
19+
| def this(
20+
| master: String,
21+
| appName: String = "") = this(null)
22+
|}
23+
|
24+
|
25+
""".stripMargin
26+
27+
override def scaladocSettings = ""
28+
29+
def testModel(rootPackage: Package) {
30+
// it didn't crash
31+
}
32+
}

0 commit comments

Comments
 (0)