Fix LSP not showing types in untyped code (#1889)

Most of the time, hovering over a symbol in untyped code would show
`Dyn` even though the symbol has a type annotation - in particular for
records accessed via a non-trivial path, e.g. `foo.bar.baz`. This is an
issue because in particular, we don't get to see the right types for
stdlib symbols when hovering over them.

This commit fixes the issue by taking the type annotation instead of the
type provided by the typechecker when the latter is `Dyn`.

lsp/nls/src/requests/hover.rs

@@ -156,20 +156,51 @@ pub fn handle(
     if let Some(hover) = hover_data {
         let mut contents = Vec::new();
 
-        if let Some(ty) = hover.ty {
-            contents.push(nickel_string(ty.to_string()));
+        // If we can't determine a static type through the typechecker because we are outside of a
+        // statically typed block, but the term points to a definition with a type annotation, we
+        // use this annotation insted.
+        let mut type_annots: Vec<_> = hover
+            .metadata
+            .iter()
+            .filter_map(|m| Some(m.annotation.typ.as_ref()?.typ.to_string()))
+            .collect();
+
+        let ty = hover
+            .ty
+            .as_ref()
+            .map(Type::to_string)
+            // Unclear whether it's useful to report `Dyn` all the time when there's no type found,
+            // but it matches the old behavior.
+            .unwrap_or_else(|| "Dyn".to_owned());
+
+        // If the type is `Dyn`, and we can find a type annotation somewhere in the metadata, we
+        // use the latter instead, which will be more precise.
+        let ty = if ty == "Dyn" {
+            // Ordering isn't meaningful here: metadata are aggregated from merged values (and
+            // merge is commutative). This list will also be sorted for deduplication later anyway.
+            // So we just pop the last one.
+            type_annots.pop().unwrap_or(ty)
         } else {
-            // Unclear whether it's useful to report `Dyn` all the
-            // time, but it matches the old behavior.
-            contents.push(nickel_string("Dyn".to_string()));
-        }
+            // If the type is both statically known and present as an annotation, we don't want to
+            // report a second time with the other contracts, so we remove it from the list.
+            //
+            // Note that there might be duplicates, and we need to remove them all, hence the
+            // `retain` (instead of a potentially more performant `iter().position(..)` followed by
+            // `swap_remove`).
+            type_annots.retain(|annot| annot != &ty);
+
+            ty
+        };
+
+        contents.push(nickel_string(ty));
 
         let mut contracts: Vec<_> = hover
             .metadata
             .iter()
             .flat_map(|m| &m.annotation.contracts)
             .chain(hover.values.iter().flat_map(annotated_contracts))
             .map(|contract| contract.label.typ.to_string())
+            .chain(type_annots)
             .collect();
 
         contracts.sort();

lsp/nls/tests/inputs/hover-stdlib-type.ncl

@@ -0,0 +1,12 @@
+### /main.ncl
+let foo = { bar.baz : String -> Number = fun x => 1 }
+in
+foo.bar.baz "0" + std.string.to_number "1"
+### [[request]]
+### type = "Hover"
+### textDocument.uri = "file:///main.ncl"
+### position = { line = 2, character = 10 }
+### [[request]]
+### type = "Hover"
+### textDocument.uri = "file:///main.ncl"
+### position = { line = 2, character = 30 }

lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__hover-stdlib-type.ncl.snap

@@ -0,0 +1,19 @@
+---
+source: lsp/nls/tests/main.rs
+expression: output
+---
+<2:0-2:11>[```nickel
+String -> Number
+```]
+<2:18-2:38>[Converts a string that represents a number to that number.
+
+# Examples
+
+```nickel
+std.string.to_number "123"
+  => 123
+```, ```nickel
+NumberLiteral -> Dyn
+```, ```nickel
+String -> Number
+```]