diff --git a/crates/nu-cmd-lang/src/parse_const_test.rs b/crates/nu-cmd-lang/src/parse_const_test.rs index 7870b37648..92b2198b24 100644 --- a/crates/nu-cmd-lang/src/parse_const_test.rs +++ b/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 _ = - nu_parser::parse_block(&mut working_set, &tokens, Span::new(0, 0), false, false); + let _ = nu_parser::parse_block( + &mut working_set, + &tokens, + Span::new(0, 0), + false, + false, + false, + ); } } true diff --git a/crates/nu-command/tests/commands/try_.rs b/crates/nu-command/tests/commands/try_.rs index 7e6952d2b9..8e7842a1ab 100644 --- a/crates/nu-command/tests/commands/try_.rs +++ b/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")); } diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 31dfbb6ea3..9fe06aaa54 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/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(); diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 715afb5bad..9b7b38361e 100644 --- a/crates/nu-parser/src/parser.rs +++ b/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, - SyntaxShape, Type, Value, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID, + FilesizeUnit, Flag, ParseError, ParseWarning, PositionalArg, ShellError, Signature, Span, + 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, + )) } }; diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs index 11be80efb9..61229fdc6d 100644 --- a/crates/nu-protocol/src/engine/state_working_set.rs +++ b/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, diff --git a/crates/nu-protocol/src/errors/parse_warning.rs b/crates/nu-protocol/src/errors/parse_warning.rs index 5f34deb20c..4c36c1f380 100644 --- a/crates/nu-protocol/src/errors/parse_warning.rs +++ b/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, } } } diff --git a/crates/nu-std/std/input/mod.nu b/crates/nu-std/std/input/mod.nu index 0a79210c19..d381c865a2 100644 --- a/crates/nu-std/std/input/mod.nu +++ b/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 } diff --git a/crates/nu-std/std/xml/mod.nu b/crates/nu-std/std/xml/mod.nu index 6cf8159afd..9616236fda 100644 --- a/crates/nu-std/std/xml/mod.nu +++ b/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 {