Fix handling of JS keyword for imported functions, types and statics, and when causing invalid code gen

CHANGELOG.md

@@ -1,6 +1,15 @@
 # `wasm-bindgen` Change Log
 --------------------------------------------------------------------------------
 
+## Unreleased
+
+### Fixed
+
+- Fixed JS keyword-like identifiers not being handled correctly for imports and exports.
+  [#4329](https://github.com/rustwasm/wasm-bindgen/pull/4329)
+
+--------------------------------------------------------------------------------
+
 ## [0.2.99](https://github.com/rustwasm/wasm-bindgen/compare/0.2.98...0.2.99)
 
 Released 2024-12-07

crates/cli/tests/reference/import.rs

@@ -25,6 +25,7 @@ extern "C" {
 
 #[wasm_bindgen(js_namespace = ["a"])]
 extern "C" {
+    // test that namespaces are overwritten and not inherited/concatenated
     #[wasm_bindgen(js_namespace = ["b"])]
     fn my_function();
     #[wasm_bindgen(thread_local_v2)]

crates/macro-support/src/parser.rs

@@ -21,9 +21,12 @@ thread_local!(static ATTRS: AttributeParseState = Default::default());
 
 /// Javascript keywords.
 ///
+/// Note that some of these keywords are only reserved in strict mode. Since we
+/// generate strict mode JS code, we treat all of these as reserved.
+///
 /// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#reserved_words
-const JS_KEYWORDS: [&str; 44] = [
-    "arguments", // reserved in strict mode, which we are in
+const JS_KEYWORDS: [&str; 47] = [
+    "arguments",
     "break",
     "case",
     "catch",
@@ -35,43 +38,69 @@ const JS_KEYWORDS: [&str; 44] = [
     "delete",
     "do",
     "else",
-    "enum", // always reserved, but not used
-    // "eval", reserved in strict mode, but behaves like a functions in JS, so we allow it
+    "enum",
+    "eval",
     "export",
     "extends",
-    // "false", false is a keyword, but behaves like a literal in JS
+    "false",
     "finally",
     "for",
     "function",
     "if",
+    "implements",
     "import",
-    "implements", // reserved in strict mode, which we are in
     "in",
     "instanceof",
-    "interface", // reserved in strict mode, which we are in
-    "let",       // reserved in strict mode, which we are in
+    "interface",
+    "let",
     "new",
     "null",
-    "package",   // reserved in strict mode, which we are in
-    "private",   // reserved in strict mode, which we are in
-    "protected", // reserved in strict mode, which we are in
-    "public",    // reserved in strict mode, which we are in
+    "package",
+    "private",
+    "protected",
+    "public",
     "return",
-    "static", // reserved in strict mode, which we are in
+    "static",
     "super",
     "switch",
     "this",
     "throw",
-    // "true", true is a keyword, but behaves like a literal in JS
+    "true",
     "try",
     "typeof",
     "var",
     "void",
     "while",
     "with",
-    "yield", // reserved in strict mode, which we are in
+    "yield",
+];
+
+/// Javascript keywords that behave like values in that they can be called like
+/// functions or have properties accessed on them.
+const VALUE_LIKE_JS_KEYWORDS: [&str; 7] = [
+    "eval",   // eval is a function-like keyword, so e.g. `eval(...)` is valid
+    "false",  // false resolves to a boolean value, so e.g. `false.toString()` is valid
+    "import", // import.meta and import()
+    "new",    // new.target
+    "super", // super can be used for a function call (`super(...)`) or property lookup (`super.prop`)
+    "this",  // this obviously can be used as a value
+    "true",  // true resolves to a boolean value, so e.g. `false.toString()` is valid
 ];
 
+/// Returns whether the given string is a JS keyword.
+fn is_js_keyword(keyword: &str) -> bool {
+    JS_KEYWORDS.contains(&keyword)
+}
+/// Returns whether the given string is a JS keyword that does NOT behave like
+/// a value.
+///
+/// Value-like keywords can be called like functions or have properties
+/// accessed, which makes it possible to use them in imports. In general,
+/// imports should use this function to check for reserved keywords.
+fn is_non_value_js_keyword(keyword: &str) -> bool {
+    JS_KEYWORDS.contains(&keyword) && !VALUE_LIKE_JS_KEYWORDS.contains(&keyword)
+}
+
 #[derive(Default)]
 struct AttributeParseState {
     parsed: Cell<usize>,
@@ -427,10 +456,9 @@ impl Parse for BindgenAttr {
                     }
                 };
 
-                // We need to allow `import`, because of the `import.meta` namespace.
                 let first = &vals[0];
-                if is_js_keyword(first) && first != "import" {
-                    let msg = format!("Namespace cannot start with the JS keyword `{}`", vals[0]);
+                if is_non_value_js_keyword(first) {
+                    let msg = format!("Namespace cannot start with the JS keyword `{}`", first);
                     return Err(syn::Error::new(spans[0], msg));
                 }
 
@@ -962,10 +990,6 @@ impl ConvertToAst<BindgenAttrs> for syn::ItemFn {
     }
 }
 
-fn is_js_keyword(keyword: &str) -> bool {
-    JS_KEYWORDS.iter().any(|this| *this == keyword)
-}
-
 /// Returns whether `self` is passed by reference or by value.
 fn get_self_method(r: syn::Receiver) -> ast::MethodSelf {
     // The tricky part here is that `r` can have many forms. E.g. `self`,
@@ -1780,42 +1804,43 @@ impl MacroParse<ForeignItemCtx> for syn::ForeignItem {
         // parsing. If there is a module, we can rename the import symbol to
         // avoid using keywords.
         let needs_check = js_namespace.is_none() && module.is_none();
-        match &kind {
-            ast::ImportKind::Function(import_function) => {
-                if needs_check
-                    && matches!(import_function.kind, ast::ImportFunctionKind::Normal)
-                    && is_js_keyword(&import_function.function.name)
-                {
-                    bail_span!(
-                        import_function.rust_name,
-                        "Imported function cannot use the JS keyword `{}` as its name.",
-                        import_function.function.name
-                    );
+        if needs_check {
+            match &kind {
+                ast::ImportKind::Function(import_function) => {
+                    if matches!(import_function.kind, ast::ImportFunctionKind::Normal)
+                        && is_non_value_js_keyword(&import_function.function.name)
+                    {
+                        bail_span!(
+                            import_function.rust_name,
+                            "Imported function cannot use the JS keyword `{}` as its name.",
+                            import_function.function.name
+                        );
+                    }
                 }
-            }
-            ast::ImportKind::Static(import_static) => {
-                if needs_check && is_js_keyword(&import_static.js_name) {
-                    bail_span!(
-                        import_static.rust_name,
-                        "Imported static cannot use the JS keyword `{}` as its name.",
-                        import_static.js_name
-                    );
+                ast::ImportKind::Static(import_static) => {
+                    if is_non_value_js_keyword(&import_static.js_name) {
+                        bail_span!(
+                            import_static.rust_name,
+                            "Imported static cannot use the JS keyword `{}` as its name.",
+                            import_static.js_name
+                        );
+                    }
                 }
-            }
-            ast::ImportKind::String(_) => {
-                // static strings don't have JS names, so we don't need to check for JS keywords
-            }
-            ast::ImportKind::Type(import_type) => {
-                if needs_check && is_js_keyword(&import_type.js_name) {
-                    bail_span!(
-                        import_type.rust_name,
-                        "Imported type cannot use the JS keyword `{}` as its name.",
-                        import_type.js_name
-                    );
+                ast::ImportKind::String(_) => {
+                    // static strings don't have JS names, so we don't need to check for JS keywords
+                }
+                ast::ImportKind::Type(import_type) => {
+                    if is_non_value_js_keyword(&import_type.js_name) {
+                        bail_span!(
+                            import_type.rust_name,
+                            "Imported type cannot use the JS keyword `{}` as its name.",
+                            import_type.js_name
+                        );
+                    }
+                }
+                ast::ImportKind::Enum(_) => {
+                    // string enums aren't possible here
                 }
-            }
-            ast::ImportKind::Enum(_) => {
-                // string enums aren't possible here
             }
         }
 

crates/macro/ui-tests/import-keyword.rs

@@ -33,7 +33,7 @@ extern "C" {
 // Namespaces
 
 // namespace on extern block
-#[wasm_bindgen(js_namespace = ["new", "foo"])]
+#[wasm_bindgen(js_namespace = ["public", "foo"])]
 extern "C" {}
 
 #[wasm_bindgen]
@@ -58,6 +58,8 @@ extern "C" {
 #[wasm_bindgen]
 pub struct class;
 #[wasm_bindgen]
+pub struct r#true; // forbid value-like keywords
+#[wasm_bindgen]
 pub enum switch {
     A,
     B,

crates/macro/ui-tests/import-keyword.stderr

@@ -22,11 +22,11 @@ error: Imported type cannot use the JS keyword `throw` as its name.
 27 |     type B;
    |          ^
 
-error: Namespace cannot start with the JS keyword `new`
+error: Namespace cannot start with the JS keyword `public`
   --> ui-tests/import-keyword.rs:36:32
    |
-36 | #[wasm_bindgen(js_namespace = ["new", "foo"])]
-   |                                ^^^^^
+36 | #[wasm_bindgen(js_namespace = ["public", "foo"])]
+   |                                ^^^^^^^^
 
 error: Namespace cannot start with the JS keyword `const`
   --> ui-tests/import-keyword.rs:42:36
@@ -46,8 +46,14 @@ error: struct cannot use the JS keyword `class` as its name
 59 | pub struct class;
    |            ^^^^^
 
+error: struct cannot use the JS keyword `true` as its name
+  --> ui-tests/import-keyword.rs:61:12
+   |
+61 | pub struct r#true; // forbid value-like keywords
+   |            ^^^^^^
+
 error: enum cannot use the JS keyword `switch` as its name
-  --> ui-tests/import-keyword.rs:61:10
+  --> ui-tests/import-keyword.rs:63:10
    |
-61 | pub enum switch {
+63 | pub enum switch {
    |          ^^^^^^