improve error message for variable not in scope when there's discarded variables

compiler-core/src/analyse.rs

···
             has_javascript_external: external_javascript.is_some(),
         };
 
-        let typed_args = arguments
-            .into_iter()
-            .zip(&prereg_args_types)
-            .map(|(a, t)| a.set_type(t.clone()))
-            .collect_vec();
+        let mut typed_args = Vec::with_capacity(arguments.len());
+        for (argument, type_) in arguments.into_iter().zip(&prereg_args_types) {
+            let argument = argument.set_type(type_.clone());
+            match &argument.names {
+                ast::ArgNames::Named { .. } | ast::ArgNames::NamedLabelled { .. } => (),
+                ast::ArgNames::Discard { name, location }
+                | ast::ArgNames::LabelledDiscard {
+                    name,
+                    name_location: location,
+                    ..
+                } => {
+                    let _ = environment.ignored_names.insert(name.clone(), *location);
+                }
+            }
+
+            typed_args.push(argument);
+        }
 
         // We have already registered the function in the `register_value_from_function`
         // method, but here we must set this as the current function again, so that anything

compiler-core/src/error.rs

···
 use crate::type_::{FieldAccessUsage, error::PatternMatchKind};
 use crate::{ast::BinOp, parse::error::ParseErrorType, type_::Type};
 use crate::{bit_array, diagnostic::Level, javascript, type_::UnifyErrorSituation};
-use ecow::EcoString;
+use ecow::{EcoString, eco_format};
 use itertools::Itertools;
 use pubgrub::Package;
 use pubgrub::{DerivationTree, VersionSet};
···
                 TypeError::UnknownVariable {
                     location,
                     variables,
+                    ignored_variables,
                     name,
                     type_with_name_in_scope,
                 } => {
···
                             wrap_format!("The name `{name}` is not in scope here.")
                         }
                     };
-                    Diagnostic {
-                        title: "Unknown variable".into(),
-                        text,
-                        hint: None,
-                        level: Level::Error,
-                        location: Some(Location {
+
+                    let ignored_variable = ignored_variables.get(&eco_format!("_{name}"));
+                    let location = if let Some(ignored_location) = ignored_variable {
+                        Location {
+                            label: Label {
+                                text: None,
+                                span: *location,
+                            },
+                            path: path.clone(),
+                            src: src.clone(),
+                            extra_labels: vec![
+                                ExtraLabel {
+                                    src_info: None,
+                                    label: Label {
+                                        text: Some("Did you mean to use this ignored variable?".into()),
+                                        span: *ignored_location,
+                                    }
+                                }
+                            ],
+                        }
+                    } else {
+                        Location {
                             label: Label {
                                 text: did_you_mean(name, variables),
                                 span: *location,
···
                             path: path.clone(),
                             src: src.clone(),
                             extra_labels: vec![],
-                        }),
+                        }
+                    };
+
+                    Diagnostic {
+                        title: "Unknown variable".into(),
+                        text,
+                        hint: None,
+                        level: Level::Error,
+                        location: Some(location),
                     }
                 }
 

compiler-core/src/type_/environment.rs

···
     /// Values defined in the current function (or the prelude)
     pub scope: im::HashMap<EcoString, ValueConstructor>,
 
+    // The names of all the ignored variables and arguments in scope:
+    // `let _var = 10` `pub fn main(_var) { todo }`.
+    pub ignored_names: HashMap<EcoString, SrcSpan>,
+
     /// Types defined in the current module (or the prelude)
     pub module_types: HashMap<EcoString, TypeConstructor>,
 
···
             unqualified_imported_types: HashMap::new(),
             accessors: prelude.accessors.clone(),
             scope: prelude.values.clone().into(),
+            ignored_names: HashMap::new(),
             importable_modules,
             current_module,
             local_variable_usages: vec![HashMap::new()],
···
 #[derive(Debug)]
 pub struct ScopeResetData {
     local_values: im::HashMap<EcoString, ValueConstructor>,
+    ignored_names: HashMap<EcoString, SrcSpan>,
 }
 
 impl Environment<'_> {
···
 
     pub fn open_new_scope(&mut self) -> ScopeResetData {
         let local_values = self.scope.clone();
+        let ignored_names = self.ignored_names.clone();
         self.local_variable_usages.push(HashMap::new());
-        ScopeResetData { local_values }
+        ScopeResetData {
+            local_values,
+            ignored_names,
+        }
     }
 
     pub fn close_scope(
···
         was_successful: bool,
         problems: &mut Problems,
     ) {
+        let ScopeResetData {
+            local_values,
+            ignored_names,
+        } = data;
+
         let unused = self
             .local_variable_usages
             .pop()
···
         if was_successful {
             self.handle_unused_variables(unused, problems);
         }
-        self.scope = data.local_values;
+        self.scope = local_values;
+        self.ignored_names = ignored_names;
     }
 
     pub fn next_uid(&mut self) -> u64 {

compiler-core/src/type_/error.rs

···
 use num_bigint::BigInt;
 #[cfg(test)]
 use pretty_assertions::assert_eq;
-use std::sync::Arc;
+use std::{collections::HashMap, sync::Arc};
 
 /// Errors and warnings discovered when compiling a module.
 ///
···
         location: SrcSpan,
         name: EcoString,
         variables: Vec<EcoString>,
+        ignored_variables: HashMap<EcoString, SrcSpan>,
         type_with_name_in_scope: bool,
     },
 
···
             location,
             name,
             variables,
+            ignored_variables: HashMap::new(),
             type_with_name_in_scope,
         },
 

compiler-core/src/type_/expression.rs

···
             .map(|type_| self.type_from_ast(&type_))
             .unwrap_or_else(|| Ok(self.new_unbound_var()))?;
 
+        match &names {
+            ArgNames::Named { .. } | ArgNames::NamedLabelled { .. } => (),
+            ArgNames::Discard { name, .. } | ArgNames::LabelledDiscard { name, .. } => {
+                let _ = self
+                    .environment
+                    .ignored_names
+                    .insert(name.clone(), location);
+            }
+        }
+
         // If we know the expected type of the argument from its contextual
         // usage then unify the newly constructed type with the expected type.
         // We do this here because then there is more type information for the
···
                 location: *location,
                 name: name.clone(),
                 variables: self.environment.local_value_names(),
+                ignored_variables: self.environment.ignored_names.clone(),
                 type_with_name_in_scope: self
                     .environment
                     .module_types

compiler-core/src/type_/pattern.rs

···
         match pattern {
             Pattern::Discard { name, location, .. } => {
                 self.check_name_case(location, &name, Named::Discard);
+                let _ = self
+                    .environment
+                    .ignored_names
+                    .insert(name.clone(), location);
                 Pattern::Discard {
                     type_,
                     name,
···
 
             Pattern::VarUsage { name, location, .. } => {
                 let constructor = match self.variables.get_mut(&name) {
-                    // If we've bound a variable in the current bit array pattern, we
-                    // want to use that.
+                    // If we've bound a variable in the current bit array pattern,
+                    // we want to use that.
                     Some(variable) if variable.in_scope() => {
                         variable.usage = Usage::UsedInPattern;
                         ValueConstructor::local_variable(
···
                                 location,
                                 name: name.clone(),
                                 variables: self.environment.local_value_names(),
+                                ignored_variables: self.environment.ignored_names.clone(),
                                 type_with_name_in_scope: self
                                     .environment
                                     .module_types
···
                             },
                         );
                     }
-                    AssignName::Discard(_) => {
-                        if let AssignName::Discard(right) = &right_side_assignment {
-                            self.check_name_case(right_location, right, Named::Discard);
-                        }
+                    AssignName::Discard(right) => {
+                        let _ = self
+                            .environment
+                            .ignored_names
+                            .insert(right.clone(), right_location);
+                        self.check_name_case(right_location, right, Named::Discard);
                     }
                 };
 

compiler-core/src/type_/tests/errors.rs

···
 fn non_utf8_string_assignment() {
     assert_error!(r#"let assert <<"Hello" as message:utf16>> = <<>>"#);
 }
+
+#[test]
+fn shadowed_function_argument() {
+    assert_module_error!(
+        "
+pub fn go(_x) {
+  x + 1
+}
+"
+    );
+}
+
+#[test]
+fn shadowed_fn_argument() {
+    assert_module_error!(
+        "
+pub fn go(x) {
+  fn(_y) {
+    y + x
+  }
+}
+"
+    );
+}
+
+#[test]
+fn shadowed_let_variable() {
+    assert_module_error!(
+        "
+pub fn go() {
+  let _x = 1
+  x + 1
+}
+"
+    );
+}
+
+#[test]
+fn shadowed_pattern_variable() {
+    assert_module_error!(
+        "
+pub type Wibble {
+  Wibble(Int)
+}
+
+pub fn go(x) {
+  case x {
+    Wibble(_y) -> y + 1
+  }
+}
+"
+    );
+}
+
+#[test]
+fn do_not_suggest_ignored_variable_outside_of_current_scope() {
+    assert_module_error!(
+        "
+pub fn go() {
+  let _ = {
+    let _y = 1 // <- this shouldn't be highlighted!
+  }
+  y
+}
+"
+    );
+}

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__do_not_suggest_ignored_variable_outside_of_current_scope.snap

···
+---
+source: compiler-core/src/type_/tests/errors.rs
+expression: "\npub fn go() {\n  let _ = {\n    let _y = 1 // <- this shouldn't be highlighted!\n  }\n  y\n}\n"
+---
+----- SOURCE CODE
+
+pub fn go() {
+  let _ = {
+    let _y = 1 // <- this shouldn't be highlighted!
+  }
+  y
+}
+
+
+----- ERROR
+error: Unknown variable
+  ┌─ /src/one/two.gleam:6:3
+  │
+6 │   y
+  │   ^
+
+The name `y` is not in scope here.

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__shadowed_fn_argument.snap

···
+---
+source: compiler-core/src/type_/tests/errors.rs
+expression: "\npub fn go(x) {\n  fn(_y) {\n    y + x\n  }\n}\n"
+---
+----- SOURCE CODE
+
+pub fn go(x) {
+  fn(_y) {
+    y + x
+  }
+}
+
+
+----- ERROR
+error: Unknown variable
+  ┌─ /src/one/two.gleam:4:5
+  │
+3 │   fn(_y) {
+  │      -- Did you mean to use this ignored variable?
+4 │     y + x
+  │     ^
+
+The name `y` is not in scope here.

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__shadowed_function_argument.snap

···
+---
+source: compiler-core/src/type_/tests/errors.rs
+expression: "\npub fn go(_x) {\n  x + 1\n}\n"
+---
+----- SOURCE CODE
+
+pub fn go(_x) {
+  x + 1
+}
+
+
+----- ERROR
+error: Unknown variable
+  ┌─ /src/one/two.gleam:3:3
+  │
+2 │ pub fn go(_x) {
+  │           -- Did you mean to use this ignored variable?
+3 │   x + 1
+  │   ^
+
+The name `x` is not in scope here.

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__shadowed_let_variable.snap

···
+---
+source: compiler-core/src/type_/tests/errors.rs
+expression: "\npub fn go() {\n  let _x = 1\n  x + 1\n}\n"
+---
+----- SOURCE CODE
+
+pub fn go() {
+  let _x = 1
+  x + 1
+}
+
+
+----- ERROR
+error: Unknown variable
+  ┌─ /src/one/two.gleam:4:3
+  │
+3 │   let _x = 1
+  │       -- Did you mean to use this ignored variable?
+4 │   x + 1
+  │   ^
+
+The name `x` is not in scope here.

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__shadowed_pattern_variable.snap

···
+---
+source: compiler-core/src/type_/tests/errors.rs
+expression: "\npub type Wibble {\n  Wibble(Int)\n}\n\npub fn go(x) {\n  case x {\n    Wibble(_y) -> y + 1\n  }\n}\n"
+---
+----- SOURCE CODE
+
+pub type Wibble {
+  Wibble(Int)
+}
+
+pub fn go(x) {
+  case x {
+    Wibble(_y) -> y + 1
+  }
+}
+
+
+----- ERROR
+error: Unknown variable
+  ┌─ /src/one/two.gleam:8:19
+  │
+8 │     Wibble(_y) -> y + 1
+  │            --     ^
+  │            │       
+  │            Did you mean to use this ignored variable?
+
+The name `y` is not in scope here.