@react macro fails when building docs if Props has scaladocs

[harpocrates]

When trying to build docs with sbt doc, the @react annotation doesn't work if the Props defined inside the class has a scaladoc on it. Outside of trying to build docs, everything works fine.

$ sbt doc
Main Scala API documentation to /Users/alec/Scratch/slinky-bug/target/scala-2.13/api...
[error] /Users/alec/Scratch/slinky-bug/src/main/scala/bug/Main.scala:11:2: Components must define a Props type or case class, but none was found.
[error] @react class MyApp extends StatelessComponent {
[error]  ^
[error] /Users/alec/Scratch/slinky-bug/src/main/scala/bug/Main.scala:23:21: not found: value MyApp
[error]     ReactDOM.render(MyApp("hello world"), container)
[error]                     ^
[error] two errors found
[error] (Compile / doc) Scaladoc generation failed
[error] Total time: 4 s, completed 6-Jun-2020 6:07:15 PM

Complete example:

package bug

import slinky.core._
import slinky.core.annotations.react
import slinky.web.ReactDOM
import slinky.web.html._

import scala.scalajs.js.annotation.{JSExportTopLevel}
import org.scalajs.dom

@react class MyApp extends StatelessComponent {

  /** @param message the message */
  case class Props(message: String)

  def render = span(props.message)
}

object Exports {

  @JSExportTopLevel("mountMyApp")
  def mountMyApp(container: dom.raw.Element): Unit = {
    ReactDOM.render(MyApp("hello world"), container)
  }
}

Version details:

• Scala 2.13.1 or 2.12.10 (both trigger the bug)
• Slinky 0.6.5
• React 16.12.0

---

Other than that, just wanted to say think you for the amazing project. ❤️

[shadaj]

Thanks for the bug report! Probably a bug with how we look for the Props definition in the macro.

[harpocrates]

I spent a little time yesterday looking into this. I worry it won't be an easy fix - see the Scala bug where I referenced this issue. The DocDef issue affects almost all quasiquote pattern matching of ASTs. Some other places this issue crops up:

1. the Props/State/Snapshot members of a class annotated with @react don't work if they have scaladocs (the original bug report)
2. the Props members of an object annotated with @react doesn't work if it has a scaladoc
3. object and class annotated with @react don't match if they have scaladocs

It is possible work around these one by one, but it gets tedious and error-prone.

Here's a sample diff which solves 1:

diff --git a/core/build.sbt b/core/build.sbt
index 809fd1a..2150c21 100644
--- a/core/build.sbt
+++ b/core/build.sbt
@@ -3,6 +3,7 @@ enablePlugins(ScalaJSPlugin)
 name := "slinky-core"
 
 libraryDependencies += "org.scala-lang" % "scala-reflect" % scalaVersion.value
+libraryDependencies += "org.scala-lang" % "scala-compiler" % scalaVersion.value
 
 scalacOptions ++= {
   if (scalaJSVersion.startsWith("0.6.")) Seq("-P:scalajs:sjsDefinedByDefault")
diff --git a/core/src/main/scala/slinky/core/annotations/react.scala b/core/src/main/scala/slinky/core/annotations/react.scala
index b6ce202..d8b034d 100644
--- a/core/src/main/scala/slinky/core/annotations/react.scala
+++ b/core/src/main/scala/slinky/core/annotations/react.scala
@@ -32,7 +32,19 @@ object ReactMacrosImpl {
   def createComponentBody(c: whitebox.Context)(cls: c.Tree): (c.Tree, List[c.Tree]) = {
     import c.universe._
     val q"..$_ class ${className: Name} extends ..$parents { $self => ..$stats}" = cls // scalafix:ok
-    val (propsDefinition, applyMethods) = stats.flatMap {
+
+    // Workaround for <https://github.com/scala/bug/issues/7993>
+    val unwrapDocDef = (t: Tree) => {
+      import scala.tools.nsc.ast.Trees
+
+      if (t.isInstanceOf[Trees#DocDef]) {
+        t.asInstanceOf[Trees#DocDef].definition.asInstanceOf[Tree]
+      } else {
+        t
+      }
+    }
+
+    val (propsDefinition, applyMethods) = stats.map(unwrapDocDef).flatMap {
       case defn @ q"..$_ type Props = ${_}" =>
         Some((defn, Seq()))
 
@@ -75,7 +87,7 @@ object ReactMacrosImpl {
     }.headOption
       .getOrElse(c.abort(c.enclosingPosition, "Components must define a Props type or case class, but none was found."))
 
-    val stateDefinition = stats.flatMap {
+    val stateDefinition = stats.map(unwrapDocDef).flatMap {
       case defn @ q"..$_ type State = ${_}" =>
         Some(defn)
       case defn @ q"case class State[..$_](...$_) extends ..$_ { $_ => ..$_ }" =>
@@ -83,7 +95,7 @@ object ReactMacrosImpl {
       case _ => None
     }.headOption
 
-    val snapshotDefinition = stats.flatMap {
+    val snapshotDefinition = stats.map(unwrapDocDef).flatMap {
       case defn @ q"type Snapshot = ${_}" =>
         Some(defn)
       case defn @ q"case class Snapshot[..$_](...$_) extends ..$_ { $_ => ..$_ }" =>
@@ -105,8 +117,10 @@ object ReactMacrosImpl {
                 null.asInstanceOf[Snapshot]
                 ()
               })
-              ..${stats.filterNot(s =>
-        s == propsDefinition || s == stateDefinition.orNull || s == snapshotDefinition.orNull
+              ..${stats.filterNot(s => {
+                val s2 = unwrapDocDef(s)
+                s2 == propsDefinition || s2 == stateDefinition.orNull || s2 == snapshotDefinition.orNull
+              }
       )}
             }"""

Note that this solution just throws away the doc (which is probably not the right thing to do - the doc should just be moved to the right place).

[shadaj]

@harpocrates thanks so much for looking into this! Would you be able to make a PR with these changes?

For now, I think it's fine to throw away the docs given that there's no other nice solution for quasiquotes, and we can create a separate issue for passing through the docs. Even though it's not the ideal result, it's probably better than just crashing the compiler.

[harpocrates]

Are you OK with the part that adds libraryDependencies += "org.scala-lang" % "scala-compiler" % scalaVersion.value though? I dislike being tied to concrete compiler types, but agree it would be better than a crash.

[shadaj]

@harpocrates apologies for the late response! I think it would be fine to include that dependency, as you said it's better than having compiler crashes.

[visox]

Hi, i had the same problem. I tried many things, what worked (not sure if only that) was when i improved my scalacOptions.

I left only

    "-deprecation", // Emit warning and location for usages of deprecated APIs.
    "-encoding",
    "utf-8", // Specify character encoding used by source files.
    "-explaintypes", // Explain type errors in more detail.
    "-feature", // Emit warning and location for usages of features that should be imported explicitly.
    "-language:existentials", // Existential types (besides wildcard types) can be written and inferred
    "-language:experimental.macros", // Allow macro definition (besides implementation and application)
    "-language:higherKinds", // Allow higher-kinded types
    "-language:implicitConversions", // Allow definition of implicit functions called views
    "-unchecked",  // Enable additional warnings where generated code depends on assumptions.
    "-Ywarn-extra-implicit", // Warn when more than one implicit parameter section is defined.
    "-Ywarn-numeric-widen",
    "-Ymacro-annotations",

i use scala 2.13.3 and slinky 0.6.6

edit: maybe its not the same problem i had only one error:

/Main.scala:51:7: not found: value App
[error]       App(()),
[error]       ^
[error] one error found