scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
230 stars 21 forks source link

Scaladoc loses documentation on whitebox macro expansion #12855

Open dbarvitsky opened 10 months ago

dbarvitsky commented 10 months ago

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):

For the second macro expansion (@enum):

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 commented 10 months ago

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

som-snytt commented 10 months ago

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.

dbarvitsky commented 10 months ago

@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).