bug icon indicating copy to clipboard operation
bug copied to clipboard

Scaladoc loses documentation on whitebox macro expansion

Open dbarvitsky opened this issue 1 year ago • 3 comments

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 derivedTrees is a single-element array containing the expanded tree (as expected)
  • the stat is the DocDef with correct comment, as expected
  • the sym is the Symbols$ModuleSymbol that is exactly the same as derivedTrees.head.symbol
  • the rewrapAfterTransform is called with transformed = me = derivedTrees.head and the first case wraps the tree as expected

For the second macro expansion (@enum):

  • the derivedTrees is likewise a single-element array containing the expanded tree
  • the stat is the DocDSef with correct comment, as before
  • the sym is the Symbols$ClassSymbol of class Nested, the derivedTrees.head.symbol is also Symbols$ClassSymbol of class Nested but they are for some reason different (with respect to equality). They both correspond to the same _rawname and have the same _rawowner, and only seem to be different by rawatt and symbolinfos.
  • the rewrapAfterTransform is called with transformed = 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.

dbarvitsky avatar Aug 28 '23 17:08 dbarvitsky