diff --git a/cli/src/repl.rs b/cli/src/repl.rs index 61c10a03a5..0ca12fba55 100644 --- a/cli/src/repl.rs +++ b/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)), diff --git a/cli/tests/repl_eval.rs b/cli/tests/repl_eval.rs index 3d36609272..5069c29f98 100644 --- a/cli/tests/repl_eval.rs +++ b/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. + "# + ), + ); + } } diff --git a/compiler/can/src/annotation.rs b/compiler/can/src/annotation.rs index 2867a23f03..a78ed60cb9 100644 --- a/compiler/can/src/annotation.rs +++ b/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 { diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 05584133e2..43e7af2e95 100644 --- a/compiler/can/src/def.rs +++ b/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, diff --git a/compiler/can/src/module.rs b/compiler/can/src/module.rs index 1f57cc8734..7a292807bb 100644 --- a/compiler/can/src/module.rs +++ b/compiler/can/src/module.rs @@ -398,7 +398,7 @@ fn fix_values_captured_in_closure_pattern( | FloatLiteral(_, _, _) | StrLiteral(_) | Underscore - | Shadowed(_, _) + | Shadowed(..) | MalformedPattern(_, _) | UnsupportedPattern(_) => (), } diff --git a/compiler/can/src/pattern.rs b/compiler/can/src/pattern.rs index 66132dbafb..6c9392e919 100644 --- a/compiler/can/src/pattern.rs +++ b/compiler/can/src/pattern.rs @@ -32,7 +32,7 @@ pub enum Pattern { Underscore, // Runtime Exceptions - Shadowed(Region, Loc), + Shadowed(Region, Loc, 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) { 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) { | 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(_) => (), } diff --git a/compiler/can/src/scope.rs b/compiler/can/src/scope.rs index eeab87438d..de7b233b08 100644 --- a/compiler/can/src/scope.rs +++ b/compiler/can/src/scope.rs @@ -110,15 +110,21 @@ impl Scope { exposed_ident_ids: &IdentIds, all_ident_ids: &mut IdentIds, region: Region, - ) -> Result)> { + ) -> Result, 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 diff --git a/compiler/can/tests/test_can.rs b/compiler/can/tests/test_can.rs index 26ec355746..196be508c0 100644 --- a/compiler/can/tests/test_can.rs +++ b/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, })); } diff --git a/compiler/constrain/src/pattern.rs b/compiler/constrain/src/pattern.rs index 3844ad3f08..2a1ce6fa46 100644 --- a/compiler/constrain/src/pattern.rs +++ b/compiler/constrain/src/pattern.rs @@ -48,12 +48,11 @@ fn headers_from_annotation_help( headers: &mut SendMap>, ) -> 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(), diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index d1e2402cca..9b83b2b388 100644 --- a/compiler/mono/src/ir.rs +++ b/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(), }), diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index d69da25ae1..c9550b9ab0 100644 --- a/reporting/tests/test_reporting.rs +++ b/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 ] ^^^^^^^^^^^^^^^^^^^