diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-04-07 10:41:31 +0200 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-04-07 10:44:35 +0200 |
commit | c4561c1d4945a38febc41436ed333569d0e9a063 (patch) | |
tree | 1ef5eb007b2c2f717b9aa9c371dcebcd926d3fbf | |
parent | 5720e97b95da57c9549698a9038efb79092394ad (diff) | |
download | scala-c4561c1d4945a38febc41436ed333569d0e9a063.tar.gz scala-c4561c1d4945a38febc41436ed333569d0e9a063.tar.bz2 scala-c4561c1d4945a38febc41436ed333569d0e9a063.zip |
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
-rw-r--r-- | src/compiler/scala/tools/nsc/ast/TreeInfo.scala | 10 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Namers.scala | 5 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/TreeInfo.scala | 5 | ||||
-rw-r--r-- | test/scaladoc/run/SI-8479.check | 1 | ||||
-rwxr-xr-x | test/scaladoc/run/SI-8479.scala | 32 |
5 files changed, 49 insertions, 4 deletions
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 + } +} |