Use indexing syntax for invalid JS identifiers (rustwasm#3146)

* Use indexing syntax for invalid JS identifiers

Fixes rustwasm#3113

* fmt

crates/cli-support/Cargo.toml

@@ -28,3 +28,4 @@ wasm-bindgen-wasm-interpreter = { path = "../wasm-interpreter", version = '=0.2.
 wit-text = "0.8.0"
 wit-walrus = "0.6.0"
 wit-validator = "0.2.0"
+unicode-ident = "1.0.5"

crates/cli-support/src/js/mod.rs

@@ -2951,7 +2951,12 @@ impl<'a> Context<'a> {
 
             AuxImport::ValueWithThis(class, name) => {
                 let class = self.import_name(class)?;
-                Ok(format!("{}.{}({})", class, name, variadic_args(&args)?))
+                Ok(format!(
+                    "{}{}({})",
+                    class,
+                    property_accessor(name),
+                    variadic_args(&args)?
+                ))
             }
 
             AuxImport::Instanceof(js) => {
@@ -3023,37 +3028,52 @@ impl<'a> Context<'a> {
                     Some(pair) => pair,
                     None => bail!("structural method calls must have at least one argument"),
                 };
-                Ok(format!("{}.{}({})", receiver, name, variadic_args(args)?))
+                Ok(format!(
+                    "{}{}({})",
+                    receiver,
+                    property_accessor(name),
+                    variadic_args(args)?
+                ))
             }
 
             AuxImport::StructuralGetter(field) => {
                 assert!(kind == AdapterJsImportKind::Normal);
                 assert!(!variadic);
                 assert_eq!(args.len(), 1);
-                Ok(format!("{}.{}", args[0], field))
+                Ok(format!("{}{}", args[0], property_accessor(field)))
             }
 
             AuxImport::StructuralClassGetter(class, field) => {
                 assert!(kind == AdapterJsImportKind::Normal);
                 assert!(!variadic);
                 assert_eq!(args.len(), 0);
                 let class = self.import_name(class)?;
-                Ok(format!("{}.{}", class, field))
+                Ok(format!("{}{}", class, property_accessor(field)))
             }
 
             AuxImport::StructuralSetter(field) => {
                 assert!(kind == AdapterJsImportKind::Normal);
                 assert!(!variadic);
                 assert_eq!(args.len(), 2);
-                Ok(format!("{}.{} = {}", args[0], field, args[1]))
+                Ok(format!(
+                    "{}{} = {}",
+                    args[0],
+                    property_accessor(field),
+                    args[1]
+                ))
             }
 
             AuxImport::StructuralClassSetter(class, field) => {
                 assert!(kind == AdapterJsImportKind::Normal);
                 assert!(!variadic);
                 assert_eq!(args.len(), 1);
                 let class = self.import_name(class)?;
-                Ok(format!("{}.{} = {}", class, field, args[0]))
+                Ok(format!(
+                    "{}{} = {}",
+                    class,
+                    property_accessor(field),
+                    args[0]
+                ))
             }
 
             AuxImport::IndexingGetterOfClass(class) => {
@@ -3912,6 +3932,62 @@ fn require_class<'a>(
         .or_insert_with(ExportedClass::default)
 }
 
+/// Returns whether a character has the Unicode `ID_Start` properly.
+///
+/// This is only ever-so-slightly different from `XID_Start` in a few edge
+/// cases, so we handle those edge cases manually and delegate everything else
+/// to `unicode-ident`.
+fn is_id_start(c: char) -> bool {
+    match c {
+        '\u{037A}' | '\u{0E33}' | '\u{0EB3}' | '\u{309B}' | '\u{309C}' | '\u{FC5E}'
+        | '\u{FC5F}' | '\u{FC60}' | '\u{FC61}' | '\u{FC62}' | '\u{FC63}' | '\u{FDFA}'
+        | '\u{FDFB}' | '\u{FE70}' | '\u{FE72}' | '\u{FE74}' | '\u{FE76}' | '\u{FE78}'
+        | '\u{FE7A}' | '\u{FE7C}' | '\u{FE7E}' | '\u{FF9E}' | '\u{FF9F}' => true,
+        _ => unicode_ident::is_xid_start(c),
+    }
+}
+
+/// Returns whether a character has the Unicode `ID_Continue` properly.
+///
+/// This is only ever-so-slightly different from `XID_Continue` in a few edge
+/// cases, so we handle those edge cases manually and delegate everything else
+/// to `unicode-ident`.
+fn is_id_continue(c: char) -> bool {
+    match c {
+        '\u{037A}' | '\u{309B}' | '\u{309C}' | '\u{FC5E}' | '\u{FC5F}' | '\u{FC60}'
+        | '\u{FC61}' | '\u{FC62}' | '\u{FC63}' | '\u{FDFA}' | '\u{FDFB}' | '\u{FE70}'
+        | '\u{FE72}' | '\u{FE74}' | '\u{FE76}' | '\u{FE78}' | '\u{FE7A}' | '\u{FE7C}'
+        | '\u{FE7E}' => true,
+        _ => unicode_ident::is_xid_continue(c),
+    }
+}
+
+/// Returns whether a string is a valid JavaScript identifier.
+/// Defined at https://tc39.es/ecma262/#prod-IdentifierName.
+fn is_valid_ident(name: &str) -> bool {
+    name.chars().enumerate().all(|(i, char)| {
+        if i == 0 {
+            is_id_start(char) || char == '$' || char == '_'
+        } else {
+            is_id_continue(char) || char == '$' || char == '\u{200C}' || char == '\u{200D}'
+        }
+    })
+}
+
+/// Returns a string to tack on to the end of an expression to access a
+/// property named `name` of the object that expression resolves to.
+///
+/// In most cases, this is `.<name>`, generating accesses like `foo.bar`.
+/// However, if `name` is not a valid JavaScript identifier, it becomes
+/// `["<name>"]` instead, creating accesses like `foo["kebab-case"]`.
+fn property_accessor(name: &str) -> String {
+    if is_valid_ident(name) {
+        format!(".{name}")
+    } else {
+        format!("[\"{}\"]", name.escape_default())
+    }
+}
+
 impl ExportedClass {
     fn push(
         &mut self,

tests/wasm/import_class.js

@@ -25,6 +25,26 @@ class Construct {
   assert_internal_string(s) {
     assert.strictEqual(this.internal_string, s);
   }
+
+  ["kebab-case"]() {
+    return 42;
+  }
+
+  get ["kebab-case-val"]() {
+    return 42;
+  }
+
+  set ["kebab-case-val"](val) {}
+
+  static ["static-kebab-case"]() {
+    return 42;
+  }
+
+  static get ["static-kebab-case-val"]() {
+    return 42;
+  }
+
+  static set ["static-kebab-case-val"](val) {}
 }
 
 Construct.internal_string = '';

tests/wasm/import_class.rs

@@ -23,6 +23,19 @@ extern "C" {
     #[wasm_bindgen(method)]
     fn assert_internal_string(this: &Construct, s: &str);
 
+    #[wasm_bindgen(method, js_name = "kebab-case")]
+    fn kebab_case(this: &Construct) -> u32;
+    #[wasm_bindgen(method, getter, js_name = "kebab-case-val")]
+    fn kebab_case_val(this: &Construct) -> u32;
+    #[wasm_bindgen(method, setter, js_name = "kebab-case-val")]
+    fn set_kebab_case_val(this: &Construct, val: u32);
+    #[wasm_bindgen(static_method_of = Construct, js_name = "static-kebab-case")]
+    fn static_kebab_case() -> u32;
+    #[wasm_bindgen(static_method_of = Construct, getter, js_name = "static-kebab-case-val")]
+    fn static_kebab_case_val() -> u32;
+    #[wasm_bindgen(static_method_of = Construct, setter, js_name = "static-kebab-case-val")]
+    fn set_static_kebab_case_val(val: u32);
+
     type NewConstructors;
     #[wasm_bindgen(constructor)]
     fn new(arg: i32) -> NewConstructors;
@@ -137,6 +150,14 @@ fn construct() {
     assert_eq!(f.clone().get_internal_string(), "this");
     f.append_to_internal_string(" foo");
     f.assert_internal_string("this foo");
+
+    assert_eq!(f.kebab_case(), 42);
+    f.set_kebab_case_val(0);
+    // our setter does nothing so this is 42 anyway
+    assert_eq!(f.kebab_case_val(), 42);
+    assert_eq!(Construct::static_kebab_case(), 42);
+    Construct::set_static_kebab_case_val(0);
+    assert_eq!(Construct::static_kebab_case_val(), 42);
 }
 
 #[wasm_bindgen_test]

tests/wasm/imports.js

@@ -140,4 +140,7 @@ exports.same_name_from_import = (a) => a * 3;
 
 exports.same_js_namespace_from_module = {
   func_from_module_1_same_js_namespace: (a) => a * 5
-}
+}
+
+exports["kebab-case"] = () => 42;
+exports["\"string'literal\nbreakers\r"] = () => 42;

tests/wasm/imports.rs

@@ -81,6 +81,12 @@ extern "C" {
 
     #[wasm_bindgen(js_namespace = same_js_namespace_from_module)]
     fn func_from_module_1_same_js_namespace(s: i32) -> i32;
+
+    #[wasm_bindgen(js_name = "kebab-case")]
+    fn kebab_case() -> u32;
+
+    #[wasm_bindgen(js_name = "\"string'literal\nbreakers\r")]
+    fn string_literal_breakers() -> u32;
 }
 
 #[wasm_bindgen(module = "tests/wasm/imports_2.js")]
@@ -321,3 +327,9 @@ fn func_from_two_modules_same_js_namespace() {
     assert_eq!(func_from_module_1_same_js_namespace(2), 10);
     assert_eq!(func_from_module_2_same_js_namespace(2), 12);
 }
+
+#[wasm_bindgen_test]
+fn invalid_idents() {
+    assert_eq!(kebab_case(), 42);
+    assert_eq!(string_literal_breakers(), 42);
+}