Skip to content

Commit

Permalink
clippy fixes (#513)
Browse files Browse the repository at this point in the history
Co-authored-by: Guy Bedford <[email protected]>
  • Loading branch information
calvinrp and guybedford authored Oct 28, 2024
1 parent e03e1b9 commit b175fa5
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 57 deletions.
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.vscode
*.swp
*.swo
node_modules
/target
/obj
Expand All @@ -10,4 +12,4 @@ node_modules
/src/**/*.d.ts.map
/tests/rundir
/tests/output
/tests/gen
/tests/gen
2 changes: 1 addition & 1 deletion crates/js-component-bindgen-component/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl Guest for JsComponentBindgenComponent {
// Add features if specified
match opts.features {
Some(EnabledFeatureSet::List(ref features)) => {
for f in features.into_iter() {
for f in features.iter() {
resolve.features.insert(f.to_string());
}
}
Expand Down
18 changes: 7 additions & 11 deletions crates/js-component-bindgen/src/esm_bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl EsmBindgen {
continue;
};
let (local_name, _) =
local_names.get_or_create(&format!("export:{export_name}"), export_name);
local_names.get_or_create(format!("export:{export_name}"), export_name);
uwriteln!(output, "const {local_name} = {{");
for (func_name, export) in iface {
let ExportBinding::Local(local_name) = export else {
Expand All @@ -181,7 +181,7 @@ impl EsmBindgen {
}
let local_name = match &self.exports[export_name] {
ExportBinding::Local(local_name) => local_name,
ExportBinding::Interface(_) => local_names.get(&format!("export:{}", export_name)),
ExportBinding::Interface(_) => local_names.get(format!("export:{}", export_name)),
};
let alias_maybe_quoted = maybe_quote_id(alias);
if local_name == alias_maybe_quoted {
Expand All @@ -201,7 +201,7 @@ impl EsmBindgen {
}
let local_name = match export {
ExportBinding::Local(local_name) => local_name,
ExportBinding::Interface(_) => local_names.get(&format!("export:{}", export_name)),
ExportBinding::Interface(_) => local_names.get(format!("export:{}", export_name)),
};
let export_name_maybe_quoted = maybe_quote_id(export_name);
if local_name == export_name_maybe_quoted {
Expand All @@ -220,7 +220,7 @@ impl EsmBindgen {
uwrite!(output, " }}");
}

fn contains_js_quote(&self, js_string: &String) -> bool {
fn contains_js_quote(&self, js_string: &str) -> bool {
js_string.contains("\"") || js_string.contains("'") || js_string.contains("`")
}

Expand All @@ -239,11 +239,7 @@ impl EsmBindgen {
} else {
&specifier[iface_idx..]
};
Some(if iface_name.starts_with("global-") {
&iface_name[7..]
} else {
""
})
Some(iface_name.strip_prefix("global-").unwrap_or_default())
} else {
None
};
Expand Down Expand Up @@ -289,7 +285,7 @@ impl EsmBindgen {
output.push_str(", ");
}
let (iface_local_name, _) = local_names.get_or_create(
&format!("import:{specifier}#{external_name}"),
format!("import:{specifier}#{external_name}"),
external_name,
);
iface_imports.push((iface_local_name.to_string(), iface));
Expand Down Expand Up @@ -331,7 +327,7 @@ impl EsmBindgen {
);
} else if let Some(idl_binding) = idl_binding {
uwrite!(output, "}} = {}()", Intrinsic::GlobalThisIdlProxy.name());
if idl_binding != "" {
if !idl_binding.is_empty() {
for segment in idl_binding.split('-') {
uwrite!(output, ".{}()", segment.to_lowercase());
}
Expand Down
4 changes: 2 additions & 2 deletions crates/js-component-bindgen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ pub fn transpile(component: &[u8], opts: TranspileOpts) -> Result<Transpiled, an
.map(|(_i, module)| core::Translation::new(module, opts.multi_memory))
.collect::<Result<_>>()?;

let mut wasmtime_component = Component::default();
let types = types.finish(&mut wasmtime_component);
let wasmtime_component = Component::default();
let types = types.finish(&wasmtime_component);

// Insert all core wasm modules into the generated `Files` which will
// end up getting used in the `generate_instantiate` method.
Expand Down
17 changes: 6 additions & 11 deletions crates/js-component-bindgen/src/names.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use heck::ToLowerCamelCase;
use std::collections::hash_map::RandomState;
use std::collections::{HashMap, HashSet};
use std::hash::{BuildHasher, Hash, Hasher};
use std::hash::{BuildHasher, Hash};

#[derive(Default)]
pub struct LocalNames {
Expand Down Expand Up @@ -43,19 +43,15 @@ impl<'a> LocalNames {
}

pub fn get<H: Hash>(&'a self, unique_id: H) -> &'a str {
let mut new_s = self.random_state.build_hasher();
unique_id.hash(&mut new_s);
let hash = new_s.finish();
let hash = self.random_state.hash_one(&unique_id);
if !self.local_name_ids.contains_key(&hash) {
panic!("Internal error, no name defined in local names map");
}
&self.local_name_ids[&hash]
}

pub fn try_get<H: Hash>(&'a self, unique_id: H) -> Option<&'a str> {
let mut new_s = self.random_state.build_hasher();
unique_id.hash(&mut new_s);
let hash = new_s.finish();
let hash = self.random_state.hash_one(&unique_id);
if !self.local_name_ids.contains_key(&hash) {
return None;
}
Expand All @@ -64,10 +60,9 @@ impl<'a> LocalNames {

/// get or create a unique identifier for a string while storing the lookup by unique id
pub fn get_or_create<H: Hash>(&'a mut self, unique_id: H, goal_name: &str) -> (&'a str, bool) {
let mut new_s = self.random_state.build_hasher();
unique_id.hash(&mut new_s);
let hash = new_s.finish();
let hash = self.random_state.hash_one(&unique_id);
let mut seen = true;
#[allow(clippy::map_entry)]
if !self.local_name_ids.contains_key(&hash) {
let goal = self.create_once(goal_name).to_string();
self.local_name_ids.insert(hash, goal);
Expand Down Expand Up @@ -123,7 +118,7 @@ pub fn is_js_identifier(s: &str) -> bool {
return false;
}
}
!is_js_reserved_word(&s)
!is_js_reserved_word(s)
}

pub fn is_js_reserved_word(s: &str) -> bool {
Expand Down
36 changes: 19 additions & 17 deletions crates/js-component-bindgen/src/transpile_bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ struct JsBindgen<'a> {
all_intrinsics: BTreeSet<Intrinsic>,
}

#[allow(clippy::too_many_arguments)]
pub fn transpile_bindgen(
name: &str,
component: &ComponentTranslation,
Expand Down Expand Up @@ -336,7 +337,7 @@ impl<'a> JsBindgen<'a> {
);
} else {
let (maybe_init_export, maybe_init) =
if self.opts.tla_compat && matches!(opts.instantiation, None) {
if self.opts.tla_compat && opts.instantiation.is_none() {
uwriteln!(self.src.js_init, "_initialized = true;");
(
"\
Expand Down Expand Up @@ -1104,7 +1105,7 @@ impl<'a> Instantiator<'a, '_> {
self.gen
.local_names
.get_or_create(
&format!(
format!(
"import:{}-{}-{}",
import_specifier,
maybe_iface_member.as_deref().unwrap_or(""),
Expand All @@ -1124,7 +1125,7 @@ impl<'a> Instantiator<'a, '_> {
format!(
"{}.{}",
Instantiator::resource_name(
&self.resolve,
self.resolve,
&mut self.gen.local_names,
resource_id,
&self.imports_resource_types
Expand All @@ -1137,7 +1138,7 @@ impl<'a> Instantiator<'a, '_> {
format!(
"new {}",
Instantiator::resource_name(
&self.resolve,
self.resolve,
&mut self.gen.local_names,
resource_id,
&self.imports_resource_types
Expand Down Expand Up @@ -1198,7 +1199,7 @@ impl<'a> Instantiator<'a, '_> {
FunctionKind::Method(resource_id) => format!(
"{}.prototype.{callee_name}",
Instantiator::resource_name(
&self.resolve,
self.resolve,
&mut self.gen.local_names,
resource_id,
&self.imports_resource_types
Expand All @@ -1220,14 +1221,14 @@ impl<'a> Instantiator<'a, '_> {
resource_tables.push(*tid);
}

if resource_tables.len() == 0 {
if resource_tables.is_empty() {
"".to_string()
} else {
format!(
" resourceTables: [{}],",
resource_tables
.iter()
.map(|x| format!("handleTable{}", x.as_u32().to_string()))
.map(|x| format!("handleTable{}", x.as_u32()))
.collect::<Vec<String>>()
.join(", ")
)
Expand Down Expand Up @@ -1268,7 +1269,7 @@ impl<'a> Instantiator<'a, '_> {
(
ty.name.as_ref().unwrap().to_upper_camel_case(),
Instantiator::resource_name(
&self.resolve,
self.resolve,
&mut self.gen.local_names,
tid,
&self.imports_resource_types,
Expand Down Expand Up @@ -1508,6 +1509,7 @@ impl<'a> Instantiator<'a, '_> {
}
}

#[allow(clippy::too_many_arguments)]
fn bindgen(
&mut self,
nparams: usize,
Expand Down Expand Up @@ -1566,7 +1568,7 @@ impl<'a> Instantiator<'a, '_> {

if self.gen.opts.tla_compat
&& matches!(abi, AbiVariant::GuestExport)
&& matches!(self.gen.opts.instantiation, None)
&& self.gen.opts.instantiation.is_none()
{
let throw_uninitialized = self.gen.intrinsic(Intrinsic::ThrowUninitialized);
uwrite!(
Expand All @@ -1578,7 +1580,7 @@ impl<'a> Instantiator<'a, '_> {
}

let mut f = FunctionBindgen {
resource_map: &resource_map,
resource_map,
cur_resource_borrows: false,
intrinsics: &mut self.gen.all_intrinsics,
valid_lifting_optimization: self.gen.opts.valid_lifting_optimization,
Expand Down Expand Up @@ -1776,7 +1778,7 @@ impl<'a> Instantiator<'a, '_> {
| FunctionKind::Static(resource_id) = func.kind
{
Instantiator::resource_name(
&self.resolve,
self.resolve,
&mut self.gen.local_names,
resource_id,
&self.exports_resource_types,
Expand All @@ -1787,8 +1789,8 @@ impl<'a> Instantiator<'a, '_> {
.to_string();
self.export_bindgen(
&local_name,
&def,
&options,
def,
options,
func,
export_name,
&resource_map,
Expand Down Expand Up @@ -1833,20 +1835,20 @@ impl<'a> Instantiator<'a, '_> {
| FunctionKind::Static(resource_id) = func.kind
{
Instantiator::resource_name(
&self.resolve,
self.resolve,
&mut self.gen.local_names,
resource_id,
&self.exports_resource_types,
)
} else {
self.gen.local_names.create_once(&func_name)
self.gen.local_names.create_once(func_name)
}
.to_string();

self.export_bindgen(
&local_name,
&def,
&options,
def,
options,
func,
export_name,
&resource_map,
Expand Down
23 changes: 10 additions & 13 deletions crates/js-component-bindgen/src/ts_bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,9 @@ pub fn ts_bindgen(
WorldItem::Interface { id, stability } => {
let iface_name = &resolve.interfaces[*id]
.name
.as_ref()
.map(String::as_str)
.as_deref()
.unwrap_or("<unnamed>");
if !feature_gate_allowed(resolve, package, &stability, iface_name)
if !feature_gate_allowed(resolve, package, stability, iface_name)
.context("failed to check feature gate for imported interface")?
{
let import_specifier = resolve.id_of(*id).unwrap();
Expand All @@ -107,7 +106,7 @@ pub fn ts_bindgen(
match name {
WorldKey::Name(name) => {
// kebab name -> direct ns namespace import
bindgen.import_interface(resolve, &name, *id, files);
bindgen.import_interface(resolve, name, *id, files);
}
// namespaced ns:pkg/iface
// TODO: map support
Expand Down Expand Up @@ -206,7 +205,7 @@ pub fn ts_bindgen(
}
};

if !feature_gate_allowed(resolve, package, &stability, iface_name)
if !feature_gate_allowed(resolve, package, stability, iface_name)
.context("failed to check feature gate for export")?
{
debug!("skipping exported interface [{export_name}] feature gate due to feature gate visibility");
Expand Down Expand Up @@ -273,7 +272,7 @@ pub fn ts_bindgen(
bindgen.src.push_str(&bindgen.export_object);
}

if opts.tla_compat && matches!(opts.instantiation, None) {
if opts.tla_compat && opts.instantiation.is_none() {
uwriteln!(
bindgen.src,
"
Expand Down Expand Up @@ -496,7 +495,7 @@ impl TsBindgen {
if !local_exists {
// TypeScript doesn't work with empty namespaces, so we don't import in this case,
// just define them as empty.
let is_empty_interface = resolve.interfaces[id].functions.len() == 0
let is_empty_interface = resolve.interfaces[id].functions.is_empty()
&& resolve.interfaces[id]
.types
.iter()
Expand Down Expand Up @@ -786,12 +785,10 @@ impl<'a> TsInterface<'a> {
}
FunctionKind::Constructor(_) => iface.src.push_str("constructor"),
}
} else if is_js_identifier(&out_name) {
iface.src.push_str(&out_name);
} else {
if is_js_identifier(&out_name) {
iface.src.push_str(&out_name);
} else {
iface.src.push_str(&format!("'{out_name}'"));
}
iface.src.push_str(&format!("'{out_name}'"));
}

let end_character = if declaration { ';' } else { ',' };
Expand Down Expand Up @@ -1036,7 +1033,7 @@ impl<'a> TsInterface<'a> {
let type_name = name.to_upper_camel_case();
match owner_not_parent {
Some(owned_interface_name) => {
let orig_id = dealias(&self.resolve, id);
let orig_id = dealias(self.resolve, id);
let orig_name = self.resolve.types[orig_id]
.name
.as_ref()
Expand Down
2 changes: 1 addition & 1 deletion crates/wasm-tools-component/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl Guest for WasmToolsJs {
// (this helps identify/use feature gating properly)
match embed_opts.features {
Some(EnabledFeatureSet::List(ref features)) => {
for f in features.into_iter() {
for f in features.iter() {
resolve.features.insert(f.to_string());
}
}
Expand Down

0 comments on commit b175fa5

Please sign in to comment.