Fix guard variable scoping

CHANGELOG.md

@@ -117,3 +117,8 @@
 - Fixed a bug where useless comparison warnings for floats compared literal
   strings, claiming for example that `1.0 == 1.` was always false.
   ([fruno](https://github.com/fruno-bulax/))
+
+- Fixed a bug where pattern variables in case clause guards would incorrectly
+  shadow outer scope variables in other branches when compiling to JavaScript.
+  ([Elias Haider](https://github.com/EliasDerHai))
+

compiler-core/src/javascript/decision.rs

@@ -584,20 +584,24 @@ impl<'a> CasePrinter<'_, '_, 'a, '_> {
             .iter()
             .partition(|(variable, _)| guard_variables.contains(variable));
 
-        let check_bindings = self.variables.bindings_ref_doc(&check_bindings);
-        let check = self.variables.expression_generator.guard(guard);
-        let if_true = self.inside_new_scope(|this| {
+        let (check_bindings, check, if_true) = self.inside_new_scope(|this| {
+            // check_bindings and if_true generation have to be in this scope so that pattern-bound
+            // variables used in guards don't leak into other case branches (if_false).
+            let check_bindings = this.variables.bindings_ref_doc(&check_bindings);
+            let check = this.variables.expression_generator.guard(guard);
             // All the other bindings that are not needed by the guard check will
             // end up directly in the body of the if clause.
             let if_true_bindings = this.variables.bindings_ref_doc(&if_true_bindings);
             let if_true_body = this.body_expression(if_true.clause_index);
-            match if_true_body {
+            let if_true = match if_true_body {
                 BodyExpression::Variable(variable) => variable,
                 BodyExpression::Expressions(if_true_body) => {
                     join_with_line(if_true_bindings, if_true_body)
                 }
-            }
+            };
+            (check_bindings, check, if_true)
         });
+
         let if_false_body = self.inside_new_scope(|this| this.decision(if_false));
 
         // We can now piece everything together into a case body!

compiler-core/src/javascript/tests/case_clause_guards.rs

@@ -576,3 +576,30 @@ pub fn main() {
 "#,
     );
 }
+
+// https://github.com/gleam-lang/gleam/issues/5094
+#[test]
+fn guard_pattern_does_not_shadow_outer_scope() {
+    assert_js!(
+        r#"
+pub type Option(a) {
+  Some(a)
+  None
+}
+
+pub type Container {
+  Container(x: Option(Int))
+}
+
+pub fn main() {
+  let x: Option(Int) = Some(42)
+  case Some(1) {
+    Some(x) if x < 0 -> Container(None)
+    _ -> {
+      Container(x:)
+    }
+  }
+}
+"#,
+    );
+}

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case__guard_variable_only_brought_into_scope_when_needed_1.snap

@@ -20,8 +20,8 @@ export function main() {
   if (i === 1) {
     return true;
   } else {
-    let i$1 = $;
-    if (i$1 < 2) {
+    let i = $;
+    if (i < 2) {
       return true;
     } else {
       return false;

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case__list_with_guard.snap

@@ -24,8 +24,8 @@ export function go(x) {
     if (first < 10) {
       return first * 2;
     } else {
-      let first$1 = x.head;
-      return first$1;
+      let first = x.head;
+      return first;
     }
   }
 }

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case_clause_guards__guard_pattern_does_not_shadow_outer_scope.snap

@@ -0,0 +1,64 @@
+---
+source: compiler-core/src/javascript/tests/case_clause_guards.rs
+expression: "\npub type Option(a) {\n  Some(a)\n  None\n}\n\npub type Container {\n  Container(x: Option(Int))\n}\n\npub fn main() {\n  let x: Option(Int) = Some(42)\n  case Some(1) {\n    Some(x) if x < 0 -> Container(None)\n    _ -> {\n      Container(x:)\n    }\n  }\n}\n"
+---
+----- SOURCE CODE
+
+pub type Option(a) {
+  Some(a)
+  None
+}
+
+pub type Container {
+  Container(x: Option(Int))
+}
+
+pub fn main() {
+  let x: Option(Int) = Some(42)
+  case Some(1) {
+    Some(x) if x < 0 -> Container(None)
+    _ -> {
+      Container(x:)
+    }
+  }
+}
+
+
+----- COMPILED JAVASCRIPT
+import { CustomType as $CustomType } from "../gleam.mjs";
+
+export class Some extends $CustomType {
+  constructor($0) {
+    super();
+    this[0] = $0;
+  }
+}
+export const Option$Some = ($0) => new Some($0);
+export const Option$isSome = (value) => value instanceof Some;
+export const Option$Some$0 = (value) => value[0];
+
+export class None extends $CustomType {}
+export const Option$None = () => new None();
+export const Option$isNone = (value) => value instanceof None;
+
+export class Container extends $CustomType {
+  constructor(x) {
+    super();
+    this.x = x;
+  }
+}
+export const Container$Container = (x) => new Container(x);
+export const Container$isContainer = (value) => value instanceof Container;
+export const Container$Container$x = (value) => value.x;
+export const Container$Container$0 = (value) => value.x;
+
+export function main() {
+  let x = new Some(42);
+  let $ = new Some(1);
+  let x$1 = $[0];
+  if (x$1 < 0) {
+    return new Container(new None());
+  } else {
+    return new Container(x);
+  }
+}