Repository: tweag/nickel
Pull request #1594 (2023-09-11 12:04:57)

print nice logs without losing speedup
Bring back ` [build name] >` prefix to log lines, but maintain speed up by writing to non-tty file so nix doesn't bother with the status bar


Patch:
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
index 241dd87c..60066980 100644
--- a/.github/workflows/continuous-integration.yml
+++ b/.github/workflows/continuous-integration.yml
@@ -54,7 +54,14 @@ jobs:
 
     - name: Run all checks
       run: |
-          nix flake check --log-format raw-with-logs
+          # see https://github.com/NixOS/nix/issues/8949 for why we redirect to
+          # an output file. Everything else is just capturing the error code
+          # while still printing the logs
+          set +e
+          nix flake check --print-build-logs > logs 2>&1
+          errcode=$?
+          cat logs
+          exit $errcode
 
     - name: Typecheck benchmarks
       run: find benches -type f -name "*.ncl" -print0 | xargs -0 -I file nix run . -- -f file typecheck
diff --git a/.github/workflows/release-artifacts.yaml b/.github/workflows/release-artifacts.yaml
index 2438bee8..5422f043 100644
--- a/.github/workflows/release-artifacts.yaml
+++ b/.github/workflows/release-artifacts.yaml
@@ -19,7 +19,7 @@ jobs:
     outputs:
       instance_id: ${{ steps.invoke-start.outputs.INSTANCE_ID }}
     steps:
-      - uses: aws-actions/configure-aws-credentials@v3
+      - uses: aws-actions/configure-aws-credentials@v4
         with:
           role-to-assume: ${{ secrets.EC2_ROLE }}
           aws-region: ${{ vars.EC2_REGION }}
@@ -53,7 +53,7 @@ jobs:
       - start-runner
       - docker-multiplatform-image
     steps:
-      - uses: aws-actions/configure-aws-credentials@v3
+      - uses: aws-actions/configure-aws-credentials@v4
         with:
           role-to-assume: ${{ secrets.EC2_ROLE }}
           aws-region: ${{ vars.EC2_REGION }}
diff --git a/core/src/typecheck/linearization.rs b/core/src/typecheck/linearization.rs
index 02651aa5..cb7d7ad1 100644
--- a/core/src/typecheck/linearization.rs
+++ b/core/src/typecheck/linearization.rs
@@ -79,6 +79,7 @@ impl LinearizationState for Uninit {}
 pub trait Linearizer {
     type Building: LinearizationState;
     type Completed: LinearizationState + Default;
+    type ItemId: Copy;
     type CompletionExtra;
 
     /// Record a new term.
@@ -88,13 +89,17 @@ pub trait Linearizer {
     ///
     /// In practice this mainly includes environment information. Providing this
     /// state is the responsibility of [Linearizer::scope]
+    ///
+    /// Returns an item id that can then be used to complete e.g. type information. Some terms
+    /// might give rise to several items in the linearization, but usually, there's a main one that
+    /// is relevant for amending the type. The result can be `None` if the term doesn't have a
+    /// location. In this case, the LSP currently ignores it.
     fn add_term(
         &mut self,
         _lin: &mut Linearization<Self::Building>,
         _term: &RichTerm,
         _ty: UnifType,
-    ) {
-    }
+    ) -> Option<Self::ItemId>;
 
     /// Record the metadata of a record field. The record field is guaranteed to be linearized
     /// next, if the it has a value.
@@ -115,6 +120,18 @@ pub trait Linearizer {
     ) {
     }
 
+    /// Allows to amend the type of an item.
+    ///
+    /// Because [Self::add_term] returns an optional, it's more ergonomic to have `retype` accept an
+    /// optional as well. If the item id is `None`, `retype` simply does nothing.
+    fn retype(
+        &mut self,
+        _lin: &mut Linearization<Self::Building>,
+        _item_id: Option<Self::ItemId>,
+        _new_type: UnifType,
+    ) {
+    }
+
     /// Defines how to turn a [Self::Building] Linearization of the tracked type into
     /// a [Self::Completed] linearization.
     /// By default creates an entirely empty [Self::Completed] object
@@ -159,6 +176,16 @@ where
     type Building = B;
     type Completed = C;
     type CompletionExtra = E;
+    type ItemId = ();
+
+    fn add_term(
+        &mut self,
+        _lin: &mut Linearization<Self::Building>,
+        _term: &RichTerm,
+        _ty: UnifType,
+    ) -> Option<()> {
+        None
+    }
 
     fn scope(&mut self) -> Self {
         StubHost::new()
diff --git a/core/src/typecheck/mod.rs b/core/src/typecheck/mod.rs
index 32fe0013..cd6b24f1 100644
--- a/core/src/typecheck/mod.rs
+++ b/core/src/typecheck/mod.rs
@@ -1659,15 +1659,33 @@ fn walk_with_annot<L: Linearizer>(
 ///
 /// [bidirectional-typing]: (https://arxiv.org/abs/1908.05839)
 fn check<L: Linearizer>(
+    state: &mut State,
+    ctxt: Context,
+    lin: &mut Linearization<L::Building>,
+    linearizer: L,
+    rt: &RichTerm,
+    ty: UnifType,
+) -> Result<(), TypecheckError> {
+    check_visited(state, ctxt, lin, linearizer, rt, ty, None)
+}
+
+/// Variant of [check] taking an additional `item_id` to indicate that checking is done on a term
+/// that has already been visited by the linearizer, for example when coming from `infer`. When
+/// `item_id` is defined, the linearizer isn't called. [check] is just [check_visited] called with
+/// `item_id` set to `None`.
+fn check_visited<L: Linearizer>(
     state: &mut State,
     mut ctxt: Context,
     lin: &mut Linearization<L::Building>,
     mut linearizer: L,
     rt: &RichTerm,
     ty: UnifType,
+    item_id: Option<L::ItemId>,
 ) -> Result<(), TypecheckError> {
     let RichTerm { term: t, pos } = rt;
-    linearizer.add_term(lin, rt, ty.clone());
+
+    let item_id = item_id.or_else(|| linearizer.add_term(lin, rt, ty.clone()));
+
     // When checking against a polymorphic type, we immediatly instantiate potential heading
     // foralls. Otherwise, this polymorphic type wouldn't unify much with other types. If we infer
     // a polymorphic type for `rt`, the subsumption rule will take care of instantiating this type
@@ -1887,7 +1905,7 @@ fn check<L: Linearizer>(
         | Term::Op2(..)
         | Term::OpN(..)
         | Term::Annotated(..) => {
-            let inferred = infer(state, ctxt.clone(), lin, linearizer, rt)?;
+            let inferred = infer_visited(state, ctxt.clone(), lin, linearizer, rt, item_id)?;
 
             // We call to `subsumption` to perform the switch from infer mode to checking mode.
             subsumption(state, ctxt, inferred, ty)
@@ -2109,12 +2127,14 @@ fn check_field<L: Linearizer>(
             linearizer,
             &field.metadata.annotation,
             field.value.as_ref(),
+            None,
         )?;
 
         subsumption(state, ctxt, inferred, ty).map_err(|err| err.into_typecheck_err(state, pos))
     }
 }
 
+/// `item_id` is the id of the linearization item corresponding to the annotated term.
 fn infer_annotated<L: Linearizer>(
     state: &mut State,
     ctxt: Context,
@@ -2122,14 +2142,19 @@ fn infer_annotated<L: Linearizer>(
     linearizer: L,
     annot: &TypeAnnotation,
     rt: &RichTerm,
+    item_id: Option<L::ItemId>,
 ) -> Result<UnifType, TypecheckError> {
-    infer_with_annot(state, ctxt, lin, linearizer, annot, Some(rt))
+    infer_with_annot(state, ctxt, lin, linearizer, annot, Some(rt), item_id)
 }
 
 /// Function handling the common part of inferring the type of terms with type or contract
 /// annotation, with or without definitions. This encompasses both standalone type annotation
 /// (where `value` is always `Some(_)`) as well as field definitions (where `value` may or may not
 /// be defined).
+///
+/// As for [check_visited] and [infer_visited], the additional `item_id` is provided when the term
+/// has been added to the linearizer before but can still benefit from updating its information
+/// with the inferred type.
 #[allow(clippy::too_many_arguments)] // TODO: Is it worth doing something about it?
 fn infer_with_annot<L: Linearizer>(
     state: &mut State,
@@ -2138,6 +2163,7 @@ fn infer_with_annot<L: Linearizer>(
     mut linearizer: L,
     annot: &TypeAnnotation,
     value: Option<&RichTerm>,
+    item_id: Option<L::ItemId>,
 ) -> Result<UnifType, TypecheckError> {
     annot
         .iter()
@@ -2153,10 +2179,12 @@ fn infer_with_annot<L: Linearizer>(
         ) => {
             let uty2 = UnifType::from_type(ty2.clone(), &ctxt.term_env);
 
+            linearizer.retype(lin, item_id, uty2.clone());
+
             check(state, ctxt, lin, linearizer, value, uty2.clone())?;
             Ok(uty2)
         }
-        // A annotation without a type but with a contract switches the typechecker back to walk
+        // An annotation without a type but with a contract switches the typechecker back to walk
         // mode. If there are several contracts, we arbitrarily chose the first one as the apparent
         // type (the most precise type would be the intersection of all contracts, but Nickel's
         // type system doesn't feature intersection types).
@@ -2172,6 +2200,8 @@ fn infer_with_annot<L: Linearizer>(
 
             let uty2 = UnifType::from_type(ty2.clone(), &ctxt.term_env);
 
+            linearizer.retype(lin, item_id, uty2.clone());
+
             // If there's an inner value, we have to walk it, as it may contain statically typed
             // blocks.
             if let Some(value) = value_opt {
@@ -2181,10 +2211,13 @@ fn infer_with_annot<L: Linearizer>(
             Ok(uty2)
         }
         // A non-empty value without a type or a contract annotation is typechecked in the same way
-        // as its inner value
+        // as its inner value. This case should only happen for record fields, as the parser can't
+        // produce an annotated term without an actual annotation. Still, such terms could be
+        // produced programmatically, and aren't necessarily an issue.
         (_, Some(value)) => infer(state, ctxt, lin, linearizer, value),
-        // A empty value is a record field without definition. We don't check anything, and infer
+        // An empty value is a record field without definition. We don't check anything, and infer
         // its type to be either the first annotation defined if any, or `Dyn` otherwise.
+        // We can only hit this case for record fields.
         //
         // TODO: we might have something to with the linearizer to clear the current metadata.
         // It looks like it may be unduly attached to the next field definition, which is not
@@ -2204,18 +2237,35 @@ fn infer_with_annot<L: Linearizer>(
 /// `infer` corresponds to the inference mode of bidirectional typechecking. Nickel uses a mix of
 /// bidirectional typechecking and traditional ML-like unification.
 fn infer<L: Linearizer>(
+    state: &mut State,
+    ctxt: Context,
+    lin: &mut Linearization<L::Building>,
+    linearizer: L,
+    rt: &RichTerm,
+) -> Result<UnifType, TypecheckError> {
+    infer_visited(state, ctxt, lin, linearizer, rt, None)
+}
+
+/// Variant of [infer] taking an additional `item_id` to indicate that inference is done on a term
+/// that has already been visited by the linearizer, for example when coming directly from [check].
+/// When `item_id` is defined, the linearizer isn't called. [infer] is just `infer_visited` called
+/// with `item_id` set to `None`.
+///
+/// Whether `item_id` is `Some` or `None`, the item is always amended with its inferred type.
+fn infer_visited<L: Linearizer>(
     state: &mut State,
     mut ctxt: Context,
     lin: &mut Linearization<L::Building>,
     mut linearizer: L,
     rt: &RichTerm,
+    item_id: Option<L::ItemId>,
 ) -> Result<UnifType, TypecheckError> {
     let RichTerm { term, pos } = rt;
 
-    // For the following infer rules, we need to be careful to call to `linearizer.add_term` at the
-    // right moment. We need to provide a type, so we can't do it before having inferred a type for
-    // `rt`. However, we must do it **before** any subsequent calls to `check` or `infer`, because
-    // the linearizer relies on terms being visited in order (pre-order).
+    // We don't have a type yet, so we put `Dyn` for now, but we'll update once we infer one via
+    // `linearizer.retype`
+    let item_id = item_id.or_else(|| linearizer.add_term(lin, rt, mk_uniftype::dynamic()));
+
     match term.as_ref() {
         Term::Var(x) => {
             let x_ty = ctxt
@@ -2224,7 +2274,8 @@ fn infer<L: Linearizer>(
                 .cloned()
                 .ok_or(TypecheckError::UnboundIdentifier(*x, *pos))?;
 
-            linearizer.add_term(lin, rt, x_ty.clone());
+            linearizer.retype(lin, item_id, x_ty.clone());
+
             Ok(x_ty)
         }
         // Theoretically, we need to instantiate the type of the head of the primop application,
@@ -2234,7 +2285,8 @@ fn infer<L: Linearizer>(
         // the type of a primop is currently always monomorphic.
         Term::Op1(op, t) => {
             let (ty_arg, ty_res) = get_uop_type(state, ctxt.var_level, op)?;
-            linearizer.add_term(lin, rt, ty_res.clone());
+
+            linearizer.retype(lin, item_id, ty_res.clone());
 
             check(state, ctxt.clone(), lin, linearizer.scope(), t, ty_arg)?;
 
@@ -2242,7 +2294,8 @@ fn infer<L: Linearizer>(
         }
         Term::Op2(op, t1, t2) => {
             let (ty_arg1, ty_arg2, ty_res) = get_bop_type(state, ctxt.var_level, op)?;
-            linearizer.add_term(lin, rt, ty_res.clone());
+
+            linearizer.retype(lin, item_id, ty_res.clone());
 
             check(state, ctxt.clone(), lin, linearizer.scope(), t1, ty_arg1)?;
             check(state, ctxt.clone(), lin, linearizer, t2, ty_arg2)?;
@@ -2251,7 +2304,8 @@ fn infer<L: Linearizer>(
         }
         Term::OpN(op, args) => {
             let (tys_args, ty_res) = get_nop_type(state, ctxt.var_level, op)?;
-            linearizer.add_term(lin, rt, ty_res.clone());
+
+            linearizer.retype(lin, item_id, ty_res.clone());
 
             tys_args.into_iter().zip(args.iter()).try_for_each(
                 |(ty_arg, arg)| -> Result<_, TypecheckError> {
@@ -2280,18 +2334,24 @@ fn infer<L: Linearizer>(
                 .unify(head, state, &ctxt)
                 .map_err(|err| err.into_typecheck_err(state, e.pos))?;
 
-            linearizer.add_term(lin, rt, codom.clone());
+            linearizer.retype(lin, item_id, codom.clone());
+
             check(state, ctxt.clone(), lin, linearizer, t, dom)?;
             Ok(codom)
         }
-        Term::Annotated(annot, rt) => infer_annotated(state, ctxt, lin, linearizer, annot, rt),
+        Term::Annotated(annot, rt) => {
+            infer_annotated(state, ctxt, lin, linearizer, annot, rt, item_id)
+        }
         _ => {
             // The remaining cases can't produce polymorphic types, and thus we can reuse the
             // checking code. Inferring the type for those rules is equivalent to checking against
             // a free unification variable. This saves use from duplicating all the remaining
             // cases.
             let inferred = state.table.fresh_type_uvar(ctxt.var_level);
-            check(state, ctxt, lin, linearizer, rt, inferred.clone())?;
+
+            linearizer.retype(lin, item_id, inferred.clone());
+
+            check_visited(state, ctxt, lin, linearizer, rt, inferred.clone(), item_id)?;
             Ok(inferred.into_root(state.table))
         }
     }
@@ -2301,17 +2361,19 @@ fn infer<L: Linearizer>(
 ///
 /// Call [`apparent_type`] to see if the binding is annotated. If it is, return this type as a
 /// [`UnifType`]. Otherwise:
-///     * in non strict mode, we won't (and possibly can't) infer the type of `bound_exp`: just
-///       return `Dyn`.
-///     * in strict mode, we will typecheck `bound_exp`: return a new unification variable to be
-///       associated to `bound_exp`.
-/// As this function is always called in a context where an `ImportResolver` is present, expect
-/// it passed in arguments.
+///
+/// - in walk mode, we won't (and possibly can't) infer the type of `bound_exp`: just return `Dyn`.
+/// - in typecheck mode, we will typecheck `bound_exp`: return a new unification variable to be
+///   associated to `bound_exp`.
+///
+/// As this function is always called in a context where an `ImportResolver` is present, expect it
+/// passed in arguments.
 ///
 /// If the annotated type contains any wildcard:
-///     * in non strict mode, wildcards are assigned `Dyn`.
-///     * in strict mode, the wildcard is typechecked, and we return the unification variable
-///       corresponding to it.
+///
+/// - in non strict mode, wildcards are assigned `Dyn`.
+/// - in strict mode, the wildcard is typechecked, and we return the unification variable
+///   corresponding to it.
 fn binding_type(state: &mut State, t: &Term, ctxt: &Context, strict: bool) -> UnifType {
     apparent_or_infer(
         state,
diff --git a/lsp/lsp-harness/src/lib.rs b/lsp/lsp-harness/src/lib.rs
index 6b782d2a..660b15a1 100644
--- a/lsp/lsp-harness/src/lib.rs
+++ b/lsp/lsp-harness/src/lib.rs
@@ -3,7 +3,9 @@ mod output;
 
 pub use jsonrpc::Server;
 use log::error;
-use lsp_types::{CompletionParams, DocumentFormattingParams, GotoDefinitionParams, Url};
+use lsp_types::{
+    CompletionParams, DocumentFormattingParams, GotoDefinitionParams, HoverParams, Url,
+};
 pub use output::LspDebug;
 use serde::Deserialize;
 
@@ -30,6 +32,7 @@ pub enum Request {
     GotoDefinition(GotoDefinitionParams),
     Completion(CompletionParams),
     Formatting(DocumentFormattingParams),
+    Hover(HoverParams),
 }
 
 #[derive(Deserialize, Debug, Default)]
diff --git a/lsp/lsp-harness/src/output.rs b/lsp/lsp-harness/src/output.rs
index b02fdfc0..f2a8667f 100644
--- a/lsp/lsp-harness/src/output.rs
+++ b/lsp/lsp-harness/src/output.rs
@@ -121,3 +121,39 @@ impl LspDebug for Vec<lsp_types::TextEdit> {
         Iter(self.iter()).debug(w)
     }
 }
+
+impl LspDebug for lsp_types::Hover {
+    fn debug(&self, mut w: impl Write) -> std::io::Result<()> {
+        write!(w, "<{}>", self.range.debug_str())?;
+
+        match &self.contents {
+            lsp_types::HoverContents::Scalar(s) => s.debug(w),
+            lsp_types::HoverContents::Array(items) => {
+                let mut items = items.clone();
+                items.sort_by_cached_key(|i| match i {
+                    lsp_types::MarkedString::String(s) => s.clone(),
+                    lsp_types::MarkedString::LanguageString(l_str) => l_str.value.clone(),
+                });
+                Iter(items.iter()).debug(w)
+            }
+            lsp_types::HoverContents::Markup(s) => s.debug(w),
+        }
+    }
+}
+
+impl LspDebug for lsp_types::MarkedString {
+    fn debug(&self, mut w: impl Write) -> std::io::Result<()> {
+        match self {
+            lsp_types::MarkedString::String(s) => write!(w, "{s}"),
+            lsp_types::MarkedString::LanguageString(l_str) => {
+                write!(w, "```{}\n{}\n```", l_str.language, l_str.value)
+            }
+        }
+    }
+}
+
+impl LspDebug for lsp_types::MarkupContent {
+    fn debug(&self, mut w: impl Write) -> std::io::Result<()> {
+        write!(w, "{}", self.value)
+    }
+}
diff --git a/lsp/nls/src/linearization/building.rs b/lsp/nls/src/linearization/building.rs
index 7c9a5e4b..0e6d5f56 100644
--- a/lsp/nls/src/linearization/building.rs
+++ b/lsp/nls/src/linearization/building.rs
@@ -15,7 +15,7 @@ use crate::linearization::interface::{TermKind, UsageState};
 
 use super::{
     interface::{Unresolved, ValueState},
-    Environment, IdGen, ItemId, LinRegistry, LinearizationItem,
+    Environment, ItemId, LinRegistry, LinearizationItem,
 };
 
 /// A concrete [LinearizationState]
@@ -88,7 +88,7 @@ impl<'b> Building<'b> {
             // specific case, we simply ignore it (meaning that one can't do goto references for
             // `std` currently).
             TermKind::Record(_) => (),
-            // unreachable()!: add_usage can only called on let binding, functions and record
+            // unreachable()!: add_usage can only be called on let bindings, functions and record
             // fields, only referring to items which support usages.
             TermKind::Type(_) | TermKind::Structure | TermKind::Usage(_) => unreachable!(),
             TermKind::Declaration { ref mut usages, .. }
@@ -122,8 +122,9 @@ impl<'b> Building<'b> {
         for (ident, field) in record_fields.iter() {
             let id = ItemId {
                 file_id: current_file,
-                index: self.id_gen().get_and_advance(),
+                index: self.next_id(),
             };
+
             self.push(LinearizationItem {
                 env: env.clone(),
                 term: record_term.clone(),
@@ -139,6 +140,7 @@ impl<'b> Building<'b> {
                 },
                 metadata: Some(field.metadata.clone()),
             });
+
             env.insert(ident.ident(), id);
             self.add_record_field(current_file, record, (ident.ident(), id))
         }
@@ -306,8 +308,11 @@ impl<'b> Building<'b> {
         }
     }
 
-    pub(super) fn id_gen(&self) -> IdGen {
-        IdGen::new(self.linearization.len())
+    /// Return the id of the next item to be inserted. Note that `next_id` doesn't mutate anything:
+    /// as long as no new item is inserted in the linearization, calling `next_id` multiple time
+    /// will return the same result, namely the size of the current linearization.
+    pub(super) fn next_id(&self) -> usize {
+        self.linearization.len()
     }
 }
 
diff --git a/lsp/nls/src/linearization/mod.rs b/lsp/nls/src/linearization/mod.rs
index 04d2b1ed..6b039772 100644
--- a/lsp/nls/src/linearization/mod.rs
+++ b/lsp/nls/src/linearization/mod.rs
@@ -131,7 +131,7 @@ pub struct AnalysisHost<'a> {
     /// ```
     ///
     /// To resolve those inner fields, accessors (`inner`, `middle`)
-    /// are recorded first until a variable (`outer`). is found.
+    /// are recorded first until a variable (`outer`) is found.
     /// Then, access to all nested records are resolved at once.
     access: Option<Vec<LocIdent>>,
 }
@@ -148,6 +148,56 @@ impl<'a> AnalysisHost<'a> {
             access: Default::default(),
         }
     }
+
+    fn next_id(&self, lin: &Linearization<Building>) -> ItemId {
+        ItemId {
+            file_id: self.file,
+            index: lin.next_id(),
+        }
+    }
+
+    // Helper with logic common to let/fun and let pattern/fun pattern.
+    //
+    // If the term is a let-binding, this function calls to the provided `push` callback to add the next
+    // item id to the relevant list of bindings.
+    //
+    // If the term is a function, this function generates a stub item for it.
+    //
+    // Returns the value state of the corresponding declaration.
+    //
+    // Panic if `rt` is neither a let/let pattern nor a fun/fun pattern.
+    fn setup_decl(
+        &mut self,
+        lin: &mut Linearization<Building>,
+        rt: &RichTerm,
+        ty: &UnifType,
+        pos: TermPos,
+        push: impl FnOnce(ItemId),
+    ) -> ValueState {
+        let next_id = self.next_id(lin);
+
+        match rt.as_ref() {
+            Term::LetPattern(..) | Term::Let(..) => {
+                push(next_id);
+                ValueState::Unknown
+            }
+            Term::FunPattern(..) | Term::Fun(..) => {
+                // stub object, representing the whole function
+                lin.push(LinearizationItem {
+                    env: self.env.clone(),
+                    term: rt.clone(),
+                    id: next_id,
+                    ty: ty.clone(),
+                    pos,
+                    kind: TermKind::Structure,
+                    metadata: self.meta.take(),
+                });
+
+                ValueState::Known(self.next_id(lin))
+            }
+            _ => panic!("expected only a let pattern or a fun pattern in this function"),
+        }
+    }
 }
 
 use nickel_lang_core::typ::Type;
@@ -156,12 +206,25 @@ impl<'a> Linearizer for AnalysisHost<'a> {
     type Building = Building<'a>;
     type Completed = Completed;
     type CompletionExtra = Extra;
+    type ItemId = ItemId;
 
-    fn add_term(&mut self, lin: &mut Linearization<Building>, rt: &RichTerm, ty: UnifType) {
+    fn add_term(
+        &mut self,
+        lin: &mut Linearization<Building>,
+        rt: &RichTerm,
+        ty: UnifType,
+    ) -> Option<ItemId> {
         let pos = rt.pos;
         let term = rt.term.as_ref();
         debug!("adding term: {:?} @ {:?}", term, pos);
-        let mut id_gen = lin.id_gen();
+
+        // The id of the main item added. Some terms give rise to several items (records or
+        // functions for example), but this is the id of the first item added, which is considered
+        // to be the item attached to the term - the rest is extra declaration/usage/etc
+        let main_id = ItemId {
+            file_id: self.file,
+            index: lin.next_id(),
+        };
 
         // Register record field if appropriate
         // `record` is the id [LinearizatonItem] of the enclosing record
@@ -180,7 +243,6 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 .map(|(record, mut fields)| (record, fields.pop().unwrap()))
             {
                 if let Some(field) = lin.linearization.get_mut(record.index + offset.index) {
-                    debug!("{:?}", field.kind);
                     let usage_offset = if matches!(term, Term::Var(_)) {
                         debug!(
                             "associating nested field {:?} with chain {:?}",
@@ -195,7 +257,7 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                         TermKind::RecordField { ref mut value, .. } => {
                             *value = ValueState::Known(ItemId {
                                 file_id: self.file,
-                                index: id_gen.get() + usage_offset,
+                                index: main_id.index + usage_offset,
                             });
                         }
                         // The linearization item of a record with n fields is expected to be
@@ -213,7 +275,7 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                         decl,
                         ItemId {
                             file_id: self.file,
-                            index: id_gen.get() + offset,
+                            index: main_id.index + offset,
                         },
                     );
                 }
@@ -221,51 +283,19 @@ impl<'a> Linearizer for AnalysisHost<'a> {
         }
 
         if pos == TermPos::None {
-            return;
+            return None;
         }
 
-        let id = ItemId {
-            file_id: self.file,
-            index: id_gen.get(),
-        };
         match term {
             Term::LetPattern(ident, destruct, ..) | Term::FunPattern(ident, destruct, _) => {
-                let mut binding = None;
-                if let Some(ident) = ident {
-                    let value_ptr = match term {
-                        Term::LetPattern(..) => {
-                            binding = Some(id);
-                            ValueState::Unknown
-                        }
-                        Term::FunPattern(..) => {
-                            // stub object, representing the whole function
-                            lin.push(LinearizationItem {
-                                env: self.env.clone(),
-                                term: rt.clone(),
-                                id: ItemId {
-                                    file_id: self.file,
-                                    index: id_gen.get_and_advance(),
-                                },
-
-                                ty: ty.clone(),
-                                pos,
-                                kind: TermKind::Structure,
-                                metadata: self.meta.take(),
-                            });
+                let mut pattern_bindings = Vec::new();
 
-                            ValueState::Known(ItemId {
-                                file_id: self.file,
-                                index: id_gen.get(),
-                            })
-                        }
-                        _ => unreachable!(),
-                    };
+                if let Some(ident) = ident {
+                    let value_ptr =
+                        self.setup_decl(lin, rt, &ty, pos, |id| pattern_bindings.push(id));
 
-                    let id = ItemId {
-                        file_id: self.file,
-                        index: id_gen.get_and_advance(),
-                    };
-                    self.env.insert(ident.ident(), id);
+                    let next_id = self.next_id(lin);
+                    self.env.insert(ident.ident(), next_id);
 
                     let kind = TermKind::Declaration {
                         id: ident.to_owned(),
@@ -273,10 +303,11 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                         value: value_ptr,
                         path: None,
                     };
+
                     lin.push(LinearizationItem {
                         env: self.env.clone(),
                         term: rt.clone(),
-                        id,
+                        id: next_id,
                         ty,
                         pos: ident.pos,
                         kind,
@@ -284,28 +315,21 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                     });
                 }
 
-                let mut let_pattern_bindings = if let Some(id) = binding {
-                    vec![id]
-                } else {
-                    Vec::new()
-                };
                 for (path, bind_ident, field) in destruct
                     .to_owned()
                     .inner()
                     .into_iter()
                     .flat_map(|matched| matched.to_flattened_bindings())
                 {
-                    let id = ItemId {
-                        file_id: self.file,
-                        index: id_gen.get_and_advance(),
-                    };
+                    let decl_id = self.next_id(lin);
+
+                    pattern_bindings.push(decl_id);
+                    self.env.insert(bind_ident.ident(), decl_id);
 
-                    let_pattern_bindings.push(id);
-                    self.env.insert(bind_ident.ident(), id);
                     lin.push(LinearizationItem {
                         env: self.env.clone(),
                         term: rt.clone(),
-                        id,
+                        id: decl_id,
                         // TODO: get type from pattern
                         ty: UnifType::concrete(TypeF::Dyn),
                         pos: bind_ident.pos,
@@ -318,44 +342,17 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                         metadata: Some(field.metadata),
                     });
                 }
-                self.bindings = Some(let_pattern_bindings);
+
+                self.bindings = Some(pattern_bindings);
             }
             Term::Let(ident, ..) | Term::Fun(ident, ..) => {
-                let value_ptr = match term {
-                    Term::Let(..) => {
-                        self.bindings = Some(vec![id]);
-                        ValueState::Unknown
-                    }
-                    Term::Fun(..) => {
-                        // stub object, representing the whole function
-                        lin.push(LinearizationItem {
-                            env: self.env.clone(),
-                            term: rt.clone(),
-                            id: ItemId {
-                                file_id: self.file,
-                                index: id_gen.get_and_advance(),
-                            },
+                let mut binding = None;
 
-                            ty: ty.clone(),
-                            pos,
-                            kind: TermKind::Structure,
-                            metadata: self.meta.take(),
-                        });
+                let value_ptr = self.setup_decl(lin, rt, &ty, pos, |id| binding = Some(id));
+                self.bindings = binding.map(|id| vec![id]);
 
-                        ValueState::Known(ItemId {
-                            file_id: self.file,
-                            index: id_gen.get(),
-                        })
-                    }
-                    _ => unreachable!(),
-                };
-                self.env.insert(
-                    ident.ident(),
-                    ItemId {
-                        file_id: self.file,
-                        index: id_gen.get(),
-                    },
-                );
+                let next_id = self.next_id(lin);
+                self.env.insert(ident.ident(), next_id);
 
                 let kind = TermKind::Declaration {
                     id: ident.to_owned(),
@@ -363,13 +360,11 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                     value: value_ptr,
                     path: None,
                 };
+
                 lin.push(LinearizationItem {
                     env: self.env.clone(),
                     term: rt.clone(),
-                    id: ItemId {
-                        file_id: self.file,
-                        index: id_gen.get(),
-                    },
+                    id: next_id,
                     ty,
                     pos: ident.pos,
                     kind,
@@ -377,21 +372,17 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 });
             }
             Term::Var(ident) => {
-                let root_id = ItemId {
-                    file_id: self.file,
-                    index: id_gen.get_and_advance(),
-                };
-
                 debug!(
                     "adding usage of variable {} followed by chain {:?}",
                     ident, self.access
                 );
 
                 let pointed = self.env.get(&ident.ident()).copied();
+
                 lin.push(LinearizationItem {
                     env: self.env.clone(),
                     term: rt.clone(),
-                    id: root_id,
+                    id: main_id,
                     pos: ident.pos,
                     ty: UnifType::concrete(TypeF::Dyn),
                     kind: TermKind::Usage(UsageState::from(pointed)),
@@ -399,17 +390,14 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 });
 
                 if let Some(referenced) = pointed {
-                    lin.add_usage(self.file, referenced, root_id)
+                    lin.add_usage(self.file, referenced, main_id)
                 }
 
                 if let Some(chain) = self.access.take() {
                     let chain: Vec<_> = chain.into_iter().rev().collect();
 
                     for accessor in chain.iter() {
-                        let id = ItemId {
-                            file_id: self.file,
-                            index: id_gen.get_and_advance(),
-                        };
+                        let id = self.next_id(lin);
 
                         lin.push(LinearizationItem {
                             env: self.env.clone(),
@@ -433,21 +421,21 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 lin.push(LinearizationItem {
                     env: self.env.clone(),
                     term: rt.clone(),
-                    id,
+                    id: main_id,
                     pos,
                     ty,
                     kind: TermKind::Record(HashMap::new()),
                     metadata: self.meta.take(),
                 });
 
-                lin.register_fields(self.file, rt, &record.fields, id, &mut self.env);
+                lin.register_fields(self.file, rt, &record.fields, main_id, &mut self.env);
                 let mut field_names = record.fields.keys().cloned().collect::<Vec<_>>();
                 field_names.sort_unstable();
 
                 self.record_fields = Some((
                     ItemId {
-                        file_id: id.file_id,
-                        index: id.index + 1,
+                        file_id: main_id.file_id,
+                        index: main_id.index + 1,
                     },
                     field_names
                         .into_iter()
@@ -480,7 +468,6 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 })
             }
             Term::ResolvedImport(file) => {
-                lin.import_locations.insert(*file, pos);
                 fn final_term_pos(term: &RichTerm) -> &TermPos {
                     let RichTerm { term, pos } = term;
                     match term.as_ref() {
@@ -492,9 +479,9 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                     }
                 }
 
-                let Some(linearization) = lin.lin_registry.map.get(file) else {
-                    return;
-                };
+                lin.import_locations.insert(*file, pos);
+
+                let linearization = lin.lin_registry.map.get(file)?;
 
                 // This is safe because the import file is resolved before we linearize the
                 // containing file, therefore the cache MUST have the term stored.
@@ -504,15 +491,12 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 // unwrap(): this unwrap fails only when position is a `TermPos::None`, which only
                 // happens if the `RichTerm` has been transformed or evaluated. None of these
                 // happen before linearization.
-                let Some(term_id) = linearization.item_at(position.unwrap().start_pos()) else {
-                    return;
-                };
-                let term_id = term_id.id;
+                let term_id = linearization.item_at(position.unwrap().start_pos())?.id;
 
                 lin.push(LinearizationItem {
                     env: self.env.clone(),
                     term: rt.clone(),
-                    id,
+                    id: main_id,
                     pos,
                     ty,
                     kind: TermKind::Usage(UsageState::Resolved(term_id)),
@@ -523,7 +507,7 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 lin.push(LinearizationItem {
                     env: self.env.clone(),
                     term: rt.clone(),
-                    id,
+                    id: main_id,
                     pos,
                     ty,
                     kind: TermKind::Type(t.clone()),
@@ -536,7 +520,7 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 lin.push(LinearizationItem {
                     env: self.env.clone(),
                     term: rt.clone(),
-                    id,
+                    id: main_id,
                     pos,
                     ty,
                     kind: TermKind::Structure,
@@ -544,6 +528,8 @@ impl<'a> Linearizer for AnalysisHost<'a> {
                 })
             }
         }
+
+        Some(main_id)
     }
 
     fn add_field_metadata(&mut self, _lin: &mut Linearization<Building>, field: &Field) {
@@ -694,28 +680,29 @@ impl<'a> Linearizer for AnalysisHost<'a> {
         {
             debug!("retyping {:?} to {:?}", ident, new_type);
             item.ty = new_type;
+        } else {
+            debug!(
+                "retype_indent failed! Environment miss: {}",
+                self.env.get(&ident.ident()).is_none()
+            );
         }
     }
-}
-
-struct IdGen(usize);
-
-impl IdGen {
-    /// Make new Generator starting at `base`
-    fn new(base: usize) -> Self {
-        IdGen(base)
-    }
 
-    /// Get the current id
-    fn get(&self) -> usize {
-        self.0
-    }
+    fn retype(
+        &mut self,
+        lin: &mut Linearization<Building>,
+        item_id: Option<ItemId>,
+        new_type: UnifType,
+    ) {
+        let Some(item_id) = item_id else {
+            return;
+        };
 
-    /// Return the **current id** and advance the generator.
-    /// Following calls to get (and get_and_advance) will return a new id
-    fn get_and_advance(&mut self) -> usize {
-        let current_id = self.0;
-        self.0 += 1;
-        current_id
+        if let Some(item) = lin.linearization.get_mut(item_id.index) {
+            debug!("retyping item {:?} to {:?}", item_id, new_type);
+            item.ty = new_type;
+        } else {
+            debug!("retype item failed (item not found)!");
+        }
     }
 }


Test patch:
diff --git a/lsp/nls/tests/inputs/hover_field_typed_block_regression_1574.ncl b/lsp/nls/tests/inputs/hover_field_typed_block_regression_1574.ncl
new file mode 100644
index 00000000..abc86c75
--- /dev/null
+++ b/lsp/nls/tests/inputs/hover_field_typed_block_regression_1574.ncl
@@ -0,0 +1,6 @@
+### /main.ncl
+let map: Array Number = std.array.map (fun x => 1) [] in map
+### [[request]]
+### type = "Hover"
+### textDocument.uri = "file:///main.ncl"
+### position = { line = 0, character = 34 }
diff --git a/lsp/nls/tests/main.rs b/lsp/nls/tests/main.rs
index 93f8bc38..8524388d 100644
--- a/lsp/nls/tests/main.rs
+++ b/lsp/nls/tests/main.rs
@@ -1,7 +1,9 @@
 use std::collections::{hash_map::Entry, HashMap};
 
 use assert_cmd::cargo::CommandCargoExt;
-use lsp_types::request::{Completion, Formatting, GotoDefinition, Request as LspRequest};
+use lsp_types::request::{
+    Completion, Formatting, GotoDefinition, HoverRequest, Request as LspRequest,
+};
 use nickel_lang_utils::project_root::project_root;
 use test_generator::test_resources;
 
@@ -36,6 +38,7 @@ impl TestHarness {
             Request::GotoDefinition(d) => self.request::<GotoDefinition>(d),
             Request::Completion(c) => self.request::<Completion>(c),
             Request::Formatting(f) => self.request::<Formatting>(f),
+            Request::Hover(h) => self.request::<HoverRequest>(h),
         }
     }
 
diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__hover_field_typed_block_regression_1574.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__hover_field_typed_block_regression_1574.ncl.snap
new file mode 100644
index 00000000..96dd35e8
--- /dev/null
+++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__hover_field_typed_block_regression_1574.ncl.snap
@@ -0,0 +1,16 @@
+---
+source: lsp/nls/tests/main.rs
+expression: output
+---
+<0:34-0:37>[Applies a function to every element in the given array. That is,
+`map f [ x1, x2, ..., xn ]` is `[ f x1, f x2, ..., f xn ]`.
+
+# Examples
+
+```nickel
+std.array.map (fun x => x + 1) [ 1, 2, 3 ] =>
+  [ 2, 3, 4 ]
+```, ```nickel
+forall a b. (a -> b) -> Array a -> Array b
+```]
+