Let metadata <expr> and stor insert/update record flags ignore pipeline input instead of erroring (#19254)

## Description

`metadata` with an argument fails when the call also has pipeline input:

```nushell
[1] | each {|x| let s = (metadata $x).span; $s }
# Error: nu::shell::incompatible_parameters
#   pipeline input was provided / but a positional metadata expression was also given
```

The user never piped anything into `metadata` here. The first statement
of a block receives the block's input, which is intended (`"a" | do {
let x = (str uppercase); $x }` returns `A`), so `metadata` gets the
closure's input along with `$x` and refuses. The same check rejects `"a"
| metadata $in`, because `$in` stores the input in the variable and
still passes it to the call.

The check came in with another PR. A command can't tell implicit block
input from an explicit pipe at run time: both arrive as ordinary
`PipelineData` in the same register.

This PR removes the check. When `metadata` gets an argument, the result
describes only that argument: a record with its `span`. The input and
its pipeline metadata are ignored. Before #16049 the argument's span was
merged with the input's pipeline metadata, which attached the input's
metadata to an unrelated value:

```nushell
open Cargo.toml | metadata 5
# {span: ..., source: ".../Cargo.toml"}    <- input's source reported for the literal 5
ls | do { let m = (metadata $env.PWD); $m }
# {span: ..., path_columns: [name], ...}   <- ls's path columns reported for $env.PWD
```

`stor insert --data-record` and `stor update --update-record` had the
same input-versus-argument check and failed the same way inside a block:

```nushell
[{a: 1}] | each {|r| stor insert -t t -d $r }
# Error: Pipeline and Flag both being used
```

## User-facing changes (Release notes)

- `metadata <expression>` no longer errors when the call also has
pipeline input, such as the input a block's first statement receives
(`[1] | each {|x| let s = (metadata $x).span; $s }`) or `"a" | metadata
$in`. With an argument, `metadata` returns that argument's span and
ignores the input; `metadata` without an argument still reports the
input's pipeline metadata.
- `stor insert --data-record` and `stor update --update-record` no
longer error with "Pipeline and Flag both being used" when the call also
has pipeline input. The flag's record takes precedence and the input is
ignored.

## Additional notes

- Since the input is ignored whenever an argument is given, an explicit
pipe into these calls is now dropped without an error too (`ls |
metadata $x`, `ls | stor insert -t t -d {a: 1}`). Erroring only on an
explicit pipe would need the compiler to mark implicit block input,
which no command can see today.
- `metadata $in` returns only the span. The input's `path_columns`,
`content_type` and custom keys are not included; use `metadata` without
an argument for those. (`source` from `open` is already removed when the
input is collected into `$in`.)

crates/nu-command/src/debug/metadata.rs

@@ -1,6 +1,5 @@
-use super::util::{build_metadata_record, extend_record_with_metadata};
+use super::util::build_metadata_record;
 use nu_engine::command_prelude::*;
-use nu_protocol::PipelineMetadata;
 
 #[derive(Clone)]
 pub struct Metadata;
@@ -14,6 +13,10 @@ impl Command for Metadata {
         "Get the metadata for items in the stream."
     }
 
+    fn extra_description(&self) -> &str {
+        "With an expression, the result describes only that expression (its span) and pipeline input is ignored. Without one, the result is the input's pipeline metadata."
+    }
+
     fn signature(&self) -> nu_protocol::Signature {
         Signature::build("metadata")
             .input_output_types(vec![(Type::Any, Type::record())])
@@ -21,7 +24,7 @@ impl Command for Metadata {
             .optional(
                 "expression",
                 SyntaxShape::Any,
-                "The expression you want metadata for.",
+                "The expression you want metadata for. When given, pipeline input is ignored.",
             )
             .category(Category::Debug)
     }
@@ -44,25 +47,14 @@ impl Command for Metadata {
             None
         };
 
-        if !matches!(input, PipelineData::Empty)
-            && let Some(ref arg_val) = arg
-        {
-            return Err(ShellError::IncompatibleParameters {
-                left_message: "pipeline input was provided".into(),
-                left_span: head,
-                right_message: "but a positional metadata expression was also given".into(),
-                right_span: arg_val.span(),
-            });
-        }
-
-        match arg {
-            Some(val) => Ok(
-                build_metadata_record_value(&val, input.metadata_ref(), head).into_pipeline_data(),
-            ),
-            None => {
-                Ok(Value::record(build_metadata_record(&input, head), head).into_pipeline_data())
-            }
-        }
+        // An expression wins over the input: the first statement of a block receives the
+        // block's input implicitly, so `each {|x| metadata $x }` gets both. The record then
+        // describes only the expression; pipeline metadata comes from `metadata` alone.
+        let record = match arg {
+            Some(val) => record! { "span" => val.span().into_value(head) },
+            None => build_metadata_record(&input, head),
+        };
+        Ok(Value::record(record, head).into_pipeline_data())
     }
 
     fn examples(&self) -> Vec<Example<'_>> {
@@ -77,20 +69,15 @@ impl Command for Metadata {
                 example: "ls | metadata",
                 result: None,
             },
+            Example {
+                description: "Get the metadata of a closure parameter. The closure's input is ignored.",
+                example: "[1] | each {|x| metadata $x } | columns",
+                result: Some(Value::test_list(vec![Value::test_string("span")])),
+            },
         ]
     }
 }
 
-fn build_metadata_record_value(
-    arg: &Value,
-    metadata: Option<&PipelineMetadata>,
-    head: Span,
-) -> Value {
-    let mut record = Record::new();
-    record.push("span", arg.span().into_value(head));
-    Value::record(extend_record_with_metadata(record, metadata, head), head)
-}
-
 #[cfg(test)]
 mod test {
     use super::*;

crates/nu-command/src/stor/insert.rs

@@ -30,7 +30,7 @@ impl Command for StorInsert {
             .named(
                 "data-record",
                 SyntaxShape::record(),
-                "A record of column names and column values to insert into the specified table.",
+                "A record of column names and column values to insert into the specified table. Takes precedence over pipeline input.",
                 Some('d'),
             )
             .allow_variants_without_examples(true)
@@ -106,15 +106,9 @@ fn handle(
     data_record: Option<Record>,
     input: PipelineData,
 ) -> Result<Vec<Record>, ShellError> {
-    // Check for conflicting use of both pipeline input and flag
+    // The flag wins over the input: the first statement of a block receives the block's
+    // input implicitly, so `each {|r| stor insert -t t -d $r }` gets both.
     if let Some(record) = data_record {
-        if !matches!(input, PipelineData::Empty) {
-            return Err(ShellError::Generic(GenericError::new(
-                "Pipeline and Flag both being used",
-                "Use either pipeline input or '--data-record' parameter",
-                span,
-            )));
-        }
         return Ok(vec![record]);
     }
 

crates/nu-command/src/stor/update.rs

@@ -30,7 +30,7 @@ impl Command for StorUpdate {
             .named(
                 "update-record",
                 SyntaxShape::record(),
-                "A record of column names and column values to update in the specified table.",
+                "A record of column names and column values to update in the specified table. Takes precedence over pipeline input.",
                 Some('u'),
             )
             .named(
@@ -111,45 +111,30 @@ fn handle(
     update_record: Option<Record>,
     input: PipelineData,
 ) -> Result<Record, ShellError> {
+    // The flag wins over the input: the first statement of a block receives the block's
+    // input implicitly, so `each {|r| stor update -t t -u $r }` gets both.
+    if let Some(record) = update_record {
+        return Ok(record);
+    }
+
     match input {
-        PipelineData::Empty => update_record.ok_or_else(|| ShellError::MissingParameter {
+        PipelineData::Empty => Err(ShellError::MissingParameter {
             param_name: "requires a record".into(),
             span,
         }),
-        PipelineData::Value(value, ..) => {
-            // Since input is being used, check if the data record parameter is used too
-            if update_record.is_some() {
-                return Err(ShellError::Generic(GenericError::new(
-                    "Pipeline and Flag both being used",
-                    "Use either pipeline input or '--update-record' parameter",
-                    span,
-                )));
-            }
-            match value {
-                Value::Record { val, .. } => Ok(val.into_owned()),
-                val => Err(ShellError::OnlySupportsThisInputType {
-                    exp_input_type: "record".into(),
-                    wrong_type: val.get_type().to_string(),
-                    dst_span: span,
-                    src_span: val.span(),
-                }),
-            }
-        }
-        _ => {
-            if update_record.is_some() {
-                return Err(ShellError::Generic(GenericError::new(
-                    "Pipeline and Flag both being used",
-                    "Use either pipeline input or '--update-record' parameter",
-                    span,
-                )));
-            }
-            Err(ShellError::OnlySupportsThisInputType {
-                exp_input_type: "record".into(),
-                wrong_type: "".into(),
-                dst_span: span,
-                src_span: span,
-            })
-        }
+        PipelineData::Value(Value::Record { val, .. }, ..) => Ok(val.into_owned()),
+        PipelineData::Value(val, ..) => Err(ShellError::OnlySupportsThisInputType {
+            exp_input_type: "record".into(),
+            wrong_type: val.get_type().to_string(),
+            dst_span: span,
+            src_span: val.span(),
+        }),
+        _ => Err(ShellError::OnlySupportsThisInputType {
+            exp_input_type: "record".into(),
+            wrong_type: "".into(),
+            dst_span: span,
+            src_span: span,
+        }),
     }
 }
 

crates/nu-command/tests/commands/debug/metadata.rs

@@ -0,0 +1,48 @@
+use nu_test_support::prelude::*;
+use rstest::rstest;
+
+// See `Metadata::run` for why the argument wins over block input.
+#[rstest]
+#[case::closure_parameter(
+    "[1] | each {|x| let s = (metadata $x).span; view span $s.start $s.end } | first",
+    "1"
+)]
+#[case::literal_in_block(
+    "'a' | do { let s = (metadata 5).span; view span $s.start $s.end }",
+    "5"
+)]
+#[case::in_in_block(
+    "'a' | do { let s = (metadata $in).span; view span $s.start $s.end }",
+    "'a'"
+)]
+#[case::in_piped("let s = ('a' | metadata $in).span; view span $s.start $s.end", "'a'")]
+fn argument_wins_over_input(#[case] code: &str, #[case] expected: &str) -> Result {
+    test().run(code).expect_value_eq(expected)
+}
+
+// `null` must still count as an argument, or the input's `content_type` would show up.
+#[rstest]
+#[case::literal("5")]
+#[case::null("null")]
+fn argument_ignores_input_metadata(#[case] arg: &str) -> Result {
+    let code = format!(
+        "
+        'x'
+        | metadata set --content-type text/plain
+        | do {{ let m = (metadata {arg}); $m | columns }}
+        "
+    );
+
+    test().run(&code).expect_value_eq(["span"])
+}
+
+#[test]
+fn no_argument_reads_block_input() -> Result {
+    let code = "
+        'x'
+        | metadata set --content-type text/plain
+        | do { metadata | get content_type }
+    ";
+
+    test().run(code).expect_value_eq("text/plain")
+}

crates/nu-command/tests/commands/debug/mod.rs

@@ -1,3 +1,4 @@
+mod metadata;
 mod metadata_access;
 mod metadata_set;
 mod timeit;

crates/nu-command/tests/commands/stor.rs

@@ -34,6 +34,31 @@ fn stor_update_with_quote() -> Result {
         .expect_value_eq("This didn't work, but should now.")
 }
 
+// The first statement of a block receives the block's input, so these calls get both that
+// input and the record flag. The flag wins.
+#[test]
+fn stor_insert_data_record_in_block() -> Result {
+    let code = "
+        stor create --table-name stor_insert_block_table --columns { a: int };
+        [{a: 1} {a: 2}] | each {|r| stor insert -t stor_insert_block_table -d $r };
+        stor open | query db 'select a from stor_insert_block_table' | get a
+    ";
+
+    test().run(code).expect_value_eq([1, 2])
+}
+
+#[test]
+fn stor_update_update_record_in_block() -> Result {
+    let code = "
+        stor create --table-name stor_update_block_table --columns { a: int };
+        stor insert -t stor_update_block_table -d {a: 1};
+        {a: 9} | do { stor update -t stor_update_block_table -u {a: 2} };
+        stor open | query db 'select a from stor_update_block_table' | get a
+    ";
+
+    test().run(code).expect_value_eq([2])
+}
+
 // `stor import` hands the path to sqlite, which resolves a relative one against the process
 // working directory. The test harness does not keep that in sync with `$env.PWD`, so these
 // tests use an absolute path inside the playground rather than setting `cwd`.