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
This commit is contained in:
Stefan Holderbach 2025-05-23 01:26:34 +02:00 committed by GitHub
parent bc043dcaeb
commit 7c50f7c714
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 3 additions and 59 deletions

View File

@ -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;

View File

@ -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() {