bug
bug copied to clipboard
Scaladoc loses documentation on whitebox macro expansion
Reproduction steps
Scala version: 2.13.11
Hit this while migrating one of the company's internal tools to 2.13.11 from 2.11.x (asking for trouble, I know).
The unit test to reproduce the problem:
import org.scalatest.matchers.should.Matchers._
...
val settings = new doc.Settings(println, println)
settings.embeddedDefaults(this.getClass().getClassLoader())
settings.language.add(settings.languageFeatures.macros.toString)
settings.YmacroAnnotations.value = true
// Other settings are not important
settings.usejavacp.value = true
settings.usemanifestcp.value = false
settings.deprecation.value = true
settings.feature.value = true
settings.showPhases.value = true
settings.verbose.value = true
val reporter = Reporter(settings)
val factory = new doc.DocFactory(reporter, settings)
val universe = factory.makeUniverse(Right("""
|package test
|
|import com.p11a.cddtools.authoring.constants
|import com.p11a.cddtools.authoring.enum
|
|/**
| * [[Container]] is an outer object.
| */
|@constants object Container {
| /**
| * [[Nested]] is an inner class.
| */
| @enum class Nested {
| }
|}
""".stripMargin))
val container0 = universe.get.rootPackage.packages.find(_.name == "test").get
.members.find(_.qualifiedName == "test.Container").get
.asInstanceOf[doc.model.DocTemplateEntity]
val nested0 = container0.members.find(_.qualifiedName == "test.Container.Nested").get
container0.comment shouldBe defined // OK
nested0.comment shouldBe defined // Fail, actually None
Problem
Both @constants and @enum are no-op whitebox macros returning annottees.head (they used to do some tree manipulation, but I created a no-op version for the test). I was able (I think) to track the problem down to MacroAnnotationNamers.expandMacroAnnotation:
override def expandMacroAnnotations(stats: List[Tree]): List[Tree] = {
...
def rewrapAfterTransform(stat: Tree, transformed: List[Tree]): List[Tree] = (stat, transformed) match {
case (stat @ DocDef(comment, _), List(transformed: MemberDef)) => List(treeCopy.DocDef(stat, comment, transformed))
case (stat @ DocDef(comment, _), List(transformed: DocDef)) => List(transformed)
case (_, Nil | List(_: MemberDef)) => transformed
case (_, unexpected) => unexpected // NOTE: who knows how people are already using macro annotations, so it's scary to fail here
}
if (phase.id > currentRun.typerPhase.id || !stats.exists(mightNeedTransform)) stats
else stats.flatMap { stat =>
if (mightNeedTransform(stat)) {
val sym = stat.symbol
assert(sym != NoSymbol, (sym, stat))
if (isMaybeExpandee(sym)) {
def assert(what: Boolean) = Predef.assert(what, s"${sym.accurateKindString} ${sym.rawname}#${sym.id} with ${sym.rawInfo.kind}")
assert(sym.rawInfo.isInstanceOf[MacroAnnotationNamer#MaybeExpandeeCompleter])
sym.rawInfo.completeOnlyExpansions(sym)
assert(!sym.rawInfo.isInstanceOf[MacroAnnotationNamer#MaybeExpandeeCompleter])
}
val derivedTrees = attachedExpansion(sym).getOrElse(List(stat))
val (me, others) = derivedTrees.partition(_.symbol == sym) // <-- SEE HERE
rewrapAfterTransform(stat, me) ++ expandMacroAnnotations(others)
} else {
List(stat)
}
}
}
For first macro expansion (@constants):
- the
derivedTreesis a single-element array containing the expanded tree (as expected) - the
statis theDocDefwith correct comment, as expected - the
symis theSymbols$ModuleSymbolthat is exactly the same asderivedTrees.head.symbol - the
rewrapAfterTransformis called withtransformed = me = derivedTrees.headand the first case wraps the tree as expected
For the second macro expansion (@enum):
- the
derivedTreesis likewise a single-element array containing the expanded tree - the
statis theDocDSefwith correct comment, as before - the
symis theSymbols$ClassSymbolofclass Nested, thederivedTrees.head.symbolis alsoSymbols$ClassSymbolofclass Nestedbut they are for some reason different (with respect to equality). They both correspond to the same_rawnameand have the same_rawowner, and only seem to be different byrawattandsymbolinfos. - the
rewrapAfterTransformis called withtransformed = me = List.empty()and the third case returns the empty list
What I think is happening here is predicate _.symbol == sym in derivedTree.partition(_.symbol == sym) returns false, and the expanded tree ends up in others as opposed to me. It looks wrong in this case, since the both sym and derivedTrees.head.symbol actually reference to the same class Nested, just before and after expansion.
Perhaps the predicate should be relaxed somehow? Admittedly, I am out of my depth here and couldn't figure out where the symbols come from during the expansion, or why they end up being different in nested case. So I apologize in advance for confusion.
Additional observations / variance
The problem does not reproduce if Nested is not nested in Container. Top-level macro applications seem to be working as expected:
package test
...
/** [[Container]]...*/
@constants object Container {...}
/** [[Nested]]...*/
@enum class NotNested {...}
The problem reproduces even if Container does not have @constants macro annotation. E.g. the following still fails in the same way.
package test
...
/** [[Container]] is an outer object. */
object Container {
/** [[Nested]] is an inner class. */
@enum class Nested {}
}
Thank you! Happy to provide more context / assistance / testing.
I attempted changing the predicate to something less strict. ~This does not solve the problem, actually makes it worse by causing the double-expansion of the @enum macro and later failing the compilation. There probably is more to it than just wrapping the right tree.~
Edit: the double-invocation of the macro was my fault, not caused by the original issue
I thought I was recently (a few days ago maybe) reading about this issue, where macro annotation doesn't run so typechecking failure also fails scaladoc. I don't think I dreamt it, but I couldn't find the conversation when I looked earlier. If anyone knows what I'm talking about, a link would be much appreciated.
@som-snytt, thanks for looking into this. I poked around in the issues, nothing familiar. In my case the macros seem to run though (confirmed by tracing / setting breakpoints).