Split the plugin crate (#12563)

# Description This breaks `nu-plugin` up into four crates: - `nu-plugin-protocol`: just the type definitions for the protocol, no I/O. If someone wanted to wire up something more bare metal, maybe for async I/O, they could use this. - `nu-plugin-core`: the shared stuff between engine/plugin. Less stable interface. - `nu-plugin-engine`: everything required for the engine to talk to plugins. Less stable interface. - `nu-plugin`: everything required for the plugin to talk to the engine, what plugin developers use. Should be the most stable interface. No changes are made to the interface exposed by `nu-plugin` - it should all still be there. Re-exports from `nu-plugin-protocol` or `nu-plugin-core` are used as required. Plugins shouldn't ever have to use those crates directly. This should be somewhat faster to compile as `nu-plugin-engine` and `nu-plugin` can compile in parallel, and the engine doesn't need `nu-plugin` and plugins don't need `nu-plugin-engine` (except for test support), so that should reduce what needs to be compiled too. The only significant change here other than splitting stuff up was to break the `source` out of `PluginCustomValue` and create a new `PluginCustomValueWithSource` type that contains that instead. One bonus of that is we get rid of the option and it's now more type-safe, but it also means that the logic for that stuff (actually running the plugin for custom value ops) can live entirely within the `nu-plugin-engine` crate. # User-Facing Changes - New crates. - Added `local-socket` feature for `nu` to try to make it possible to compile without that support if needed. # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib`
2025-08-12 21:37:52 +02:00 · 2024-04-27 10:08:12 -07:00
parent 884d5312bb
commit 0c4d5330ee
74 changed files with 3514 additions and 3110 deletions
--- a/crates/nu-plugin-engine/src/plugin_custom_value_with_source/mod.rs
+++ b/crates/nu-plugin-engine/src/plugin_custom_value_with_source/mod.rs
@ -0,0 +1,274 @@
+use std::{cmp::Ordering, sync::Arc};
+
+use nu_plugin_core::util::with_custom_values_in;
+use nu_plugin_protocol::PluginCustomValue;
+use nu_protocol::{ast::Operator, CustomValue, IntoSpanned, ShellError, Span, Spanned, Value};
+use serde::Serialize;
+
+use crate::{PluginInterface, PluginSource};
+
+#[cfg(test)]
+mod tests;
+
+/// Wraps a [`PluginCustomValue`] together with its [`PluginSource`], so that the [`CustomValue`]
+/// methods can be implemented by calling the plugin, and to ensure that any custom values sent to a
+/// plugin came from it originally.
+#[derive(Debug, Clone)]
+pub struct PluginCustomValueWithSource {
+    inner: PluginCustomValue,
+
+    /// Which plugin the custom value came from. This is not sent over the serialization boundary.
+    source: Arc<PluginSource>,
+}
+
+impl PluginCustomValueWithSource {
+    /// Wrap a [`PluginCustomValue`] together with its source.
+    pub fn new(inner: PluginCustomValue, source: Arc<PluginSource>) -> PluginCustomValueWithSource {
+        PluginCustomValueWithSource { inner, source }
+    }
+
+    /// Create a [`Value`] containing this custom value.
+    pub fn into_value(self, span: Span) -> Value {
+        Value::custom(Box::new(self), span)
+    }
+
+    /// Which plugin the custom value came from. This provides a direct reference to be able to get
+    /// a plugin interface in order to make a call, when needed.
+    pub fn source(&self) -> &Arc<PluginSource> {
+        &self.source
+    }
+
+    /// Unwrap the [`PluginCustomValueWithSource`], discarding the source.
+    pub fn without_source(self) -> PluginCustomValue {
+        // Because of the `Drop` implementation, we can't destructure this.
+        self.inner.clone()
+    }
+
+    /// Helper to get the plugin to implement an op
+    fn get_plugin(&self, span: Option<Span>, for_op: &str) -> Result<PluginInterface, ShellError> {
+        let wrap_err = |err: ShellError| ShellError::GenericError {
+            error: format!(
+                "Unable to spawn plugin `{}` to {for_op}",
+                self.source.name()
+            ),
+            msg: err.to_string(),
+            span,
+            help: None,
+            inner: vec![err],
+        };
+
+        self.source
+            .clone()
+            .persistent(span)
+            .and_then(|p| p.get_plugin(None))
+            .map_err(wrap_err)
+    }
+
+    /// Add a [`PluginSource`] to the given [`CustomValue`] if it is a [`PluginCustomValue`].
+    pub fn add_source(value: &mut Box<dyn CustomValue>, source: &Arc<PluginSource>) {
+        if let Some(custom_value) = value.as_any().downcast_ref::<PluginCustomValue>() {
+            *value = Box::new(custom_value.clone().with_source(source.clone()));
+        }
+    }
+
+    /// Add a [`PluginSource`] to all [`PluginCustomValue`]s within the value, recursively.
+    pub fn add_source_in(value: &mut Value, source: &Arc<PluginSource>) -> Result<(), ShellError> {
+        with_custom_values_in(value, |custom_value| {
+            Self::add_source(custom_value.item, source);
+            Ok::<_, ShellError>(())
+        })
+    }
+
+    /// Remove a [`PluginSource`] from the given [`CustomValue`] if it is a
+    /// [`PluginCustomValueWithSource`]. This will turn it back into a [`PluginCustomValue`].
+    pub fn remove_source(value: &mut Box<dyn CustomValue>) {
+        if let Some(custom_value) = value.as_any().downcast_ref::<PluginCustomValueWithSource>() {
+            *value = Box::new(custom_value.clone().without_source());
+        }
+    }
+
+    /// Remove the [`PluginSource`] from all [`PluginCustomValue`]s within the value, recursively.
+    pub fn remove_source_in(value: &mut Value) -> Result<(), ShellError> {
+        with_custom_values_in(value, |custom_value| {
+            Self::remove_source(custom_value.item);
+            Ok::<_, ShellError>(())
+        })
+    }
+
+    /// Check that `self` came from the given `source`, and return an `error` if not.
+    pub fn verify_source(&self, span: Span, source: &PluginSource) -> Result<(), ShellError> {
+        if self.source.is_compatible(source) {
+            Ok(())
+        } else {
+            Err(ShellError::CustomValueIncorrectForPlugin {
+                name: self.name().to_owned(),
+                span,
+                dest_plugin: source.name().to_owned(),
+                src_plugin: Some(self.source.name().to_owned()),
+            })
+        }
+    }
+
+    /// Check that a [`CustomValue`] is a [`PluginCustomValueWithSource`] that came from the given
+    /// `source`, and return an error if not.
+    pub fn verify_source_of_custom_value(
+        value: Spanned<&dyn CustomValue>,
+        source: &PluginSource,
+    ) -> Result<(), ShellError> {
+        if let Some(custom_value) = value
+            .item
+            .as_any()
+            .downcast_ref::<PluginCustomValueWithSource>()
+        {
+            custom_value.verify_source(value.span, source)
+        } else {
+            // Only PluginCustomValueWithSource can be sent
+            Err(ShellError::CustomValueIncorrectForPlugin {
+                name: value.item.type_name(),
+                span: value.span,
+                dest_plugin: source.name().to_owned(),
+                src_plugin: None,
+            })
+        }
+    }
+}
+
+impl std::ops::Deref for PluginCustomValueWithSource {
+    type Target = PluginCustomValue;
+
+    fn deref(&self) -> &PluginCustomValue {
+        &self.inner
+    }
+}
+
+/// This `Serialize` implementation always produces an error. Strip the source before sending.
+impl Serialize for PluginCustomValueWithSource {
+    fn serialize<S>(&self, _serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::Error;
+        Err(Error::custom(
+            "can't serialize PluginCustomValueWithSource, remove the source first",
+        ))
+    }
+}
+
+impl CustomValue for PluginCustomValueWithSource {
+    fn clone_value(&self, span: Span) -> Value {
+        self.clone().into_value(span)
+    }
+
+    fn type_name(&self) -> String {
+        self.name().to_owned()
+    }
+
+    fn to_base_value(&self, span: Span) -> Result<Value, ShellError> {
+        self.get_plugin(Some(span), "get base value")?
+            .custom_value_to_base_value(self.clone().into_spanned(span))
+    }
+
+    fn follow_path_int(
+        &self,
+        self_span: Span,
+        index: usize,
+        path_span: Span,
+    ) -> Result<Value, ShellError> {
+        self.get_plugin(Some(self_span), "follow cell path")?
+            .custom_value_follow_path_int(
+                self.clone().into_spanned(self_span),
+                index.into_spanned(path_span),
+            )
+    }
+
+    fn follow_path_string(
+        &self,
+        self_span: Span,
+        column_name: String,
+        path_span: Span,
+    ) -> Result<Value, ShellError> {
+        self.get_plugin(Some(self_span), "follow cell path")?
+            .custom_value_follow_path_string(
+                self.clone().into_spanned(self_span),
+                column_name.into_spanned(path_span),
+            )
+    }
+
+    fn partial_cmp(&self, other: &Value) -> Option<Ordering> {
+        self.get_plugin(Some(other.span()), "perform comparison")
+            .and_then(|plugin| {
+                // We're passing Span::unknown() here because we don't have one, and it probably
+                // shouldn't matter here and is just a consequence of the API
+                plugin.custom_value_partial_cmp(self.clone(), other.clone())
+            })
+            .unwrap_or_else(|err| {
+                // We can't do anything with the error other than log it.
+                log::warn!(
+                    "Error in partial_cmp on plugin custom value (source={source:?}): {err}",
+                    source = self.source
+                );
+                None
+            })
+            .map(|ordering| ordering.into())
+    }
+
+    fn operation(
+        &self,
+        lhs_span: Span,
+        operator: Operator,
+        op_span: Span,
+        right: &Value,
+    ) -> Result<Value, ShellError> {
+        self.get_plugin(Some(lhs_span), "invoke operator")?
+            .custom_value_operation(
+                self.clone().into_spanned(lhs_span),
+                operator.into_spanned(op_span),
+                right.clone(),
+            )
+    }
+
+    fn as_any(&self) -> &dyn std::any::Any {
+        self
+    }
+
+    fn as_mut_any(&mut self) -> &mut dyn std::any::Any {
+        self
+    }
+
+    #[doc(hidden)]
+    fn typetag_name(&self) -> &'static str {
+        "PluginCustomValueWithSource"
+    }
+
+    #[doc(hidden)]
+    fn typetag_deserialize(&self) {}
+}
+
+impl Drop for PluginCustomValueWithSource {
+    fn drop(&mut self) {
+        // If the custom value specifies notify_on_drop and this is the last copy, we need to let
+        // the plugin know about it if we can.
+        if self.notify_on_drop() && self.inner.ref_count() == 1 {
+            self.get_plugin(None, "drop")
+                // While notifying drop, we don't need a copy of the source
+                .and_then(|plugin| plugin.custom_value_dropped(self.inner.clone()))
+                .unwrap_or_else(|err| {
+                    // We shouldn't do anything with the error except log it
+                    let name = self.name();
+                    log::warn!("Failed to notify drop of custom value ({name}): {err}")
+                });
+        }
+    }
+}
+
+/// Helper trait for adding a source to a [`PluginCustomValue`]
+pub trait WithSource {
+    /// Add a source to a plugin custom value
+    fn with_source(self, source: Arc<PluginSource>) -> PluginCustomValueWithSource;
+}
+
+impl WithSource for PluginCustomValue {
+    fn with_source(self, source: Arc<PluginSource>) -> PluginCustomValueWithSource {
+        PluginCustomValueWithSource::new(self, source)
+    }
+}
--- a/crates/nu-plugin-engine/src/plugin_custom_value_with_source/tests.rs
+++ b/crates/nu-plugin-engine/src/plugin_custom_value_with_source/tests.rs
@ -0,0 +1,198 @@
+use std::sync::Arc;
+
+use nu_plugin_protocol::test_util::{test_plugin_custom_value, TestCustomValue};
+use nu_protocol::{engine::Closure, record, CustomValue, IntoSpanned, ShellError, Span, Value};
+
+use crate::{
+    test_util::test_plugin_custom_value_with_source, PluginCustomValueWithSource, PluginSource,
+};
+
+use super::WithSource;
+
+#[test]
+fn add_source_in_at_root() -> Result<(), ShellError> {
+    let mut val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
+    let source = Arc::new(PluginSource::new_fake("foo"));
+    PluginCustomValueWithSource::add_source_in(&mut val, &source)?;
+
+    let custom_value = val.as_custom_value()?;
+    let plugin_custom_value: &PluginCustomValueWithSource = custom_value
+        .as_any()
+        .downcast_ref()
+        .expect("not PluginCustomValueWithSource");
+    assert_eq!(
+        Arc::as_ptr(&source),
+        Arc::as_ptr(&plugin_custom_value.source)
+    );
+    Ok(())
+}
+
+fn check_record_custom_values(
+    val: &Value,
+    keys: &[&str],
+    mut f: impl FnMut(&str, &dyn CustomValue) -> Result<(), ShellError>,
+) -> Result<(), ShellError> {
+    let record = val.as_record()?;
+    for key in keys {
+        let val = record
+            .get(key)
+            .unwrap_or_else(|| panic!("record does not contain '{key}'"));
+        let custom_value = val
+            .as_custom_value()
+            .unwrap_or_else(|_| panic!("'{key}' not custom value"));
+        f(key, custom_value)?;
+    }
+    Ok(())
+}
+
+#[test]
+fn add_source_in_nested_record() -> Result<(), ShellError> {
+    let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
+    let mut val = Value::test_record(record! {
+        "foo" => orig_custom_val.clone(),
+        "bar" => orig_custom_val.clone(),
+    });
+    let source = Arc::new(PluginSource::new_fake("foo"));
+    PluginCustomValueWithSource::add_source_in(&mut val, &source)?;
+
+    check_record_custom_values(&val, &["foo", "bar"], |key, custom_value| {
+        let plugin_custom_value: &PluginCustomValueWithSource = custom_value
+            .as_any()
+            .downcast_ref()
+            .unwrap_or_else(|| panic!("'{key}' not PluginCustomValueWithSource"));
+        assert_eq!(
+            Arc::as_ptr(&source),
+            Arc::as_ptr(&plugin_custom_value.source),
+            "'{key}' source not set correctly"
+        );
+        Ok(())
+    })
+}
+
+fn check_list_custom_values(
+    val: &Value,
+    indices: impl IntoIterator<Item = usize>,
+    mut f: impl FnMut(usize, &dyn CustomValue) -> Result<(), ShellError>,
+) -> Result<(), ShellError> {
+    let list = val.as_list()?;
+    for index in indices {
+        let val = list
+            .get(index)
+            .unwrap_or_else(|| panic!("[{index}] not present in list"));
+        let custom_value = val
+            .as_custom_value()
+            .unwrap_or_else(|_| panic!("[{index}] not custom value"));
+        f(index, custom_value)?;
+    }
+    Ok(())
+}
+
+#[test]
+fn add_source_in_nested_list() -> Result<(), ShellError> {
+    let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
+    let mut val = Value::test_list(vec![orig_custom_val.clone(), orig_custom_val.clone()]);
+    let source = Arc::new(PluginSource::new_fake("foo"));
+    PluginCustomValueWithSource::add_source_in(&mut val, &source)?;
+
+    check_list_custom_values(&val, 0..=1, |index, custom_value| {
+        let plugin_custom_value: &PluginCustomValueWithSource = custom_value
+            .as_any()
+            .downcast_ref()
+            .unwrap_or_else(|| panic!("[{index}] not PluginCustomValueWithSource"));
+        assert_eq!(
+            Arc::as_ptr(&source),
+            Arc::as_ptr(&plugin_custom_value.source),
+            "[{index}] source not set correctly"
+        );
+        Ok(())
+    })
+}
+
+fn check_closure_custom_values(
+    val: &Value,
+    indices: impl IntoIterator<Item = usize>,
+    mut f: impl FnMut(usize, &dyn CustomValue) -> Result<(), ShellError>,
+) -> Result<(), ShellError> {
+    let closure = val.as_closure()?;
+    for index in indices {
+        let val = closure
+            .captures
+            .get(index)
+            .unwrap_or_else(|| panic!("[{index}] not present in closure"));
+        let custom_value = val
+            .1
+            .as_custom_value()
+            .unwrap_or_else(|_| panic!("[{index}] not custom value"));
+        f(index, custom_value)?;
+    }
+    Ok(())
+}
+
+#[test]
+fn add_source_in_nested_closure() -> Result<(), ShellError> {
+    let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
+    let mut val = Value::test_closure(Closure {
+        block_id: 0,
+        captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())],
+    });
+    let source = Arc::new(PluginSource::new_fake("foo"));
+    PluginCustomValueWithSource::add_source_in(&mut val, &source)?;
+
+    check_closure_custom_values(&val, 0..=1, |index, custom_value| {
+        let plugin_custom_value: &PluginCustomValueWithSource = custom_value
+            .as_any()
+            .downcast_ref()
+            .unwrap_or_else(|| panic!("[{index}] not PluginCustomValueWithSource"));
+        assert_eq!(
+            Arc::as_ptr(&source),
+            Arc::as_ptr(&plugin_custom_value.source),
+            "[{index}] source not set correctly"
+        );
+        Ok(())
+    })
+}
+
+#[test]
+fn verify_source_error_message() -> Result<(), ShellError> {
+    let span = Span::new(5, 7);
+    let ok_val = test_plugin_custom_value_with_source();
+    let native_val = TestCustomValue(32);
+    let foreign_val =
+        test_plugin_custom_value().with_source(Arc::new(PluginSource::new_fake("other")));
+    let source = PluginSource::new_fake("test");
+
+    PluginCustomValueWithSource::verify_source_of_custom_value(
+        (&ok_val as &dyn CustomValue).into_spanned(span),
+        &source,
+    )
+    .expect("ok_val should be verified ok");
+
+    for (val, src_plugin) in [
+        (&native_val as &dyn CustomValue, None),
+        (&foreign_val as &dyn CustomValue, Some("other")),
+    ] {
+        let error = PluginCustomValueWithSource::verify_source_of_custom_value(
+            val.into_spanned(span),
+            &source,
+        )
+        .expect_err(&format!(
+            "a custom value from {src_plugin:?} should result in an error"
+        ));
+        if let ShellError::CustomValueIncorrectForPlugin {
+            name,
+            span: err_span,
+            dest_plugin,
+            src_plugin: err_src_plugin,
+        } = error
+        {
+            assert_eq!("TestCustomValue", name, "error.name from {src_plugin:?}");
+            assert_eq!(span, err_span, "error.span from {src_plugin:?}");
+            assert_eq!("test", dest_plugin, "error.dest_plugin from {src_plugin:?}");
+            assert_eq!(src_plugin, err_src_plugin.as_deref(), "error.src_plugin");
+        } else {
+            panic!("the error returned should be CustomValueIncorrectForPlugin");
+        }
+    }
+
+    Ok(())
+}