bugfix: show import renames in hover (#5982)

* bugfix: show using renames in hover * show import renames in hover Scala 3 * show import renames in hover Scala 2 * scala 3 fix * put used used rename in history * change context for Scala 3 hover Co-Authored-By: Jakub Cieśluk <[email protected]> --------- Co-authored-by: Jakub Cieśluk <[email protected]>
scalameta · Jan 11, 2024 · cd993c5 · cd993c5
1 parent 33ae25a
commit cd993c5
Show file tree

Hide file tree

Showing 12 changed files with 190 additions and 40 deletions.
diff --git a/mtags-shared/src/main/scala/scala/meta/internal/pc/HoverMarkup.scala b/mtags-shared/src/main/scala/scala/meta/internal/pc/HoverMarkup.scala
@@ -18,9 +18,16 @@ object HoverMarkup {
       expressionType: String,
       optSymbolSignature: Option[String],
       docstring: String,
-      forceExpressionType: Boolean = false
+      forceExpressionType: Boolean = false,
+      contextInfo: List[String] = Nil
   ): String = {
     val markdown = new StringBuilder()
+    if (contextInfo.nonEmpty) {
+      markdown
+        .append("```scala\n")
+        .append(contextInfo.mkString("\n"))
+        .append("\n```\n\n")
+    }
     if (forceExpressionType || optSymbolSignature.isEmpty) {
       markdown
         .append(

diff --git a/mtags-shared/src/main/scala/scala/meta/internal/pc/ScalaHover.scala b/mtags-shared/src/main/scala/scala/meta/internal/pc/ScalaHover.scala
@@ -12,17 +12,35 @@ case class ScalaHover(
     symbolSignature: Option[String] = None,
     docstring: Option[String] = None,
     forceExpressionType: Boolean = false,
-    range: Option[lsp4j.Range] = None
+    range: Option[lsp4j.Range] = None,
+    contextInfo: List[String] // e.g. info about rename imports
 ) extends HoverSignature {
 
+  def this(
+      expressionType: Option[String],
+      symbolSignature: Option[String],
+      docstring: Option[String],
+      forceExpressionType: Boolean,
+      range: Option[lsp4j.Range]
+  ) =
+    this(
+      expressionType,
+      symbolSignature,
+      docstring,
+      forceExpressionType,
+      range,
+      contextInfo = Nil
+    )
+
   def signature(): Optional[String] = symbolSignature.asJava
   def toLsp(): lsp4j.Hover = {
     val markdown =
       HoverMarkup(
         expressionType.getOrElse(""),
         symbolSignature,
         docstring.getOrElse(""),
-        forceExpressionType
+        forceExpressionType,
+        contextInfo
       )
     new lsp4j.Hover(markdown.toMarkupContent, range.orNull)
   }
@@ -35,7 +53,8 @@ case class ScalaHover(
       symbolSignature,
       docstring,
       forceExpressionType,
-      Some(range)
+      Some(range),
+      contextInfo
     )
 
 }
diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/HoverProvider.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/HoverProvider.scala
@@ -217,27 +217,36 @@ class HoverProvider(val compiler: MetalsGlobal, params: OffsetParams)(implicit
         pos.start != pos.end && (symbol == null || symbol == NoSymbol || symbol.isErroneous)
       ) {
         val context = doLocateContext(pos)
+        val re: scala.collection.Map[Symbol, Name] = renamedSymbols(context)
         val history = new ShortenedNames(
-          lookupSymbol = name => context.lookupSymbol(name, _ => true) :: Nil
+          lookupSymbol = name => context.lookupSymbol(name, _ => true) :: Nil,
+          renames = re
         )
         val prettyType = metalsToLongString(tpe.widen.finalResultType, history)
         val lspRange = if (range.isRange) Some(range.toLsp) else None
         Some(
-          new ScalaHover(expressionType = Some(prettyType), range = lspRange)
+          new ScalaHover(
+            expressionType = Some(prettyType),
+            range = lspRange,
+            contextInfo = history.getUsedRenamesInfo()
+          )
         )
       } else if (symbol == null || tpe.typeSymbol.isAnonymousClass) None
       else if (symbol.hasPackageFlag || symbol.hasModuleFlag) {
         Some(
           new ScalaHover(
             expressionType = Some(
               s"${symbol.javaClassSymbol.keyString} ${symbol.fullName}"
-            )
+            ),
+            contextInfo = Nil
           )
         )
       } else {
         val context = doLocateContext(pos)
+        val re: scala.collection.Map[Symbol, Name] = renamedSymbols(context)
         val history = new ShortenedNames(
-          lookupSymbol = name => context.lookupSymbol(name, _ => true) :: Nil
+          lookupSymbol = name => context.lookupSymbol(name, _ => true) :: Nil,
+          renames = re
         )
         val symbolInfo =
           if (seenFrom.isErroneous) symbol.info
@@ -273,7 +282,8 @@ class HoverProvider(val compiler: MetalsGlobal, params: OffsetParams)(implicit
             docstring = Some(docstring),
             forceExpressionType =
               pos.start != pos.end || !prettySignature.endsWith(prettyType),
-            range = if (range.isRange) Some(range.toLsp) else None
+            range = if (range.isRange) Some(range.toLsp) else None,
+            contextInfo = history.getUsedRenamesInfo()
           )
         )
       }

diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/MetalsGlobal.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/MetalsGlobal.scala
@@ -311,7 +311,7 @@ class MetalsGlobal(
                 !metalsConfig.isDefaultSymbolPrefixes || hasConflictingMembersInScope
 
               if (shouldRenamePrefix) {
-                val existingRename = history.renames.get(ownerSym)
+                val existingRename = history.rename(ownerSym)
                 existingRename.isEmpty && history.tryShortenName(
                   ShortName(rename, ownerSymbol)
                 )
@@ -329,9 +329,8 @@ class MetalsGlobal(
                   args.map(arg => loop(arg, None))
                 )
               case _ =>
-                history.renames.get(sym) match {
-                  case Some(rename)
-                      if history.nameResolvesToSymbol(rename, sym) =>
+                history.rename(sym) match {
+                  case Some(rename) =>
                     TypeRef(
                       NoPrefix,
                       sym.newErrorSymbol(rename),
@@ -411,7 +410,7 @@ class MetalsGlobal(
           // to make sure we always use renamed package
           // what is saved in renames is actually companion module of a package
           val renamedOwnerIndex =
-            owners.indexWhere(s => history.renames.contains(s.companionModule))
+            owners.indexWhere(s => history.rename(s.companionModule).nonEmpty)
           if (renamedOwnerIndex < 0 && history.tryShortenName(name)) NoPrefix
           else {
             val prefix =
@@ -434,8 +433,8 @@ class MetalsGlobal(
                 .reverse
                 .map(s =>
                   m.Term.Name(
-                    history.renames
-                      .get(s.companionModule)
+                    history
+                      .rename(s.companionModule)
                       .map(_.toString())
                       .getOrElse(s.nameSyntax)
                   )

diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/Signatures.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/Signatures.scala
@@ -102,9 +102,22 @@ trait Signatures { compiler: MetalsGlobal =>
       val history: mutable.Map[Name, ShortName] = mutable.Map.empty,
       val lookupSymbol: Name => List[NameLookup] = _ => Nil,
       val config: collection.Map[Symbol, Name] = Map.empty,
-      val renames: collection.Map[Symbol, Name] = Map.empty,
+      renames: collection.Map[Symbol, Name] = Map.empty,
       val owners: collection.Set[Symbol] = Set.empty
   ) {
+
+    private val lookedUpRenames = mutable.Set[Symbol]()
+
+    def rename(sym: Symbol): Option[Name] = {
+      lookedUpRenames.add(sym)
+      renames.get(sym)
+    }
+
+    def getUsedRenamesInfo(): List[String] =
+      lookedUpRenames.flatMap { key =>
+        renames.get(key).map(v => s"type $v = ${key.nameString}")
+      }.toList
+
     def this(context: Context) =
       this(lookupSymbol = { name =>
         context.lookupSymbol(name, _ => true) :: Nil

diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/HoverProvider.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/HoverProvider.scala
@@ -32,16 +32,19 @@ object HoverProvider:
     val uri = params.uri
     val sourceFile = CompilerInterfaces.toSource(params.uri, params.text)
     driver.run(uri, sourceFile)
+    val unit = driver.compilationUnits.get(uri)
 
-    given ctx: Context = driver.currentCtx
+    given ctx: Context =
+      val ctx = driver.currentCtx
+      unit.map(ctx.fresh.setCompilationUnit).getOrElse(ctx)
     val pos = driver.sourcePosition(params)
-    val trees = driver.openedTrees(uri)
+    val path = unit
+      .map(unit => Interactive.pathTo(unit.tpdTree, pos.span))
+      .getOrElse(Interactive.pathTo(driver.openedTrees(uri), pos))
     val indexedContext = IndexedContext(ctx)
-
     def typeFromPath(path: List[Tree]) =
       if path.isEmpty then NoType else path.head.tpe
 
-    val path = Interactive.pathTo(trees, pos)
     val tp = typeFromPath(path)
     val tpw = tp.widenTermRefExpr
     // For expression we need to find all enclosing applies to get the exact generic type
@@ -67,14 +70,16 @@ object HoverProvider:
               |path:
               |${path
                .map(tree => s"""|```scala
-                                |$tree
+                                |${tree.show}
                                 |```
                                 |""".stripMargin)
                .mkString("\n")}
               |trees:
-              |${trees
+              |${unit
+               .map(u => List(u.tpdTree))
+               .getOrElse(driver.openedTrees(uri).map(_.tree))
                .map(tree => s"""|```scala
-                                |$tree
+                                |${tree.show}
                                 |```
                                 |""".stripMargin)
                .mkString("\n")}
@@ -89,16 +94,9 @@ object HoverProvider:
     else
       val skipCheckOnName =
         !pos.isPoint // don't check isHoveringOnName for RangeHover
-
-      val printerContext =
-        driver.compilationUnits.get(uri) match
-          case Some(unit) =>
-            val newctx =
-              ctx.fresh.setCompilationUnit(unit)
-            MetalsInteractive.contextOfPath(enclosing)(using newctx)
-          case None => ctx
+      val printerCtx = MetalsInteractive.contextOfPath(path)
       val printer = MetalsPrinter.standard(
-        IndexedContext(printerContext),
+        IndexedContext(printerCtx),
         search,
         includeDefaultParam = MetalsPrinter.IncludeDefaultParam.Include,
       )
@@ -152,6 +150,7 @@ object HoverProvider:
                   symbolSignature = Some(hoverString),
                   docstring = Some(docString),
                   forceExpressionType = forceExpressionType,
+                  contextInfo = printer.getUsedRenamesInfo(),
                 )
               )
             case _ =>
@@ -185,6 +184,7 @@ object HoverProvider:
               new ScalaHover(
                 expressionType = Some(tpeString),
                 symbolSignature = Some(s"$valOrDef $name$tpeString"),
+                contextInfo = printer.getUsedRenamesInfo(),
               )
             )
           case RefinedType(parent, _, _) =>

diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/printer/DotcPrinter.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/printer/DotcPrinter.scala
@@ -91,9 +91,15 @@ object DotcPrinter:
         case tp: AppliedType =>
           tp.tycon match
             case p: PrettyType =>
-              Str(p.toString) ~ "[" ~ toText(tp.args, ", ") ~ "]"
+              val args: List[Text] =
+                tp.args.map {
+                  case _: TypeBounds => "?"
+                  case arg => toText(arg)
+                }
+              Str(p.toString) ~ "[" ~ Text(args, ", ") ~ "]"
             case other => super.toText(tp)
         case other => super.toText(tp)
+
   end Std
 
   /**

diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/printer/MetalsPrinter.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/printer/MetalsPrinter.scala
@@ -55,6 +55,8 @@ class MetalsPrinter(
 
   def shortenedNames: List[ShortName] = names.namesToImport
 
+  def getUsedRenamesInfo(): List[String] = names.getUsedRenamesInfo
+
   def expressionType(tpw: Type)(using Context): Option[String] =
     tpw match
       case t: PolyType =>

diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/printer/ShortenedNames.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/printer/ShortenedNames.scala
@@ -29,6 +29,13 @@ class ShortenedNames(
 
   private val history = collection.mutable.Map.empty[Name, ShortName]
 
+  private val foundRenames = collection.mutable.Map.empty[Symbol, String]
+
+  def getUsedRenamesInfo(using Context): List[String] =
+    foundRenames.map { (from, to) =>
+      s"type $to = ${from.showName}"
+    }.toList
+
   /**
    * Returns a list of shortened names
    */
@@ -137,6 +144,7 @@ class ShortenedNames(
                   // for example, we have `import java.lang.{Boolean => JBoolean}` and
                   // complete `java.lang.Boolean`. See `CompletionOverrideSuite`'s `rename'.
                   case Some(rename) =>
+                    foundRenames += (h -> rename)
                     PrettyType(
                       (rename :: prev.map(_.name)).mkString(".")
                     )
@@ -147,6 +155,7 @@ class ShortenedNames(
             processOwners(sym, Nil, sym.ownersIterator.toList)
           renames.get(sym.owner) match
             case Some(rename) =>
+              foundRenames += (sym.owner -> rename)
               val short = ShortName(Names.termName(rename), sym.owner)
               if tryShortenName(short) then
                 PrettyType(s"$rename.${sym.name.show}")

diff --git a/tests/cross/src/test/scala/tests/hover/HoverDefnSuite.scala b/tests/cross/src/test/scala/tests/hover/HoverDefnSuite.scala
@@ -171,19 +171,15 @@ class HoverDefnSuite extends BaseHoverSuite {
   )
 
   check(
-    "package",
+    "package".tag(IgnoreForScala3CompilerPC),
     """package b.p@@kg
       |object Main
       |""".stripMargin,
     """```scala
       |package b.pkg
       |```
       |""".stripMargin,
-    automaticPackage = false,
-    compat = Map(
-      // TODO hover doesn't show information on package
-      "3" -> "".hover
-    )
+    automaticPackage = false
   )
 
   check(