From 7c50f7c7146cc4a9a604f745c04fcccf4ef92ec2 Mon Sep 17 00:00:00 2001
From: Stefan Holderbach <sholderbach@users.noreply.github.com>
Date: Fri, 23 May 2025 01:26:34 +0200
Subject: [PATCH] Clean public API of `EngineState` and friends (#15636)

# Description
`pub` has been overused in many parts of `nu-protocol`. This exposes
implementation details in ways that things could break should we involve
the internals. Also each public member can slow down the decisions to
improve a certain implementation. Furthermore dead code can't be
detected if things are marked as `pub`. Thus we need to judiciously
remove more accidentally `pub` markings and eliminate the dead code if
we come across it.

This PR tackles `EngineState` and `StateWorkingSet` as important
components of the engine and `nu-protocol`. Prompted by a large number
of confusingly named methods surrounding overlays and scope management.

- **Hide overlay predecl logic**
- **Remove dead overlay code**
- **Remove unused helper**
- **Remove dead overlay code from `EngineState`**
- **Hide update_plugin_file impl detail**
- **Hide another overlay internal detail`**

# API User-Facing Changes
Removal of several formerly public members that potentially give
intrusive access to the engine. We will button up some of our public
API, feel free to explicitly complain so we can figure out what access
should be granted. We want to evolve to stable APIs as much as possible
which means hiding more implementation details and committing to a
select few well defined and documented interfaces
---
 crates/nu-protocol/src/engine/engine_state.rs | 14 +-----
 .../src/engine/state_working_set.rs           | 48 +------------------
 2 files changed, 3 insertions(+), 59 deletions(-)

diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs
index d33a6c2000..c2496c1d3a 100644
--- a/crates/nu-protocol/src/engine/engine_state.rs
+++ b/crates/nu-protocol/src/engine/engine_state.rs
@@ -368,13 +368,6 @@ impl EngineState {
         Ok(())
     }
 
-    pub fn has_overlay(&self, name: &[u8]) -> bool {
-        self.scope
-            .overlays
-            .iter()
-            .any(|(overlay_name, _)| name == overlay_name)
-    }
-
     pub fn active_overlay_ids<'a, 'b>(
         &'b self,
         removed_overlays: &'a [Vec<u8>],
@@ -412,7 +405,7 @@ impl EngineState {
     }
 
     /// Translate overlay IDs from other to IDs in self
-    pub fn translate_overlay_ids(&self, other: &ScopeFrame) -> Vec<OverlayId> {
+    fn translate_overlay_ids(&self, other: &ScopeFrame) -> Vec<OverlayId> {
         let other_names = other.active_overlays.iter().map(|other_id| {
             &other
                 .overlays
@@ -520,10 +513,7 @@ impl EngineState {
     }
 
     #[cfg(feature = "plugin")]
-    pub fn update_plugin_file(
-        &self,
-        updated_items: Vec<PluginRegistryItem>,
-    ) -> Result<(), ShellError> {
+    fn update_plugin_file(&self, updated_items: Vec<PluginRegistryItem>) -> Result<(), ShellError> {
         // Updating the signatures plugin file with the added signatures
         use std::fs::File;
 
diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs
index 5fb8e15bc0..ab04ad1964 100644
--- a/crates/nu-protocol/src/engine/state_working_set.rs
+++ b/crates/nu-protocol/src/engine/state_working_set.rs
@@ -207,7 +207,7 @@ impl<'a> StateWorkingSet<'a> {
         None
     }
 
-    pub fn move_predecls_to_overlay(&mut self) {
+    fn move_predecls_to_overlay(&mut self) {
         let predecls: HashMap<Vec<u8>, DeclId> =
             self.delta.last_scope_frame_mut().predecls.drain().collect();
 
@@ -313,10 +313,6 @@ impl<'a> StateWorkingSet<'a> {
         }
     }
 
-    pub fn global_span_offset(&self) -> usize {
-        self.permanent_state.next_span_start()
-    }
-
     pub fn files(&self) -> impl Iterator<Item = &CachedFile> {
         self.permanent_state.files().chain(self.delta.files.iter())
     }
@@ -554,34 +550,6 @@ impl<'a> StateWorkingSet<'a> {
         None
     }
 
-    pub fn contains_decl_partial_match(&self, name: &[u8]) -> bool {
-        let mut removed_overlays = vec![];
-
-        for scope_frame in self.delta.scope.iter().rev() {
-            for overlay_frame in scope_frame.active_overlays(&mut removed_overlays).rev() {
-                for decl in &overlay_frame.decls {
-                    if decl.0.starts_with(name) {
-                        return true;
-                    }
-                }
-            }
-        }
-
-        for overlay_frame in self
-            .permanent_state
-            .active_overlays(&removed_overlays)
-            .rev()
-        {
-            for decl in &overlay_frame.decls {
-                if decl.0.starts_with(name) {
-                    return true;
-                }
-            }
-        }
-
-        false
-    }
-
     pub fn next_var_id(&self) -> VarId {
         let num_permanent_vars = self.permanent_state.num_vars();
         VarId::new(num_permanent_vars + self.delta.vars.len())
@@ -850,20 +818,6 @@ impl<'a> StateWorkingSet<'a> {
         }
     }
 
-    pub fn has_overlay(&self, name: &[u8]) -> bool {
-        for scope_frame in self.delta.scope.iter().rev() {
-            if scope_frame
-                .overlays
-                .iter()
-                .any(|(overlay_name, _)| name == overlay_name)
-            {
-                return true;
-            }
-        }
-
-        self.permanent_state.has_overlay(name)
-    }
-
     /// Find the overlay corresponding to `name`.
     pub fn find_overlay(&self, name: &[u8]) -> Option<&OverlayFrame> {
         for scope_frame in self.delta.scope.iter().rev() {