Fallback to file completer in custom/external completer (#14781)

# Description

Closes #14595. This modifies the behavior of both custom and external
completers so that if the custom/external completer returns an invalid
value, completions are suppressed and an error is logged. However, if
the completer returns `null` (which this PR treats as a special value),
we fall back to file completions.

Previously, custom completers and external completers had different
behavior. Any time an external completer returned an invalid value
(including `null`), we would fall back to file completions. Any time a
custom completer returned an invalid value (including `null`), we would
suppress completions.

I'm not too happy about the implementation, but it's the least intrusive
way I could think of to do it. I added a `fallback` field to
`CustomCompletions` that's checked after calling its `fetch()` method.
If `fallback` is true, then we use file completions afterwards.

An alternative would be to make `CustomCompletions` no longer implement
the `Completer` trait, and instead have its `fetch()` method return an
`Option<Vec<Suggestion>>`. But that resulted in a teeny bit of code
duplication.

# User-Facing Changes

For those using an external completer, if they want to fall back to file
completions on invalid values, their completer will have to explicitly
return `null`. Returning `"foo"` or something will no longer make
Nushell use file completions instead.

For those making custom completers, they now have the option to fall
back to file completions.

# Tests + Formatting

Added some tests and manually tested that if the completer returns an
invalid value or the completer throws an error, that gets logged and
completions are suppressed.

# After Submitting

The documentation for custom completions and external completers will
have to be updated after this.

crates/nu-cli/src/completions/completer.rs

@@ -99,18 +99,24 @@ impl NuCompleter {
         );
 
         match result.and_then(|data| data.into_value(span)) {
-            Ok(value) => {
+            Ok(Value::List { vals, .. }) => {
-                if let Value::List { vals, .. } = value {
                 let result =
                     map_value_completions(vals.iter(), Span::new(span.start, span.end), offset);
-
+                Some(result)
-                    return Some(result);
+            }
+            Ok(Value::Nothing { .. }) => None,
+            Ok(value) => {
+                log::error!(
+                    "External completer returned invalid value of type {}",
+                    value.get_type().to_string()
+                );
+                Some(vec![])
+            }
+            Err(err) => {
+                log::error!("failed to eval completer block: {err}");
+                Some(vec![])
             }
         }
-            Err(err) => println!("failed to eval completer block: {err}"),
-        }
-
-        None
     }
 
     fn completion_helper(&mut self, line: &str, pos: usize) -> Vec<SemanticSuggestion> {
@@ -319,6 +325,7 @@ impl NuCompleter {
                                     self.stack.clone(),
                                     *decl_id,
                                     initial_line,
+                                    FileCompletion::new(),
                                 );
 
                                 return self.process_completion(

crates/nu-cli/src/completions/custom_completions.rs

@@ -12,32 +12,34 @@ use std::collections::HashMap;
 
 use super::completion_options::NuMatcher;
 
-pub struct CustomCompletion {
+pub struct CustomCompletion<T: Completer> {
     stack: Stack,
     decl_id: DeclId,
     line: String,
+    fallback: T,
 }
 
-impl CustomCompletion {
+impl<T: Completer> CustomCompletion<T> {
-    pub fn new(stack: Stack, decl_id: DeclId, line: String) -> Self {
+    pub fn new(stack: Stack, decl_id: DeclId, line: String, fallback: T) -> Self {
         Self {
             stack,
             decl_id,
             line,
+            fallback,
         }
     }
 }
 
-impl Completer for CustomCompletion {
+impl<T: Completer> Completer for CustomCompletion<T> {
     fn fetch(
         &mut self,
         working_set: &StateWorkingSet,
-        _stack: &Stack,
+        stack: &Stack,
         prefix: &[u8],
         span: Span,
         offset: usize,
         pos: usize,
-        completion_options: &CompletionOptions,
+        orig_options: &CompletionOptions,
     ) -> Vec<SemanticSuggestion> {
         // Line position
         let line_pos = pos - offset;
@@ -66,13 +68,12 @@ impl Completer for CustomCompletion {
             PipelineData::empty(),
         );
 
-        let mut completion_options = completion_options.clone();
+        let mut completion_options = orig_options.clone();
         let mut should_sort = true;
 
         // Parse result
-        let suggestions = result
+        let suggestions = match result.and_then(|data| data.into_value(span)) {
-            .and_then(|data| data.into_value(span))
+            Ok(value) => match &value {
-            .map(|value| match &value {
                 Value::Record { val, .. } => {
                     let completions = val
                         .get("completions")
@@ -112,9 +113,30 @@ impl Completer for CustomCompletion {
                     completions
                 }
                 Value::List { vals, .. } => map_value_completions(vals.iter(), span, offset),
-                _ => vec![],
+                Value::Nothing { .. } => {
-            })
+                    return self.fallback.fetch(
-            .unwrap_or_default();
+                        working_set,
+                        stack,
+                        prefix,
+                        span,
+                        offset,
+                        pos,
+                        orig_options,
+                    );
+                }
+                _ => {
+                    log::error!(
+                        "Custom completer returned invalid value of type {}",
+                        value.get_type().to_string()
+                    );
+                    return vec![];
+                }
+            },
+            Err(e) => {
+                log::error!("Error getting custom completions: {e}");
+                return vec![];
+            }
+        };
 
         let mut matcher = NuMatcher::new(String::from_utf8_lossy(prefix), completion_options);
 

crates/nu-cli/tests/completions/mod.rs

@@ -234,6 +234,37 @@ fn customcompletions_no_sort() {
     match_suggestions(&expected, &suggestions);
 }
 
+/// Fallback to file completions if custom completer returns null
+#[test]
+fn customcompletions_fallback() {
+    let (_, _, mut engine, mut stack) = new_engine();
+    let command = r#"
+        def comp [] { null }
+        def my-command [arg: string@comp] {}"#;
+    assert!(support::merge_input(command.as_bytes(), &mut engine, &mut stack).is_ok());
+
+    let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
+    let completion_str = "my-command test";
+    let suggestions = completer.complete(completion_str, completion_str.len());
+    let expected: Vec<String> = vec![folder("test_a"), file("test_a_symlink"), folder("test_b")];
+    match_suggestions(&expected, &suggestions);
+}
+
+/// Suppress completions for invalid values
+#[test]
+fn customcompletions_invalid() {
+    let (_, _, mut engine, mut stack) = new_engine();
+    let command = r#"
+        def comp [] { 123 }
+        def my-command [arg: string@comp] {}"#;
+    assert!(support::merge_input(command.as_bytes(), &mut engine, &mut stack).is_ok());
+    let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
+
+    let completion_str = "my-command foo";
+    let suggestions = completer.complete(completion_str, completion_str.len());
+    assert!(suggestions.is_empty());
+}
+
 #[test]
 fn dotnu_completions() {
     // Create a new engine
@@ -312,6 +343,27 @@ fn external_completer_pass_flags() {
     assert_eq!("--", suggestions.get(2).unwrap().value);
 }
 
+/// Fallback to file completions when external completer returns null
+#[test]
+fn external_completer_fallback() {
+    let block = "{|spans| null}";
+    let input = "foo test".to_string();
+
+    let expected = vec![folder("test_a"), file("test_a_symlink"), folder("test_b")];
+    let suggestions = run_external_completion(block, &input);
+    match_suggestions(&expected, &suggestions);
+}
+
+/// Suppress completions when external completer returns invalid value
+#[test]
+fn external_completer_invalid() {
+    let block = "{|spans| 123}";
+    let input = "foo ".to_string();
+
+    let suggestions = run_external_completion(block, &input);
+    assert!(suggestions.is_empty());
+}
+
 #[test]
 fn file_completions() {
     // Create a new engine