Check parent docs even when class is indexed

scalameta · Jun 28, 2023 · 278f17b · 278f17b
1 parent 6e9bca7
commit 278f17b
Show file tree

Hide file tree

Showing 2 changed files with 117 additions and 34 deletions.
diff --git a/mtags/src/main/scala/scala/meta/internal/metals/Docstrings.scala b/mtags/src/main/scala/scala/meta/internal/metals/Docstrings.scala
@@ -36,46 +36,52 @@ class Docstrings(index: GlobalSymbolIndex) {
       symbol: String,
       parents: ParentSymbols
   ): Optional[SymbolDocumentation] = {
-    cache.get(symbol) match {
+    val result = cache.get(symbol) match {
       case Some(value) =>
-        if (value == EmptySymbolDocumentation) Optional.empty()
-        else Optional.of(value)
+        if (value == EmptySymbolDocumentation) None
+        else Some(value)
       case None =>
         indexSymbol(symbol)
         val result = cache.get(symbol)
-        val resultWithDocs = result match {
-          case None =>
-            cache(symbol) = EmptySymbolDocumentation
-            result
-          /* Fall back to parent scaladocs if nothing is specified for the current symbol
-           * This way we also cache the result in order not to calculate parents again.
-           */
-          case Some(value: MetalsSymbolDocumentation)
-              if value.docstring.isEmpty() =>
-            parents
-              .parents()
-              .asScala
-              .flatMap { s =>
-                if (cache.contains(s)) cache.get(s)
-                else {
-                  indexSymbol(s)
-                  cache.get(s)
-                }
-              }
-              .find(_.docstring().nonEmpty)
-              .fold {
-                result
-              } { withDocs =>
-                val updated = value.copy(docstring = withDocs.docstring())
-                cache(symbol) = updated
-                Some(updated)
-              }
-          case _ =>
-            result
+        if (result.isEmpty)
+          cache(symbol) = EmptySymbolDocumentation
+        result
+    }
+    /* Fall back to parent javadocs/scaladocs if nothing is specified for the current symbol
+     * This way we also cache the result in order not to calculate parents again.
+     */
+    val resultWithParentDocs = result match {
+      case Some(value: MetalsSymbolDocumentation)
+          if value.docstring.isEmpty() =>
+        Some(parentDocumentation(symbol, value, parents))
+      case _ => result
+    }
+    Optional.ofNullable(resultWithParentDocs.orNull)
+  }
 
+  def parentDocumentation(
+      symbol: String,
+      docs: MetalsSymbolDocumentation,
+      parents: ParentSymbols
+  ): SymbolDocumentation = {
+    parents
+      .parents()
+      .asScala
+      .flatMap { s =>
+        if (cache.contains(s)) cache.get(s)
+        else {
+          indexSymbol(s)
+          cache.get(s)
         }
-        Optional.ofNullable(resultWithDocs.orNull)
-    }
+      }
+      .find(_.docstring().nonEmpty)
+      .fold {
+        docs
+      } { withDocs =>
+        val updated = docs.copy(docstring = withDocs.docstring())
+        cache(symbol) = updated
+        updated
+      }
   }
 
   /**

diff --git a/tests/unit/src/test/scala/tests/HoverLspSuite.scala b/tests/unit/src/test/scala/tests/HoverLspSuite.scala
@@ -150,6 +150,83 @@ class HoverLspSuite extends BaseLspSuite("hover-") with TestHovers {
     } yield ()
   }
 
+  test("docstrings java parentdoc".tag(FlakyWindows)) {
+    for {
+      _ <- initialize(
+        """/metals.json
+          |{"a":{}}
+          |/a/src/main/java/a/Foo.java
+          |package a;
+          |public class Foo {
+          |  public static class Def {
+          |    /**
+          |      * test docs
+          |      */
+          |    public void foo(int x) {}
+          |  }
+          |
+          |  public static class ChildDef extends Def {
+          |    @Override
+          |    public void foo(int x) {}
+          |  }
+          |  void test() {
+          |    new ChildDef().foo(1);
+          |  }
+          |}
+        """.stripMargin
+      )
+      _ <- server.assertHover(
+        "a/src/main/java/a/Foo.java",
+        """package a;
+          |public class Foo {
+          |  public static class Def {
+          |    /**
+          |      * test docs
+          |      */
+          |    public void foo(int x) {}
+          |  }
+          |
+          |  public static class ChildDef extends Def {
+          |    @Override
+          |    public void foo(int x) {}
+          |  }
+          |  void test() {
+          |    new Chil@@dDef().foo(1);
+          |  }
+          |}""".stripMargin,
+        """```java
+          |public ChildDef()
+          |```
+          |""".stripMargin.hover,
+      )
+      _ <- server.assertHover(
+        "a/src/main/java/a/Foo.java",
+        """package a;
+          |public class Foo {
+          |  public static class Def {
+          |    /**
+          |      * test docs
+          |      */
+          |    public void foo(int x) {}
+          |  }
+          |
+          |  public static class ChildDef extends Def {
+          |    @Override
+          |    public void foo(int x) {}
+          |  }
+          |  void test() {
+          |    new ChildDef().fo@@o(1);
+          |  }
+          |}""".stripMargin,
+        """```java
+          |public void foo(int x)
+          |```
+          |test docs
+          |""".stripMargin.hover,
+      )
+    } yield ()
+  }
+
   test("dependencies".tag(FlakyWindows), withoutVirtualDocs = true) {
     for {
       _ <- initialize(