Repository: gleam-lang/gleam (rust)

Issue:
Invalid JS generated when compiling a record that has a field named `constructor`, crashes at runtime
<!--
Please include the following:
- Steps and code to reproduce the problem
- Versions for Gleam, OS, the runtime and any other software used
- Logs from running Gleam with the `GLEAM_LOG` environment variable set to `trace`
-->

The crash seems to be due to the call to `withFields` during a record update, as that function calls the constructor of the type

Reproduction: 
```gleam
pub type Thing {
  Thing(constructor: Nil)
}

pub fn main() {
  let a = Thing(constructor: Nil)
  let b = Thing(..a)
}
```

Resulting JS for type:
```js
export class Thing extends $CustomType {
  constructor(constructor) {
    super();
    this.constructor = constructor;
  }
}
```

Gleam version: 1.5.1

Crash:
```text
TypeError: this.constructor is not a constructor
    at Thing.withFields (file:///Users/rattard/code/gleam/constructor/build/dev/javascript/prelude.mjs:9:12)
    at main (file:///Users/rattard/code/gleam/constructor/build/dev/javascript/constructor/constructor.mjs:12:13)
    at file:///Users/rattard/code/gleam/constructor/build/dev/javascript/constructor/gleam.main.mjs:2:1
    at ModuleJob.run (node:internal/modules/esm/module_job:268:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:543:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:116:5)

Node.js v23.1.0
```

Pull request description:
Escape property names with special meaning on JavaScript
Escape `constructor`, `then`, `prototype` and `__proto__`, since they all have special meaning.

fix #3813

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 42d27dc88..19f863b57 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Changelog
 
+## Unreleased
+
+### Bug fixes
+
+- Fixed a bug where some reserved field names were not properly escaped in
+  custom types on the JavaScript target.
+  ([yoshi](https://github.com/joshi-monster))
+
 ## v1.6.0-rc1 - 2024-11-10
 
 ### Build tool
diff --git a/compiler-core/src/javascript.rs b/compiler-core/src/javascript.rs
index 82becb1ec..d5b681aa4 100644
--- a/compiler-core/src/javascript.rs
+++ b/compiler-core/src/javascript.rs
@@ -293,7 +293,9 @@ impl<'a> Generator<'a> {
                 let var = parameter((i, arg));
                 match &arg.label {
                     None => docvec!["this[", i, "] = ", var, ";"],
-                    Some((_, name)) => docvec!["this.", name, " = ", var, ";"],
+                    Some((_, name)) => {
+                        docvec!["this.", maybe_escape_property_doc(name), " = ", var, ";"]
+                    }
                 }
             }),
             line(),
@@ -740,6 +742,21 @@ fn is_usable_js_identifier(word: &str) -> bool {
     )
 }
 
+fn is_usable_js_property(label: &str) -> bool {
+    !matches!(
+        label,
+        // `then` to avoid a custom type that defines a `then` function being used as a `thenable`
+        // in Javascript.
+        "then"
+            // `constructor` to avoid unintentional overriding of the constructor of records,
+            // leading to potential runtime crashes while using `withFields`.
+            | "constructor"
+            // `prototype` and `__proto__` to avoid unintentionally overriding the prototype chain
+            | "prototype"
+            | "__proto__"
+    )
+}
+
 fn maybe_escape_identifier_string(word: &str) -> EcoString {
     if is_usable_js_identifier(word) {
         EcoString::from(word)
@@ -760,6 +777,14 @@ fn maybe_escape_identifier_doc(word: &str) -> Document<'_> {
     }
 }
 
+fn maybe_escape_property_doc(label: &str) -> Document<'_> {
+    if is_usable_js_property(label) {
+        label.to_doc()
+    } else {
+        escape_identifier(label).to_doc()
+    }
+}
+
 #[derive(Debug, Default)]
 pub(crate) struct UsageTracker {
     pub ok_used: bool,
diff --git a/compiler-core/src/javascript/expression.rs b/compiler-core/src/javascript/expression.rs
index 5892fabe5..a8c4c8fcc 100644
--- a/compiler-core/src/javascript/expression.rs
+++ b/compiler-core/src/javascript/expression.rs
@@ -854,7 +854,7 @@ impl<'module> Generator<'module> {
     fn record_access<'a>(&mut self, record: &'a TypedExpr, label: &'a str) -> Output<'a> {
         self.not_in_tail_position(|gen| {
             let record = gen.wrap_expression(record)?;
-            Ok(docvec![record, ".", label])
+            Ok(docvec![record, ".", maybe_escape_property_doc(label)])
         })
     }
 
@@ -868,7 +868,7 @@ impl<'module> Generator<'module> {
             let fields = updates
                 .iter()
                 .map(|TypedRecordUpdateArg { label, value, .. }| {
-                    (label.to_doc(), gen.wrap_expression(value))
+                    (maybe_escape_property_doc(label), gen.wrap_expression(value))
                 });
             let object = try_wrap_object(fields)?;
             Ok(docvec![record, ".withFields(", object, ")"])
diff --git a/compiler-core/src/javascript/pattern.rs b/compiler-core/src/javascript/pattern.rs
index 6e5ff6653..5cd041eac 100644
--- a/compiler-core/src/javascript/pattern.rs
+++ b/compiler-core/src/javascript/pattern.rs
@@ -156,7 +156,7 @@ impl<'module_ctx, 'expression_gen, 'a> Generator<'module_ctx, 'expression_gen, '
         concat(self.path.iter().map(|segment| match segment {
             Index::Int(i) => eco_format!("[{i}]").to_doc(),
             // TODO: escape string if needed
-            Index::String(s) => docvec!(".", s),
+            Index::String(s) => docvec!(".", maybe_escape_property_doc(s)),
             Index::ByteAt(i) => docvec!(".byteAt(", i, ")"),
             Index::IntFromSlice {
                 start,
@@ -365,7 +365,11 @@ impl<'module_ctx, 'expression_gen, 'a> Generator<'module_ctx, 'expression_gen, '
             ClauseGuard::FieldAccess {
                 label, container, ..
             } => {
-                docvec!(self.guard(container)?, ".", label)
+                docvec!(
+                    self.guard(container)?,
+                    ".",
+                    maybe_escape_property_doc(label)
+                )
             }
 
             ClauseGuard::ModuleSelect {


Test patch:
diff --git a/compiler-core/src/javascript/tests/custom_types.rs b/compiler-core/src/javascript/tests/custom_types.rs
index e08376113..19693f5ea 100644
--- a/compiler-core/src/javascript/tests/custom_types.rs
+++ b/compiler-core/src/javascript/tests/custom_types.rs
@@ -693,3 +693,77 @@ pub fn main() {
 "#
     );
 }
+
+// https://github.com/gleam-lang/gleam/issues/3813
+#[test]
+fn record_with_field_named_constructor() {
+    assert_js!(
+        r#"
+pub type Thing {
+  Thing(constructor: Nil)
+}
+
+pub fn main() {
+  let a = Thing(constructor: Nil)
+  let b = Thing(..a, constructor: Nil)
+  b.constructor
+}
+"#
+    );
+}
+
+#[test]
+fn record_with_field_named_then() {
+    assert_js!(
+        r#"
+pub type Thing {
+  Thing(then: Nil)
+}
+
+pub fn main() {
+  let a = Thing(then: Nil)
+  let b = Thing(..a, then: Nil)
+  b.then
+}
+"#
+    );
+}
+#[test]
+fn record_access_in_guard_with_reserved_field_name() {
+    assert_js!(
+        r#"
+pub type Thing {
+  Thing(constructor: Nil)
+}
+
+pub fn main() {
+  let a = Thing(constructor: Nil)
+  case Nil {
+      Nil if a.constructor == Nil -> a.constructor
+      _ -> Nil
+  }
+}
+"#
+    );
+}
+
+#[test]
+fn record_access_in_pattern_with_reserved_field_name() {
+    assert_js!(
+        r#"
+pub type Thing {
+  Thing(constructor: Nil)
+}
+
+pub fn main() {
+  let a = Thing(constructor: Nil)
+  let Thing(constructor: ctor) = a
+  case a {
+      a if a.constructor == ctor -> Nil
+      Thing(constructor:) if ctor == constructor -> Nil
+      _ -> Nil
+  }
+}
+"#
+    );
+}
diff --git a/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_access_in_guard_with_reserved_field_name.snap b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_access_in_guard_with_reserved_field_name.snap
new file mode 100644
index 000000000..1141aa508
--- /dev/null
+++ b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_access_in_guard_with_reserved_field_name.snap
@@ -0,0 +1,38 @@
+---
+source: compiler-core/src/javascript/tests/custom_types.rs
+expression: "\npub type Thing {\n  Thing(constructor: Nil)\n}\n\npub fn main() {\n  let a = Thing(constructor: Nil)\n  case Nil {\n      Nil if a.constructor == Nil -> a.constructor\n      _ -> Nil\n  }\n}\n"
+---
+----- SOURCE CODE
+
+pub type Thing {
+  Thing(constructor: Nil)
+}
+
+pub fn main() {
+  let a = Thing(constructor: Nil)
+  case Nil {
+      Nil if a.constructor == Nil -> a.constructor
+      _ -> Nil
+  }
+}
+
+
+----- COMPILED JAVASCRIPT
+import { CustomType as $CustomType } from "../gleam.mjs";
+
+export class Thing extends $CustomType {
+  constructor(constructor) {
+    super();
+    this.constructor$ = constructor;
+  }
+}
+
+export function main() {
+  let a = new Thing(undefined);
+  let $ = undefined;
+  if (!$ && (a.constructor$ === undefined)) {
+    return a.constructor$;
+  } else {
+    return undefined;
+  }
+}
diff --git a/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_access_in_pattern_with_reserved_field_name.snap b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_access_in_pattern_with_reserved_field_name.snap
new file mode 100644
index 000000000..618c625a6
--- /dev/null
+++ b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_access_in_pattern_with_reserved_field_name.snap
@@ -0,0 +1,44 @@
+---
+source: compiler-core/src/javascript/tests/custom_types.rs
+expression: "\npub type Thing {\n  Thing(constructor: Nil)\n}\n\npub fn main() {\n  let a = Thing(constructor: Nil)\n  let Thing(constructor: ctor) = a\n  case a {\n      a if a.constructor == ctor -> Nil\n      Thing(constructor:) if ctor == constructor -> Nil\n      _ -> Nil\n  }\n}\n"
+---
+----- SOURCE CODE
+
+pub type Thing {
+  Thing(constructor: Nil)
+}
+
+pub fn main() {
+  let a = Thing(constructor: Nil)
+  let Thing(constructor: ctor) = a
+  case a {
+      a if a.constructor == ctor -> Nil
+      Thing(constructor:) if ctor == constructor -> Nil
+      _ -> Nil
+  }
+}
+
+
+----- COMPILED JAVASCRIPT
+import { CustomType as $CustomType } from "../gleam.mjs";
+
+export class Thing extends $CustomType {
+  constructor(constructor) {
+    super();
+    this.constructor$ = constructor;
+  }
+}
+
+export function main() {
+  let a = new Thing(undefined);
+  let ctor = a.constructor$;
+  if (a.constructor$ === ctor) {
+    let a$1 = a;
+    return undefined;
+  } else if (a instanceof Thing && (ctor === a.constructor$)) {
+    let constructor = a.constructor$;
+    return undefined;
+  } else {
+    return undefined;
+  }
+}
diff --git a/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_with_field_named_constructor.snap b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_with_field_named_constructor.snap
new file mode 100644
index 000000000..e6cb41def
--- /dev/null
+++ b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_with_field_named_constructor.snap
@@ -0,0 +1,32 @@
+---
+source: compiler-core/src/javascript/tests/custom_types.rs
+expression: "\npub type Thing {\n  Thing(constructor: Nil)\n}\n\npub fn main() {\n  let a = Thing(constructor: Nil)\n  let b = Thing(..a, constructor: Nil)\n  b.constructor\n}\n"
+---
+----- SOURCE CODE
+
+pub type Thing {
+  Thing(constructor: Nil)
+}
+
+pub fn main() {
+  let a = Thing(constructor: Nil)
+  let b = Thing(..a, constructor: Nil)
+  b.constructor
+}
+
+
+----- COMPILED JAVASCRIPT
+import { CustomType as $CustomType } from "../gleam.mjs";
+
+export class Thing extends $CustomType {
+  constructor(constructor) {
+    super();
+    this.constructor$ = constructor;
+  }
+}
+
+export function main() {
+  let a = new Thing(undefined);
+  let b = a.withFields({ constructor$: undefined });
+  return b.constructor$;
+}
diff --git a/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_with_field_named_then.snap b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_with_field_named_then.snap
new file mode 100644
index 000000000..940bc9dcf
--- /dev/null
+++ b/compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__custom_types__record_with_field_named_then.snap
@@ -0,0 +1,32 @@
+---
+source: compiler-core/src/javascript/tests/custom_types.rs
+expression: "\npub type Thing {\n  Thing(then: Nil)\n}\n\npub fn main() {\n  let a = Thing(then: Nil)\n  let b = Thing(..a, then: Nil)\n  b.then\n}\n"
+---
+----- SOURCE CODE
+
+pub type Thing {
+  Thing(then: Nil)
+}
+
+pub fn main() {
+  let a = Thing(then: Nil)
+  let b = Thing(..a, then: Nil)
+  b.then
+}
+
+
+----- COMPILED JAVASCRIPT
+import { CustomType as $CustomType } from "../gleam.mjs";
+
+export class Thing extends $CustomType {
+  constructor(then$) {
+    super();
+    this.then$ = then$;
+  }
+}
+
+export function main() {
+  let a = new Thing(undefined);
+  let b = a.withFields({ then$: undefined });
+  return b.then$;
+}