Merge pull request #2393 from rtfeldman/i/2343

Generate unique symbols for shadowing identifiers

cli/src/repl.rs

@@ -1,7 +1,7 @@
 use const_format::concatcp;
 #[cfg(feature = "llvm")]
 use gen::{gen_and_eval, ReplOutput};
-use roc_parse::parser::{EExpr, SyntaxError};
+use roc_parse::parser::{EExpr, ELambda, SyntaxError};
 use rustyline::highlight::{Highlighter, PromptInfo};
 use rustyline::validate::{self, ValidationContext, ValidationResult, Validator};
 use rustyline_derive::{Completer, Helper, Hinter};
@@ -97,7 +97,8 @@ impl Validator for InputValidator {
             match roc_parse::expr::parse_loc_expr(0, &arena, state) {
                 // Special case some syntax errors to allow for multi-line inputs
                 Err((_, EExpr::DefMissingFinalExpr(_), _))
-                | Err((_, EExpr::DefMissingFinalExpr2(_, _), _)) => {
+                | Err((_, EExpr::DefMissingFinalExpr2(_, _), _))
+                | Err((_, EExpr::Lambda(ELambda::Body(_, _), _), _)) => {
                     Ok(ValidationResult::Incomplete)
                 }
                 _ => Ok(ValidationResult::Valid(None)),

cli/tests/repl_eval.rs

@@ -925,4 +925,38 @@ mod repl_eval {
             ),
         );
     }
+
+    #[test]
+    fn issue_2343_complete_mono_with_shadowed_vars() {
+        expect_failure(
+            indoc!(
+                r#"
+                b = False
+                f = \b ->
+                    when b is
+                        True -> 5
+                        False -> 15
+                f b
+                "#
+            ),
+            indoc!(
+                r#"
+                ── DUPLICATE NAME ──────────────────────────────────────────────────────────────
+
+                The b name is first defined here:
+
+                4│      b = False
+                        ^
+
+                But then it's defined a second time here:
+
+                5│      f = \b ->
+                             ^
+
+                Since these variables have the same name, it's easy to use the wrong
+                one on accident. Give one of them a new name.
+                "#
+            ),
+        );
+    }
 }

compiler/can/src/annotation.rs

@@ -390,7 +390,7 @@ fn can_annotation_help(
             ) {
                 Ok(symbol) => symbol,
 
-                Err((original_region, shadow)) => {
+                Err((original_region, shadow, _new_symbol)) => {
                     let problem = Problem::Shadowed(original_region, shadow.clone());
 
                     env.problem(roc_problem::can::Problem::ShadowingInAnnotation {

compiler/can/src/def.rs

@@ -809,7 +809,7 @@ fn pattern_to_vars_by_symbol(
 ) {
     use Pattern::*;
     match pattern {
-        Identifier(symbol) => {
+        Identifier(symbol) | Shadowed(_, _, symbol) => {
             vars_by_symbol.insert(*symbol, expr_var);
         }
 
@@ -832,8 +832,6 @@ fn pattern_to_vars_by_symbol(
         | Underscore
         | MalformedPattern(_, _)
         | UnsupportedPattern(_) => {}
-
-        Shadowed(_, _) => {}
     }
 }
 
@@ -884,7 +882,7 @@ fn canonicalize_pending_def<'a>(
                 Pattern::Identifier(symbol) => RuntimeError::NoImplementationNamed {
                     def_symbol: *symbol,
                 },
-                Pattern::Shadowed(region, loc_ident) => RuntimeError::Shadowing {
+                Pattern::Shadowed(region, loc_ident, _new_symbol) => RuntimeError::Shadowing {
                     original_region: *region,
                     shadow: loc_ident.clone(),
                 },
@@ -1502,7 +1500,7 @@ fn to_pending_def<'a>(
                     ))
                 }
 
-                Err((original_region, loc_shadowed_symbol)) => {
+                Err((original_region, loc_shadowed_symbol, _new_symbol)) => {
                     env.problem(Problem::ShadowingInAnnotation {
                         original_region,
                         shadow: loc_shadowed_symbol,

compiler/can/src/module.rs

@@ -398,7 +398,7 @@ fn fix_values_captured_in_closure_pattern(
         | FloatLiteral(_, _, _)
         | StrLiteral(_)
         | Underscore
-        | Shadowed(_, _)
+        | Shadowed(..)
         | MalformedPattern(_, _)
         | UnsupportedPattern(_) => (),
     }

compiler/can/src/pattern.rs

@@ -32,7 +32,7 @@ pub enum Pattern {
     Underscore,
 
     // Runtime Exceptions
-    Shadowed(Region, Loc<Ident>),
+    Shadowed(Region, Loc<Ident>, Symbol),
     // Example: (5 = 1 + 2) is an unsupported pattern in an assignment; Int patterns aren't allowed in assignments!
     UnsupportedPattern(Region),
     // parse error patterns
@@ -65,7 +65,7 @@ pub fn symbols_from_pattern_help(pattern: &Pattern, symbols: &mut Vec<Symbol>) {
     use Pattern::*;
 
     match pattern {
-        Identifier(symbol) => {
+        Identifier(symbol) | Shadowed(_, _, symbol) => {
             symbols.push(*symbol);
         }
 
@@ -92,8 +92,6 @@ pub fn symbols_from_pattern_help(pattern: &Pattern, symbols: &mut Vec<Symbol>) {
         | Underscore
         | MalformedPattern(_, _)
         | UnsupportedPattern(_) => {}
-
-        Shadowed(_, _) => {}
     }
 }
 
@@ -121,13 +119,14 @@ pub fn canonicalize_pattern<'a>(
 
                 Pattern::Identifier(symbol)
             }
-            Err((original_region, shadow)) => {
+            Err((original_region, shadow, new_symbol)) => {
                 env.problem(Problem::RuntimeError(RuntimeError::Shadowing {
                     original_region,
                     shadow: shadow.clone(),
                 }));
+                output.references.bound_symbols.insert(new_symbol);
 
-                Pattern::Shadowed(original_region, shadow)
+                Pattern::Shadowed(original_region, shadow, new_symbol)
             }
         },
         GlobalTag(name) => {
@@ -268,7 +267,7 @@ pub fn canonicalize_pattern<'a>(
                                     },
                                 });
                             }
-                            Err((original_region, shadow)) => {
+                            Err((original_region, shadow, new_symbol)) => {
                                 env.problem(Problem::RuntimeError(RuntimeError::Shadowing {
                                     original_region,
                                     shadow: shadow.clone(),
@@ -278,7 +277,8 @@ pub fn canonicalize_pattern<'a>(
                                 // are, we're definitely shadowed and will
                                 // get a runtime exception as soon as we
                                 // encounter the first bad pattern.
-                                opt_erroneous = Some(Pattern::Shadowed(original_region, shadow));
+                                opt_erroneous =
+                                    Some(Pattern::Shadowed(original_region, shadow, new_symbol));
                             }
                         };
                     }
@@ -339,7 +339,7 @@ pub fn canonicalize_pattern<'a>(
                                     },
                                 });
                             }
-                            Err((original_region, shadow)) => {
+                            Err((original_region, shadow, new_symbol)) => {
                                 env.problem(Problem::RuntimeError(RuntimeError::Shadowing {
                                     original_region,
                                     shadow: shadow.clone(),
@@ -349,7 +349,8 @@ pub fn canonicalize_pattern<'a>(
                                 // are, we're definitely shadowed and will
                                 // get a runtime exception as soon as we
                                 // encounter the first bad pattern.
-                                opt_erroneous = Some(Pattern::Shadowed(original_region, shadow));
+                                opt_erroneous =
+                                    Some(Pattern::Shadowed(original_region, shadow, new_symbol));
                             }
                         };
                     }
@@ -452,7 +453,7 @@ fn add_bindings_from_patterns(
     use Pattern::*;
 
     match pattern {
-        Identifier(symbol) => {
+        Identifier(symbol) | Shadowed(_, _, symbol) => {
             answer.push((*symbol, *region));
         }
         AppliedTag {
@@ -477,7 +478,6 @@ fn add_bindings_from_patterns(
         | FloatLiteral(_, _, _)
         | StrLiteral(_)
         | Underscore
-        | Shadowed(_, _)
         | MalformedPattern(_, _)
         | UnsupportedPattern(_) => (),
     }

compiler/can/src/scope.rs

@@ -110,15 +110,21 @@ impl Scope {
         exposed_ident_ids: &IdentIds,
         all_ident_ids: &mut IdentIds,
         region: Region,
-    ) -> Result<Symbol, (Region, Loc<Ident>)> {
+    ) -> Result<Symbol, (Region, Loc<Ident>, Symbol)> {
         match self.idents.get(&ident) {
-            Some((_, original_region)) => {
+            Some(&(_, original_region)) => {
                 let shadow = Loc {
-                    value: ident,
+                    value: ident.clone(),
                     region,
                 };
 
-                Err((*original_region, shadow))
+                let ident_id = all_ident_ids.add(ident.clone());
+                let symbol = Symbol::new(self.home, ident_id);
+
+                self.symbols.insert(symbol, region);
+                self.idents.insert(ident, (symbol, region));
+
+                Err((original_region, shadow, symbol))
             }
             None => {
                 // If this IdentId was already added previously

compiler/can/tests/test_can.rs

@@ -368,9 +368,11 @@ mod test_can {
         let arena = Bump::new();
         let CanExprOut { problems, .. } = can_expr_with(&arena, test_home(), src);
 
-        assert_eq!(problems.len(), 1);
+        assert_eq!(problems.len(), 2);
         assert!(problems.iter().all(|problem| match problem {
             Problem::RuntimeError(RuntimeError::Shadowing { .. }) => true,
+            // Due to one of the shadows
+            Problem::UnusedDef(..) => true,
             _ => false,
         }));
     }
@@ -389,9 +391,11 @@ mod test_can {
         let arena = Bump::new();
         let CanExprOut { problems, .. } = can_expr_with(&arena, test_home(), src);
 
-        assert_eq!(problems.len(), 1);
+        assert_eq!(problems.len(), 2);
         assert!(problems.iter().all(|problem| match problem {
             Problem::RuntimeError(RuntimeError::Shadowing { .. }) => true,
+            // Due to one of the shadows
+            Problem::UnusedDef(..) => true,
             _ => false,
         }));
     }
@@ -410,10 +414,12 @@ mod test_can {
         let arena = Bump::new();
         let CanExprOut { problems, .. } = can_expr_with(&arena, test_home(), src);
 
-        assert_eq!(problems.len(), 1);
+        assert_eq!(problems.len(), 2);
         println!("{:#?}", problems);
         assert!(problems.iter().all(|problem| match problem {
             Problem::RuntimeError(RuntimeError::Shadowing { .. }) => true,
+            // Due to one of the shadows
+            Problem::UnusedDef(..) => true,
             _ => false,
         }));
     }

compiler/constrain/src/pattern.rs

@@ -48,12 +48,11 @@ fn headers_from_annotation_help(
     headers: &mut SendMap<Symbol, Loc<Type>>,
 ) -> bool {
     match pattern {
-        Identifier(symbol) => {
+        Identifier(symbol) | Shadowed(_, _, symbol) => {
             headers.insert(*symbol, annotation.clone());
             true
         }
         Underscore
-        | Shadowed(_, _)
         | MalformedPattern(_, _)
         | UnsupportedPattern(_)
         | NumLiteral(_, _, _)
@@ -159,11 +158,11 @@ pub fn constrain_pattern(
                 PresenceConstraint::IsOpen,
             ));
         }
-        Underscore | UnsupportedPattern(_) | MalformedPattern(_, _) | Shadowed(_, _) => {
+        Underscore | UnsupportedPattern(_) | MalformedPattern(_, _) => {
             // Neither the _ pattern nor erroneous ones add any constraints.
         }
 
-        Identifier(symbol) => {
+        Identifier(symbol) | Shadowed(_, _, symbol) => {
             if destruct_position {
                 state.constraints.push(Constraint::Present(
                     expected.get_type_ref().clone(),

compiler/mono/src/ir.rs

@@ -1980,12 +1980,12 @@ fn pattern_to_when<'a>(
             // for underscore we generate a dummy Symbol
             (env.unique_symbol(), body)
         }
-        Shadowed(region, loc_ident) => {
+        Shadowed(region, loc_ident, new_symbol) => {
             let error = roc_problem::can::RuntimeError::Shadowing {
                 original_region: *region,
                 shadow: loc_ident.clone(),
             };
-            (env.unique_symbol(), Loc::at_zero(RuntimeError(error)))
+            (*new_symbol, Loc::at_zero(RuntimeError(error)))
         }
 
         UnsupportedPattern(region) => {
@@ -7653,7 +7653,7 @@ fn from_can_pattern_help<'a>(
             }
         }
         StrLiteral(v) => Ok(Pattern::StrLiteral(v.clone())),
-        Shadowed(region, ident) => Err(RuntimeError::Shadowing {
+        Shadowed(region, ident, _new_symbol) => Err(RuntimeError::Shadowing {
             original_region: *region,
             shadow: ident.clone(),
         }),

reporting/tests/test_reporting.rs

@@ -424,6 +424,16 @@ mod test_reporting {
 
                 `Booly` is not used anywhere in your code.
 
+                3│  Booly : [ Yes, No, Maybe ]
+                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
+                
+                If you didn't intend on using `Booly` then remove it so future readers
+                of your code don't wonder why it is there.
+                
+                ── UNUSED DEFINITION ───────────────────────────────────────────────────────────
+                
+                `Booly` is not used anywhere in your code.
+                
                 1│  Booly : [ Yes, No ]
                     ^^^^^^^^^^^^^^^^^^^