From c1cfd77cc7ab59bd2a7cab1ac4681a6329fdee28 Mon Sep 17 00:00:00 2001
From: Sergei Petunin <s.petunin@me.com>
Date: Wed, 8 Jul 2020 07:12:22 +0200
Subject: [PATCH] [bug#11021]

The issue was due to incompatibility of the two features (one is parsing @hideImplicitConversion to filter out unwanted conversions, another is parsing @throws tag to add a link to a referenced exception).
The code that filters out the hidden implicit conversions tries to parse the comment in search for the @hideImplicitConversion tag. Previously this happened during ModelFactory.makeModel().
But if a comment contains a @throws tag, the code that parses this tag assumes the model is already finished (ModelFactory.modelFinished == true), since it needs to lookup the exception.
Solved by moving the implicit handling to the ModelFactory.completeModel() stage, after the model is already finished.
---
 .../tools/nsc/doc/model/ModelFactory.scala    |  5 +--
 .../model/ModelFactoryImplicitSupport.scala   | 10 ++---
 .../resources/implicits-throws-res.scala      |  9 +++++
 test/scaladoc/run/implicits-throws.check      |  1 +
 test/scaladoc/run/implicits-throws.scala      | 37 +++++++++++++++++++
 5 files changed, 54 insertions(+), 8 deletions(-)
 create mode 100644 test/scaladoc/resources/implicits-throws-res.scala
 create mode 100644 test/scaladoc/run/implicits-throws.check
 create mode 100644 test/scaladoc/run/implicits-throws.scala

diff --git a/src/scaladoc/scala/tools/nsc/doc/model/ModelFactory.scala b/src/scaladoc/scala/tools/nsc/doc/model/ModelFactory.scala
index 034ec38564b..2757d5d538a 100644
--- a/src/scaladoc/scala/tools/nsc/doc/model/ModelFactory.scala
+++ b/src/scaladoc/scala/tools/nsc/doc/model/ModelFactory.scala
@@ -365,9 +365,8 @@ class ModelFactory(val global: Global, val settings: doc.Settings) {
       else
         implicitlyConvertibleClassesCache.toList
 
-    // the implicit conversions are generated eagerly, but the members generated by implicit conversions are added
-    // lazily, on completeModel
-    val conversions: List[ImplicitConversionImpl] =
+    // the implicit conversions are generated lazily, on completeModel
+    lazy val conversions: List[ImplicitConversionImpl] =
       if (settings.docImplicits) makeImplicitConversions(sym, this) else Nil
 
     // members as given by the compiler
diff --git a/src/scaladoc/scala/tools/nsc/doc/model/ModelFactoryImplicitSupport.scala b/src/scaladoc/scala/tools/nsc/doc/model/ModelFactoryImplicitSupport.scala
index 1d52b17a646..0695530f707 100644
--- a/src/scaladoc/scala/tools/nsc/doc/model/ModelFactoryImplicitSupport.scala
+++ b/src/scaladoc/scala/tools/nsc/doc/model/ModelFactoryImplicitSupport.scala
@@ -31,10 +31,10 @@ import scala.tools.nsc.Reporting.WarningCategory
  *
  *      class C extends B {
  *        def bar = 2
- *        class implicit
+ *        class D
  *      }
  *
- *      D def conv(a: A) = new C
+ *      implicit def conv(a: A) = new C
  *    }
  * }}}
  *
@@ -103,10 +103,10 @@ trait ModelFactoryImplicitSupport {
       // also keep empty conversions, so they appear in diagrams
       // conversions = conversions.filter(!_.members.isEmpty)
 
-      val hiddenConversions: Seq[String] = thisFactory
+      val hiddenConversions: Set[String] = thisFactory
         .comment(sym, inTpl.linkTarget, inTpl)
-        .map(_.hideImplicitConversions)
-        .getOrElse(Nil)
+        .map(_.hideImplicitConversions.toSet)
+        .getOrElse(Set.empty)
 
       conversions = conversions filterNot { conv: ImplicitConversionImpl =>
         hiddenConversions.contains(conv.conversionShortName) ||
diff --git a/test/scaladoc/resources/implicits-throws-res.scala b/test/scaladoc/resources/implicits-throws-res.scala
new file mode 100644
index 00000000000..7bff5317e8b
--- /dev/null
+++ b/test/scaladoc/resources/implicits-throws-res.scala
@@ -0,0 +1,9 @@
+package scala.test.scaladoc.implicits.throws {
+
+  /**
+   * @throws java.lang.Exception exception
+   */
+  class A {
+  }
+
+}
diff --git a/test/scaladoc/run/implicits-throws.check b/test/scaladoc/run/implicits-throws.check
new file mode 100644
index 00000000000..619c56180bb
--- /dev/null
+++ b/test/scaladoc/run/implicits-throws.check
@@ -0,0 +1 @@
+Done.
diff --git a/test/scaladoc/run/implicits-throws.scala b/test/scaladoc/run/implicits-throws.scala
new file mode 100644
index 00000000000..ccca253450f
--- /dev/null
+++ b/test/scaladoc/run/implicits-throws.scala
@@ -0,0 +1,37 @@
+import scala.language._
+import scala.tools.nsc.doc.base.comment.Text
+import scala.tools.nsc.doc.model._
+import scala.tools.partest.ScaladocModelTest
+
+/**
+ * Reproducer for https://github.com/scala/bug/issues/11021
+ * The @throws tag shouldn't cause an error when -implicits is enabled.
+ */
+object Test extends ScaladocModelTest {
+
+  // test a file instead of a piece of code
+  override def resourceFile = "implicits-throws-res.scala"
+
+  // start implicits
+  def scaladocSettings = "-implicits"
+
+  def testModel(root: Package) = {
+    // get the quick access implicit defs in scope (_package(s), _class(es), _trait(s), object(s) _method(s), _value(s))
+    import access._
+
+    val base = root._package("scala")._package("test")._package("scaladoc")._package("implicits")._package("throws")
+
+    val A = base._class("A")
+
+    val expectedComment = "exception"
+
+    A.comment
+      .flatMap(comment => comment.throws.get("java.lang.Exception"))
+      .flatMap(body => body.summary) match {
+      case Some(Text(actualComment)) =>
+        assert(actualComment == expectedComment, s"Expected: $expectedComment, actual: $actualComment")
+      case _ =>
+        assert(false, s"Failed to match the comment")
+    }
+  }
+}