Skip to content

Commit

Permalink
fix: resource shadowing cross-reference case (#340)
Browse files Browse the repository at this point in the history
  • Loading branch information
guybedford authored Jan 15, 2024
1 parent e6325fa commit 1f85139
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 31 deletions.
26 changes: 7 additions & 19 deletions crates/js-component-bindgen/src/transpile_bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1636,28 +1636,16 @@ impl<'a> Instantiator<'a, '_> {
}
);
}
FunctionKind::Constructor(ty) => {
FunctionKind::Constructor(_) => {
if self.defined_resource_classes.contains(local_name) {
panic!("Internal error: Resource constructor must be defined before other methods and statics");
}
let ty = &self.resolve.types[ty];
let name = ty.name.as_ref().unwrap();
if name.to_upper_camel_case() == local_name {
uwrite!(
self.src.js,
"
class {local_name} {{
constructor"
);
} else {
uwrite!(
self.src.js,
"
const {local_name} = class {} {{
constructor",
name.to_upper_camel_case()
);
}
uwrite!(
self.src.js,
"
class {local_name} {{
constructor"
);
self.defined_resource_classes.insert(local_name.to_string());
}
}
Expand Down
17 changes: 11 additions & 6 deletions crates/wasm-tools-component/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::VecDeque;
use std::fs::metadata;
use std::path::PathBuf;
use wasm_encoder::{Encode, Section};
use wasm_metadata::Producers;
Expand Down Expand Up @@ -74,16 +75,20 @@ impl Guest for WasmToolsJs {

let mut resolve = Resolve::default();

let pkg = if let Some(wit_source) = &embed_opts.wit_source {
let id = if let Some(wit_source) = &embed_opts.wit_source {
let path = PathBuf::from("component.wit");
UnresolvedPackage::parse(&path, wit_source).map_err(|e| e.to_string())?
let pkg = UnresolvedPackage::parse(&path, wit_source).map_err(|e| e.to_string())?;
resolve.push(pkg).map_err(|e| e.to_string())?
} else {
let wit_path = &embed_opts.wit_path.as_ref().unwrap();
UnresolvedPackage::parse_file(&PathBuf::from(wit_path)).map_err(|e| e.to_string())?
let wit_path = &PathBuf::from(embed_opts.wit_path.as_ref().unwrap());
if metadata(wit_path).unwrap().is_file() {
let pkg = UnresolvedPackage::parse_file(wit_path).map_err(|e| e.to_string())?;
resolve.push(pkg).map_err(|e| e.to_string())?
} else {
resolve.push_dir(wit_path).map_err(|e| e.to_string())?.0
}
};

let id = resolve.push(pkg).map_err(|e| e.to_string())?;

let world_string = embed_opts.world.as_ref().map(|world| world.to_string());
let world = resolve
.select_world(id, world_string.as_deref())
Expand Down
4 changes: 4 additions & 0 deletions packages/preview2-shim/lib/nodejs/filesystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,10 @@ function convertFsError(e) {
return "unsupported";
case "ENOTTY":
return "no-tty";
// windows gives this error for badly structured `//` reads
// this seems like a slightly better error than unknown given
// that it's a common footgun
case -4094:
case "ENXIO":
return "no-such-device";
case "EOVERFLOW":
Expand Down
23 changes: 19 additions & 4 deletions src/cmd/transpile.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,34 @@
import { $init, generate } from '../../obj/js-component-bindgen-component.js';
import { writeFile } from 'fs/promises';
import { mkdir } from 'fs/promises';
import { dirname, extname, basename } from 'path';
import { writeFile } from 'node:fs/promises';
import { mkdir } from 'node:fs/promises';
import { dirname, extname, basename, resolve } from 'node:path';
import c from 'chalk-template';
import { readFile, sizeStr, table, spawnIOTmp, setShowSpinner, getShowSpinner } from '../common.js';
import { optimizeComponent } from './opt.js';
import { minify } from 'terser';
import { fileURLToPath } from 'url';
import { $init as wasmToolsInit, tools } from "../../obj/wasm-tools.js";
const { componentEmbed, componentNew } = tools;
import ora from '#ora';
import { platform } from 'node:process';

const isWindows = platform === 'win32';

export async function transpile (componentPath, opts, program) {
const varIdx = program?.parent.rawArgs.indexOf('--');
if (varIdx !== undefined && varIdx !== -1)
opts.optArgs = program.parent.rawArgs.slice(varIdx + 1);
const component = await readFile(componentPath);

let component;
if (!opts.stub) {
component = await readFile(componentPath);
} else {
await wasmToolsInit;
component = componentNew(componentEmbed({
dummy: true,
witPath: (isWindows ? '//?/' : '') + resolve(componentPath)
}), []);
}

if (!opts.quiet)
setShowSpinner(true);
Expand Down
1 change: 0 additions & 1 deletion src/cmd/wasm-tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { $init, tools } from "../../obj/wasm-tools.js";
const { print: printFn, parse: parseFn, componentWit: componentWitFn, componentNew: componentNewFn, componentEmbed: componentEmbedFn, metadataAdd: metadataAddFn, metadataShow: metadataShowFn } = tools;
import { resolve, basename, extname } from 'node:path';
import c from 'chalk-template';
import { platform } from 'node:process';

export async function parse(file, opts) {
await $init;
Expand Down
1 change: 1 addition & 0 deletions src/jco.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ program.command('transpile')
.option('--no-nodejs-compat', 'disables compatibility in Node.js without a fetch global')
.option('-M, --map <mappings...>', 'specifier=./output custom mappings for the component imports')
.option('--no-wasi-shim', 'disable automatic rewriting of WASI imports to use @bytecodealliance/preview2-shim')
.option('--stub', 'generate a stub implementation from a WIT file directly')
.option('--js', 'output JS instead of core WebAssembly')
.addOption(new Option('-I, --instantiation [mode]', 'output for custom module instantiation').choices(['async', 'sync']).preset('async'))
.option('-q, --quiet', 'disable logging')
Expand Down
7 changes: 7 additions & 0 deletions test/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ export async function cliTest (fixtures) {
}
});

test('Wit shadowing stub test', async () => {
const { stderr, stdout } = await exec(jcoPath, 'transpile', `test/fixtures/wit/deps/app/app.wit`, '-o', outDir, '--stub');
strictEqual(stderr, '');
const source = await readFile(`${outDir}/app.js`);
ok(source.includes('class PString$1{'));
});

test('Wit & New', async () => {
const { stderr, stdout } = await exec(jcoPath, 'wit', `test/fixtures/components/flavorful.component.wasm`);
strictEqual(stderr, '');
Expand Down
2 changes: 1 addition & 1 deletion test/codegen.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export async function codegenTest (fixtures) {

const bindingsSource = new TextDecoder().decode(files['resource-naming.js']);

ok(bindingsSource.includes('const Thing$1 = class Thing'));
ok(bindingsSource.includes('class Thing$1{'));
ok(bindingsSource.includes('Thing: Thing$1'));
});
});
Expand Down
34 changes: 34 additions & 0 deletions test/fixtures/wit/deps/app/app.wit
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package jcbhmr:hello-world-rust-wasm-component-lib;

interface cbh {
resource p-string {
call: func(a: string);
}
resource r-string {
call: func() -> string;
}
}
interface cb {
use cbh.{p-string as h-p-string, r-string as h-r-string};
resource p-string {
constructor(cb: h-p-string);
call: func(a: string);
}
resource r-string {
constructor(cb: h-r-string);
call: func() -> string;
}
}

interface hello-world {
use cb.{p-string, r-string};
set-cb: func(cb: p-string);
run-cb-with-result-of: func(cb: borrow<r-string>);
}

world hello-world-rust-wasm-component-lib {
import cbh;
export cb;

export hello-world;
}

0 comments on commit 1f85139

Please sign in to comment.