rustwasm / wasm-bindgen

Facilitating high-level interactions between Wasm modules and JavaScript
https://rustwasm.github.io/docs/wasm-bindgen/
Apache License 2.0
7.84k stars 1.08k forks source link

Generated JS code can end up with (reserved) keywords as identifiers #2806

Open y21 opened 2 years ago

y21 commented 2 years ago

It seems wasm-bindgen does not check that identifiers that are valid in Rust code might not be valid in JavaScript.

Building a project containing a function like this:

use wasm_bindgen::prelude::wasm_bindgen;

#[wasm_bindgen]
pub fn test(class: u8) {}

... generates this piece of JS code:

/**
* @param {number} class
*/
export function test(class) {
    wasm.test(class);
}

which is invalid (class is a keyword and can't be used for identifiers in JS)

Steps to Reproduce

  1. wasm-pack new bug
  2. Paste the code above in src/lib.rs
  3. wasm-pack build
  4. Check pkg/bug_bg.js (filename depends on the project name)

Expected Behavior

Prefix identifier with _ so it can't clash with keywords (or mangle identifiers in some other way)

Actual Behavior

The identifier is not changed and clashes with the JavaScript "class" keyword

petrstudynka commented 2 years ago

I am not sure whether generating idents with prefixes is the "right" approach. It breaks the Rust -> JS interface mapping contract.

I'd suggest issue compiler warnings once user uses idents that are keywords in js.

Nyrox commented 2 years ago

I guess I will tack on to this issue, but a slightly more insidious version of the same problem is that names in your rust code can also shadow the same names in the generated JS code.

I just ran into this with a function with the signature: pub fn process_frame(mut wasm: WasmEngine, time: u64) which then generated this piece of JS shim:

export function process_frame(wasm, time) {
    _assertClass(wasm, WasmEngine);
    if (wasm.ptr === 0) {
        throw new Error('Attempt to use a moved value');
    }
    var ptr0 = wasm.ptr;
    wasm.ptr = 0;
    uint64CvtShim[0] = time;
    const low1 = u32CvtShim[0];
    const high1 = u32CvtShim[1];
    wasm.process_frame(ptr0, low1, high1);
}

which in quite hilarious fashion breaks, because the parameter called 'wasm' shadows the fairly important import: import * as wasm from './[REDACTED]_wasm_bg.wasm'; at the top of the file. :p

Ideally I guess all generated JS code should prefix their names with something, but some warnings for names like 'wasm' would be a good idea. 😂

daxpedda commented 1 year ago

Duplicate of #2338.

EDIT: nope, that was wrong.