Merge 0736ea995a into 95dcb2fd6c

crates/nu-cmd-lang/src/parse_const_test.rs

@@ -11,8 +11,14 @@ fn quickcheck_parse(data: String) -> bool {
             let mut working_set = StateWorkingSet::new(&context);
             let _ = working_set.add_file("quickcheck".into(), data.as_bytes());
 
-            let _ =
+            let _ = nu_parser::parse_block(
-                nu_parser::parse_block(&mut working_set, &tokens, Span::new(0, 0), false, false);
+                &mut working_set,
+                &tokens,
+                Span::new(0, 0),
+                false,
+                false,
+                false,
+            );
         }
     }
     true

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

@@ -23,7 +23,7 @@ fn catch_can_access_error() {
 
 #[test]
 fn catch_can_access_error_as_dollar_in() {
-    let output = nu!("try { foobarbaz } catch { $in | get raw }");
+    let output = nu!("try { foobarbaz } catch { get raw }");
 
     assert!(output.err.contains("External command failed"));
 }

crates/nu-parser/src/parse_keywords.rs

@@ -3339,7 +3339,8 @@ pub fn parse_let(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline
                     }
 
                     let rvalue_span = Span::concat(&spans[(span.0 + 1)..]);
-                    let rvalue_block = parse_block(working_set, &tokens, rvalue_span, false, true);
+                    let rvalue_block =
+                        parse_block(working_set, &tokens, rvalue_span, false, true, false);
 
                     let output_type = rvalue_block.output_type();
 
@@ -3459,7 +3460,7 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeli
 
                     trace!("parsing: const right-hand side subexpression");
                     let rvalue_block =
-                        parse_block(working_set, &rvalue_tokens, rvalue_span, false, true);
+                        parse_block(working_set, &rvalue_tokens, rvalue_span, false, true, false);
                     let rvalue_ty = rvalue_block.output_type();
                     let rvalue_block_id = working_set.add_block(Arc::new(rvalue_block));
                     let rvalue = Expression::new(
@@ -3621,7 +3622,8 @@ pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline
                     }
 
                     let rvalue_span = Span::concat(&spans[(span.0 + 1)..]);
-                    let rvalue_block = parse_block(working_set, &tokens, rvalue_span, false, true);
+                    let rvalue_block =
+                        parse_block(working_set, &tokens, rvalue_span, false, true, false);
 
                     let output_type = rvalue_block.output_type();
 

crates/nu-parser/src/parser.rs

@@ -14,8 +14,8 @@ use log::trace;
 use nu_engine::DIR_VAR_PARSER_INFO;
 use nu_protocol::{
     ast::*, engine::StateWorkingSet, eval_const::eval_constant, BlockId, DeclId, DidYouMean,
-    FilesizeUnit, Flag, ParseError, PositionalArg, ShellError, Signature, Span, Spanned,
+    FilesizeUnit, Flag, ParseError, ParseWarning, PositionalArg, ShellError, Signature, Span,
-    SyntaxShape, Type, Value, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID,
+    Spanned, SyntaxShape, Type, Value, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID,
 };
 use std::{
     collections::{HashMap, HashSet},
@@ -2452,7 +2452,7 @@ pub fn parse_full_cell_path(
 
             // Creating a Type scope to parse the new block. This will keep track of
             // the previous input type found in that block
-            let output = parse_block(working_set, &output, span, true, true);
+            let output = parse_block(working_set, &output, span, true, true, false);
 
             let ty = output.output_type();
 
@@ -4596,7 +4596,14 @@ pub fn parse_block_expression(working_set: &mut StateWorkingSet, span: Span) ->
         _ => (None, 0),
     };
 
-    let mut output = parse_block(working_set, &output[amt_to_skip..], span, false, false);
+    let mut output = parse_block(
+        working_set,
+        &output[amt_to_skip..],
+        span,
+        false,
+        false,
+        false,
+    );
 
     if let Some(signature) = signature {
         output.signature = signature.0;
@@ -4915,7 +4922,14 @@ pub fn parse_closure_expression(
         }
     }
 
-    let mut output = parse_block(working_set, &output[amt_to_skip..], span, false, false);
+    let mut output = parse_block(
+        working_set,
+        &output[amt_to_skip..],
+        span,
+        false,
+        false,
+        true,
+    );
 
     if let Some(signature) = signature {
         output.signature = signature.0;
@@ -5213,7 +5227,7 @@ pub fn parse_assignment_expression(
     working_set.parse_errors.extend(rhs_error);
 
     trace!("parsing: assignment right-hand side subexpression");
-    let rhs_block = parse_block(working_set, &rhs_tokens, rhs_span, false, true);
+    let rhs_block = parse_block(working_set, &rhs_tokens, rhs_span, false, true, false);
     let rhs_ty = rhs_block.output_type();
 
     // TEMP: double-check that if the RHS block starts with an external call, it must start with a
@@ -6305,6 +6319,7 @@ pub fn parse_block(
     span: Span,
     scoped: bool,
     is_subexpression: bool,
+    is_closure: bool,
 ) -> Block {
     let (lite_block, err) = lite_parse(tokens, working_set);
     if let Some(err) = err {
@@ -6342,6 +6357,10 @@ pub fn parse_block(
             .flat_map(|pipeline| pipeline.elements.first())
             .any(|element| element.has_in_variable(working_set))
     {
+        if is_closure {
+            check_block_pipes_in(working_set, &block);
+        }
+
         // Move the block out to prepare it to become a subexpression
         let inner_block = std::mem::take(&mut block);
         block.span = inner_block.span;
@@ -6851,6 +6870,36 @@ fn wrap_expr_with_collect(working_set: &mut StateWorkingSet, expr: Expression) -
     )
 }
 
+// Report a warning if the first pipeline element of a block is `$in` followed by a pipe
+fn check_block_pipes_in(working_set: &mut StateWorkingSet, block: &Block) {
+    if let Some(elements) = block.pipelines.first().map(|pipeline| &pipeline.elements) {
+        // only warn if we're also piping into something
+        let element = match elements.len() {
+            ..2 => return,
+            2.. => elements.first().expect("at least two elements"),
+        };
+
+        if let Expr::FullCellPath(cell_path) = &element.expr.expr {
+            if matches!(
+                **cell_path,
+                FullCellPath {
+                    head: Expression {
+                        expr: Expr::Var(id),
+                        ..
+                    },
+                    ..
+                } if id == IN_VARIABLE_ID
+            ) {
+                working_set
+                    .parse_warnings
+                    .push(ParseWarning::UnnecessaryInVariable {
+                        span: element.expr.span(working_set),
+                    })
+            }
+        }
+    }
+}
+
 // Parses a vector of u8 to create an AST Block. If a file name is given, then
 // the name is stored in the working set. When parsing a source without a file
 // name, the source of bytes is stored as "source"
@@ -6889,7 +6938,14 @@ pub fn parse(
                 working_set.error(err)
             }
 
-            Arc::new(parse_block(working_set, &output, new_span, scoped, false))
+            Arc::new(parse_block(
+                working_set,
+                &output,
+                new_span,
+                scoped,
+                false,
+                false,
+            ))
         }
     };
 

crates/nu-protocol/src/engine/state_working_set.rs

@@ -1132,6 +1132,21 @@ impl<'a> GetSpan for &'a StateWorkingSet<'a> {
     }
 }
 
+impl GetSpan for StateWorkingSet<'_> {
+    fn get_span(&self, span_id: SpanId) -> Span {
+        let num_permanent_spans = self.permanent_state.num_spans();
+        if span_id.get() < num_permanent_spans {
+            self.permanent_state.get_span(span_id)
+        } else {
+            *self
+                .delta
+                .spans
+                .get(span_id.get() - num_permanent_spans)
+                .expect("internal error: missing span")
+        }
+    }
+}
+
 impl miette::SourceCode for &StateWorkingSet<'_> {
     fn read_span<'b>(
         &'b self,

crates/nu-protocol/src/errors/parse_warning.rs

@@ -14,12 +14,20 @@ pub enum ParseWarning {
         span: Span,
         url: String,
     },
+
+    #[error("Found $in at the start of a command.")]
+    #[diagnostic(help("Using $in at the start of a command collects the pipeline input.\nIf you did mean to collect the pipeline input, replace this with the `collect` command."))]
+    UnnecessaryInVariable {
+        #[label("try removing this")]
+        span: Span,
+    },
 }
 
 impl ParseWarning {
     pub fn span(&self) -> Span {
         match self {
             ParseWarning::DeprecatedWarning { span, .. } => *span,
+            ParseWarning::UnnecessaryInVariable { span, .. } => *span,
         }
     }
 }

crates/nu-std/std/input/mod.nu

@@ -4,7 +4,7 @@ def format-event [ ] {
   # Replace numeric value of raw_code with hex string
   let record = match $record {
     {raw_code: $code} => {
-      $record | update raw_code {|| $in | format number | get upperhex}
+      $record | update raw_code {|| format number | get upperhex}
     }
     _ => $record
   }
@@ -12,7 +12,7 @@ def format-event [ ] {
   # Replace numeric value of raw_modifiers with binary string
   let record = match $record {
     {raw_modifiers: $flags} => {
-      $record | update raw_modifiers {|| $in | format number | get binary}
+      $record | update raw_modifiers {|| format number | get binary}
     }
     _ => $record
   }

crates/nu-std/std/xml/mod.nu

@@ -95,7 +95,7 @@ def xupdate-string-step [ step: string rest: list updater: closure ] {
 }
 
 def xupdate-int-step [ step: int rest: list updater: closure ] {
-    $in | enumerate | each {|it|
+    enumerate | each {|it|
         let item = $it.item
         let idx = $it.index
 
@@ -108,7 +108,7 @@ def xupdate-int-step [ step: int rest: list updater: closure ] {
 }
 
 def xupdate-closure-step [ step: closure rest: list updater: closure ] {
-    $in | each {|it|
+    each {|it|
         if (do $step $it) {
             [ $it ] | xupdate-internal $rest $updater | get 0
         } else {
@@ -194,7 +194,7 @@ export def xinsert [
                     # position is greater than number of elements) in content of all entries of input matched by
                     # path. If not specified inserts at the end.
 ] {
-    $in | xupdate $path {|entry|
+    xupdate $path {|entry|
         match ($entry | xtype) {
             'tag' => {
                 let new_content = if $position == null {