From 649b428f190d844f8df70c524063fa35d6922017 Mon Sep 17 00:00:00 2001
From: 132ikl <132@ikl.sh>
Date: Wed, 26 Feb 2025 22:01:54 -0500
Subject: [PATCH] Add warning when using $in at the beginning of a block

---
 crates/nu-parser/src/parser.rs                | 39 ++++++++++++++++++-
 .../src/engine/state_working_set.rs           | 15 +++++++
 .../nu-protocol/src/errors/parse_warning.rs   |  8 ++++
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index 7c2b162c37..8367e4c9a4 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},
@@ -6330,6 +6330,8 @@ pub fn parse_block(
             .flat_map(|pipeline| pipeline.elements.first())
             .any(|element| element.has_in_variable(working_set))
     {
+        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;
@@ -6839,6 +6841,39 @@ 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
+        if elements.len() < 2 {
+            return;
+        }
+        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"
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,
         }
     }
 }