From 52c8a0af44a83b065bf30bfc78dc40f300e4abac Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Tue, 25 Oct 2022 14:41:49 +0200 Subject: [PATCH] CI: enable `clippy` for workspace tests (#539) * add CI pass to run clippy on the workspace's tests * apply clippy suggestions (tests) * apply clippy (nightly) suggestions * apply more clippy (+nightly) suggestions --- .github/workflows/rust.yml | 5 +++++ crates/arena/src/tests.rs | 2 +- crates/core/src/units.rs | 15 +++++++++------ crates/wasmi/src/engine/func_builder/error.rs | 4 ++-- .../src/engine/func_builder/locals_registry.rs | 4 ++-- crates/wasmi/src/engine/func_builder/mod.rs | 2 +- crates/wasmi/src/engine/func_types.rs | 2 +- crates/wasmi/src/engine/tests.rs | 10 +++------- crates/wasmi/src/func_type.rs | 18 +++++++++--------- crates/wasmi/src/lib.rs | 1 - crates/wasmi/src/linker.rs | 6 +++--- crates/wasmi/src/module/error.rs | 8 ++------ crates/wasmi/src/module/import.rs | 4 ++-- crates/wasmi/src/module/instantiate/error.rs | 11 ++++------- crates/wasmi/src/module/mod.rs | 8 ++++---- crates/wasmi/src/store.rs | 15 ++++++++------- crates/wasmi/tests/e2e/v1/func.rs | 2 +- crates/wasmi/tests/spec/context.rs | 15 ++++++--------- crates/wasmi/tests/spec/descriptor.rs | 9 ++++----- crates/wasmi/tests/spec/error.rs | 11 +++-------- crates/wasmi/tests/spec/run.rs | 9 ++++----- 21 files changed, 74 insertions(+), 87 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 8ece73217b..e62c7eb9cd 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -172,6 +172,11 @@ jobs: with: command: clippy args: --workspace --no-default-features -- -D warnings + - name: Clippy (tests) + uses: actions-rs/cargo@v1 + with: + command: clippy + args: --workspace --tests -- -D warnings coverage: name: Coverage diff --git a/crates/arena/src/tests.rs b/crates/arena/src/tests.rs index 727113458c..c7fb416d62 100644 --- a/crates/arena/src/tests.rs +++ b/crates/arena/src/tests.rs @@ -10,7 +10,7 @@ impl Index for usize { } } -const TEST_ENTITIES: &[&'static str] = &["a", "b", "c", "d"]; +const TEST_ENTITIES: &[&str] = &["a", "b", "c", "d"]; mod arena { use super::*; diff --git a/crates/core/src/units.rs b/crates/core/src/units.rs index 26755f6d2a..7604f83842 100644 --- a/crates/core/src/units.rs +++ b/crates/core/src/units.rs @@ -188,7 +188,7 @@ mod tests { #[test] fn pages_max() { - assert_eq!(Pages::max(), pages(u16::MAX as u32 + 1)); + assert_eq!(Pages::max(), pages(u32::from(u16::MAX) + 1)); } #[test] @@ -196,9 +196,12 @@ mod tests { assert_eq!(Pages::new(0), Some(Pages(0))); assert_eq!(Pages::new(1), Some(Pages(1))); assert_eq!(Pages::new(1000), Some(Pages(1000))); - assert_eq!(Pages::new(u16::MAX as u32), Some(Pages(u16::MAX as u32))); - assert_eq!(Pages::new(u16::MAX as u32 + 1), Some(Pages::max())); - assert_eq!(Pages::new(u16::MAX as u32 + 2), None); + assert_eq!( + Pages::new(u32::from(u16::MAX)), + Some(Pages(u32::from(u16::MAX))) + ); + assert_eq!(Pages::new(u32::from(u16::MAX) + 1), Some(Pages::max())); + assert_eq!(Pages::new(u32::from(u16::MAX) + 2), None); assert_eq!(Pages::new(u32::MAX), None); } @@ -273,8 +276,8 @@ mod tests { Some(bytes(n * bytes_per_page)) ); } - assert!(Bytes::new64(Pages(u16::MAX as u32 + 1)).is_some()); - assert!(Bytes::new64(Pages(u16::MAX as u32 + 2)).is_none()); + assert!(Bytes::new64(Pages(u32::from(u16::MAX) + 1)).is_some()); + assert!(Bytes::new64(Pages(u32::from(u16::MAX) + 2)).is_none()); assert!(Bytes::new64(Pages::max()).is_some()); } } diff --git a/crates/wasmi/src/engine/func_builder/error.rs b/crates/wasmi/src/engine/func_builder/error.rs index 8ecc1460fc..6a93010337 100644 --- a/crates/wasmi/src/engine/func_builder/error.rs +++ b/crates/wasmi/src/engine/func_builder/error.rs @@ -46,10 +46,10 @@ impl Display for TranslationError { match &*self.inner { TranslationErrorInner::Validate(error) => error.fmt(f), TranslationErrorInner::UnsupportedBlockType(error) => { - write!(f, "encountered unsupported Wasm block type: {:?}", error) + write!(f, "encountered unsupported Wasm block type: {error:?}") } TranslationErrorInner::UnsupportedValueType(error) => { - write!(f, "encountered unsupported Wasm value type: {:?}", error) + write!(f, "encountered unsupported Wasm value type: {error:?}") } TranslationErrorInner::DropKeep(error) => error.fmt(f), } diff --git a/crates/wasmi/src/engine/func_builder/locals_registry.rs b/crates/wasmi/src/engine/func_builder/locals_registry.rs index c1c482c4af..c6fb44696a 100644 --- a/crates/wasmi/src/engine/func_builder/locals_registry.rs +++ b/crates/wasmi/src/engine/func_builder/locals_registry.rs @@ -46,8 +46,8 @@ impl LocalsRegistry { } self.len_registered = self.len_registered.checked_add(amount).unwrap_or_else(|| { panic!( - "tried to register too many local variables for function: got {}, additional {}", - self.len_registered, amount + "tried to register too many local variables for function: got {}, additional {amount}", + self.len_registered ) }); } diff --git a/crates/wasmi/src/engine/func_builder/mod.rs b/crates/wasmi/src/engine/func_builder/mod.rs index 86bb82ab20..7758b95955 100644 --- a/crates/wasmi/src/engine/func_builder/mod.rs +++ b/crates/wasmi/src/engine/func_builder/mod.rs @@ -313,7 +313,7 @@ impl<'parser> FuncBuilder<'parser> { stack_height .checked_add(len_params_locals) .and_then(|x| x.checked_sub(local_idx as usize)) - .unwrap_or_else(|| panic!("cannot convert local index into local depth: {}", local_idx)) + .unwrap_or_else(|| panic!("cannot convert local index into local depth: {local_idx}")) } /// Returns the target at the given `depth` together with its [`DropKeep`]. diff --git a/crates/wasmi/src/engine/func_types.rs b/crates/wasmi/src/engine/func_types.rs index 9ebe2b3cb4..1256c18f93 100644 --- a/crates/wasmi/src/engine/func_types.rs +++ b/crates/wasmi/src/engine/func_types.rs @@ -131,6 +131,6 @@ impl FuncTypeRegistry { let entity_index = self.unwrap_index(func_type.into_inner()); self.func_types .get(entity_index) - .unwrap_or_else(|| panic!("failed to resolve stored function type: {:?}", entity_index)) + .unwrap_or_else(|| panic!("failed to resolve stored function type: {entity_index:?}")) } } diff --git a/crates/wasmi/src/engine/tests.rs b/crates/wasmi/src/engine/tests.rs index 4505d79d11..143c016456 100644 --- a/crates/wasmi/src/engine/tests.rs +++ b/crates/wasmi/src/engine/tests.rs @@ -48,7 +48,7 @@ fn assert_func_body( ( index, engine.resolve_inst(func_body, index).unwrap_or_else(|| { - panic!("encountered missing instruction at position {}", index) + panic!("encountered missing instruction at position {index}") }), expected, ) @@ -57,16 +57,12 @@ fn assert_func_body( assert_eq!( actual, expected, - "encountered instruction mismatch for {} at position {}", + "encountered instruction mismatch for {} at position {index}", engine.resolve_func_type(func_type, Clone::clone), - index ); } if let Some(unexpected) = engine.resolve_inst(func_body, len_expected) { - panic!( - "encountered unexpected instruction at position {}: {:?}", - len_expected, unexpected, - ); + panic!("encountered unexpected instruction at position {len_expected}: {unexpected:?}",); } } diff --git a/crates/wasmi/src/func_type.rs b/crates/wasmi/src/func_type.rs index e87c784514..fe0d7a6db4 100644 --- a/crates/wasmi/src/func_type.rs +++ b/crates/wasmi/src/func_type.rs @@ -24,9 +24,9 @@ impl Display for FuncType { write!(f, "fn(")?; let (params, results) = self.params_results(); if let Some((first, rest)) = params.split_first() { - write!(f, "{}", first)?; + write!(f, "{first}")?; for param in rest { - write!(f, ", {}", param)?; + write!(f, ", {param}")?; } } write!(f, ")")?; @@ -35,9 +35,9 @@ impl Display for FuncType { if !rest.is_empty() { write!(f, "(")?; } - write!(f, "{}", first)?; + write!(f, "{first}")?; for result in rest { - write!(f, ", {}", result)?; + write!(f, ", {result}")?; } if !rest.is_empty() { write!(f, ")")?; @@ -90,13 +90,13 @@ mod tests { #[test] fn display_0in_0out() { let func_type = FuncType::new([], []); - assert_eq!(format!("{}", func_type), String::from("fn()"),); + assert_eq!(format!("{func_type}"), String::from("fn()"),); } #[test] fn display_1in_1out() { let func_type = FuncType::new([ValueType::I32], [ValueType::I32]); - assert_eq!(format!("{}", func_type), String::from("fn(i32) -> i32"),); + assert_eq!(format!("{func_type}"), String::from("fn(i32) -> i32"),); } #[test] @@ -111,7 +111,7 @@ mod tests { [], ); assert_eq!( - format!("{}", func_type), + format!("{func_type}"), String::from("fn(i32, i64, f32, f64)"), ); } @@ -128,7 +128,7 @@ mod tests { ], ); assert_eq!( - format!("{}", func_type), + format!("{func_type}"), String::from("fn() -> (i32, i64, f32, f64)"), ); } @@ -150,7 +150,7 @@ mod tests { ], ); assert_eq!( - format!("{}", func_type), + format!("{func_type}"), String::from("fn(i32, i64, f32, f64) -> (i32, i64, f32, f64)"), ); } diff --git a/crates/wasmi/src/lib.rs b/crates/wasmi/src/lib.rs index 144d0903af..81ea76ac54 100644 --- a/crates/wasmi/src/lib.rs +++ b/crates/wasmi/src/lib.rs @@ -78,7 +78,6 @@ clippy::type_repetition_in_bounds, clippy::inconsistent_struct_constructor, clippy::default_trait_access, - clippy::map_unwrap_or, clippy::items_after_statements )] #![recursion_limit = "550"] diff --git a/crates/wasmi/src/linker.rs b/crates/wasmi/src/linker.rs index 2b551154ce..ea09f69cb8 100644 --- a/crates/wasmi/src/linker.rs +++ b/crates/wasmi/src/linker.rs @@ -315,9 +315,9 @@ impl Linker { fn insert(&mut self, key: ImportKey, item: Extern) -> Result<(), LinkerError> { match self.definitions.entry(key) { Entry::Occupied(_) => { - let (module_name, field_name) = self.resolve_import_key(key).unwrap_or_else(|| { - panic!("encountered missing import names for key {:?}", key) - }); + let (module_name, field_name) = self + .resolve_import_key(key) + .unwrap_or_else(|| panic!("encountered missing import names for key {key:?}")); let import_name = ImportName::new(module_name, field_name); return Err(LinkerError::DuplicateDefinition { import_name, diff --git a/crates/wasmi/src/module/error.rs b/crates/wasmi/src/module/error.rs index 6862be0891..3beb453dc2 100644 --- a/crates/wasmi/src/module/error.rs +++ b/crates/wasmi/src/module/error.rs @@ -22,7 +22,7 @@ pub enum ModuleError { impl ModuleError { pub(crate) fn unsupported(definition: impl Debug) -> Self { Self::Unsupported { - message: format!("{:?}", definition).into(), + message: format!("{definition:?}").into(), } } } @@ -34,11 +34,7 @@ impl Display for ModuleError { ModuleError::Parser(error) => Display::fmt(error, f), ModuleError::Translation(error) => Display::fmt(error, f), ModuleError::Unsupported { message } => { - write!( - f, - "encountered unsupported Wasm proposal item: {:?}", - message - ) + write!(f, "encountered unsupported Wasm proposal item: {message:?}",) } } } diff --git a/crates/wasmi/src/module/import.rs b/crates/wasmi/src/module/import.rs index 8e4f319575..ee33bee0b8 100644 --- a/crates/wasmi/src/module/import.rs +++ b/crates/wasmi/src/module/import.rs @@ -31,9 +31,9 @@ impl Display for ImportName { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let module_name = &*self.module; if let Some(field_name) = self.field.as_deref() { - write!(f, "{}::{}", module_name, field_name) + write!(f, "{module_name}::{field_name}") } else { - write!(f, "{}", module_name) + write!(f, "{module_name}") } } } diff --git a/crates/wasmi/src/module/instantiate/error.rs b/crates/wasmi/src/module/instantiate/error.rs index 17db9b9b55..a17bd61e7f 100644 --- a/crates/wasmi/src/module/instantiate/error.rs +++ b/crates/wasmi/src/module/instantiate/error.rs @@ -63,14 +63,12 @@ impl Display for InstantiationError { ), Self::ImportsExternalsMismatch { expected, actual } => write!( f, - "expected {:?} external for import but found {:?}", - expected, actual + "expected {expected:?} external for import but found {actual:?}", ), Self::SignatureMismatch { expected, actual } => { write!( f, - "expected {:?} function signature but found {:?}", - expected, actual + "expected {expected:?} function signature but found {actual:?}", ) } Self::ElementSegmentDoesNotFit { @@ -79,11 +77,10 @@ impl Display for InstantiationError { amount, } => write!( f, - "table {:?} does not fit {} elements starting from offset {}", - table, offset, amount, + "table {table:?} does not fit {offset} elements starting from offset {amount}", ), Self::FoundStartFn { index } => { - write!(f, "found an unexpected start function with index {}", index) + write!(f, "found an unexpected start function with index {index}") } Self::Table(error) => Display::fmt(error, f), Self::Memory(error) => Display::fmt(error, f), diff --git a/crates/wasmi/src/module/mod.rs b/crates/wasmi/src/module/mod.rs index dc31a2fc71..97bf2b00d2 100644 --- a/crates/wasmi/src/module/mod.rs +++ b/crates/wasmi/src/module/mod.rs @@ -248,25 +248,25 @@ impl<'a> Iterator for ModuleImportsIter<'a> { Some(imported) => match imported { Imported::Func(name) => { let func_type = self.funcs.next().unwrap_or_else(|| { - panic!("unexpected missing imported function for {:?}", name) + panic!("unexpected missing imported function for {name:?}") }); ModuleImport::new(name, *func_type) } Imported::Table(name) => { let table_type = self.tables.next().unwrap_or_else(|| { - panic!("unexpected missing imported table for {:?}", name) + panic!("unexpected missing imported table for {name:?}") }); ModuleImport::new(name, *table_type) } Imported::Memory(name) => { let memory_type = self.memories.next().unwrap_or_else(|| { - panic!("unexpected missing imported linear memory for {:?}", name) + panic!("unexpected missing imported linear memory for {name:?}") }); ModuleImport::new(name, *memory_type) } Imported::Global(name) => { let global_type = self.globals.next().unwrap_or_else(|| { - panic!("unexpected missing imported global variable for {:?}", name) + panic!("unexpected missing imported global variable for {name:?}") }); ModuleImport::new(name, *global_type) } diff --git a/crates/wasmi/src/store.rs b/crates/wasmi/src/store.rs index 797684c3a5..d086b58fc7 100644 --- a/crates/wasmi/src/store.rs +++ b/crates/wasmi/src/store.rs @@ -256,7 +256,7 @@ impl Store { let entity_index = self.unwrap_index(table.into_inner()); self.tables .get(entity_index) - .unwrap_or_else(|| panic!("failed to resolve stored table: {:?}", entity_index)) + .unwrap_or_else(|| panic!("failed to resolve stored table: {entity_index:?}")) } /// Returns an exclusive reference to the associated entity of the table. @@ -269,7 +269,7 @@ impl Store { let entity_index = self.unwrap_index(table.into_inner()); self.tables .get_mut(entity_index) - .unwrap_or_else(|| panic!("failed to resolve stored table: {:?}", entity_index)) + .unwrap_or_else(|| panic!("failed to resolve stored table: {entity_index:?}")) } /// Returns a shared reference to the associated entity of the linear memory. @@ -282,7 +282,7 @@ impl Store { let entity_index = self.unwrap_index(memory.into_inner()); self.memories .get(entity_index) - .unwrap_or_else(|| panic!("failed to resolve stored linear memory: {:?}", entity_index)) + .unwrap_or_else(|| panic!("failed to resolve stored linear memory: {entity_index:?}")) } /// Returns an exclusive reference to the associated entity of the linear memory. @@ -295,7 +295,7 @@ impl Store { let entity_index = self.unwrap_index(memory.into_inner()); self.memories .get_mut(entity_index) - .unwrap_or_else(|| panic!("failed to resolve stored linear memory: {:?}", entity_index)) + .unwrap_or_else(|| panic!("failed to resolve stored linear memory: {entity_index:?}")) } /// Returns an exclusive reference to the associated entity of the linear memory and an @@ -310,9 +310,10 @@ impl Store { memory: Memory, ) -> (&mut MemoryEntity, &mut T) { let entity_index = self.unwrap_index(memory.into_inner()); - let memory_entity = self.memories.get_mut(entity_index).unwrap_or_else(|| { - panic!("failed to resolve stored linear memory: {:?}", entity_index) - }); + let memory_entity = self + .memories + .get_mut(entity_index) + .unwrap_or_else(|| panic!("failed to resolve stored linear memory: {entity_index:?}")); (memory_entity, &mut self.user_state) } diff --git a/crates/wasmi/tests/e2e/v1/func.rs b/crates/wasmi/tests/e2e/v1/func.rs index baf1a19561..5336684991 100644 --- a/crates/wasmi/tests/e2e/v1/func.rs +++ b/crates/wasmi/tests/e2e/v1/func.rs @@ -199,7 +199,7 @@ fn static_many_params_works() { fn setup_many_results() -> (Store<()>, Func) { let mut store = test_setup(); // Function taking 16 arguments (maximum) and doing nothing. - let func = Func::wrap(&mut store, || ascending_tuple()); + let func = Func::wrap(&mut store, ascending_tuple); (store, func) } diff --git a/crates/wasmi/tests/spec/context.rs b/crates/wasmi/tests/spec/context.rs index dcbc518e8a..d3bd5bc67c 100644 --- a/crates/wasmi/tests/spec/context.rs +++ b/crates/wasmi/tests/spec/context.rs @@ -60,19 +60,19 @@ impl<'a> TestContext<'a> { println!("print"); }); let print_i32 = Func::wrap(&mut store, |value: i32| { - println!("print: {}", value); + println!("print: {value}"); }); let print_f32 = Func::wrap(&mut store, |value: F32| { - println!("print: {:?}", value); + println!("print: {value:?}"); }); let print_f64 = Func::wrap(&mut store, |value: F64| { - println!("print: {:?}", value); + println!("print: {value:?}"); }); let print_i32_f32 = Func::wrap(&mut store, |v0: i32, v1: F32| { - println!("print: {:?} {:?}", v0, v1); + println!("print: {v0:?} {v1:?}"); }); let print_f64_f64 = Func::wrap(&mut store, |v0: F64, v1: F64| { - println!("print: {:?} {:?}", v0, v1); + println!("print: {v0:?} {v1:?}"); }); linker.define("spectest", "memory", default_memory).unwrap(); linker.define("spectest", "table", default_table).unwrap(); @@ -176,10 +176,7 @@ impl TestContext<'_> { /// If there have been no Wasm module instances registered so far. pub fn instance_by_name_or_last(&self, name: Option<&str>) -> Result { name.map(|name| self.instance_by_name(name)) - .unwrap_or_else(|| { - self.last_instance - .ok_or_else(|| TestError::NoModuleInstancesFound) - }) + .unwrap_or_else(|| self.last_instance.ok_or(TestError::NoModuleInstancesFound)) } /// Registers the given [`Instance`] with the given `name` and sets it as the last instance. diff --git a/crates/wasmi/tests/spec/descriptor.rs b/crates/wasmi/tests/spec/descriptor.rs index 9fa762f69a..64b8614969 100644 --- a/crates/wasmi/tests/spec/descriptor.rs +++ b/crates/wasmi/tests/spec/descriptor.rs @@ -20,10 +20,9 @@ impl TestDescriptor { /// /// If the corresponding Wasm test spec file cannot properly be read. pub fn new(name: &str) -> Self { - let path = format!("tests/spec/{}.wast", name); - let file = fs::read_to_string(&path).unwrap_or_else(|error| { - panic!("{}, failed to read `.wast` test file: {}", path, error) - }); + let path = format!("tests/spec/{name}.wast"); + let file = fs::read_to_string(&path) + .unwrap_or_else(|error| panic!("{path}, failed to read `.wast` test file: {error}")); Self { path, file } } @@ -61,6 +60,6 @@ pub struct TestSpan<'a> { impl Display for TestSpan<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let (line, col) = self.span.linecol_in(self.contents); - write!(f, "{}:{}:{}", self.path, line, col) + write!(f, "{}:{line}:{col}", self.path) } } diff --git a/crates/wasmi/tests/spec/error.rs b/crates/wasmi/tests/spec/error.rs index 1d03850b03..1b76d7bbc4 100644 --- a/crates/wasmi/tests/spec/error.rs +++ b/crates/wasmi/tests/spec/error.rs @@ -25,7 +25,7 @@ impl Display for TestError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::InstanceNotRegistered { name } => { - write!(f, "missing module instance with name: {}", name) + write!(f, "missing module instance with name: {name}") } Self::NoModuleInstancesFound => { write!(f, "found no module instances registered so far") @@ -34,11 +34,7 @@ impl Display for TestError { module_name, func_name, } => { - write!( - f, - "missing func exported as: {:?}::{}", - module_name, func_name - ) + write!(f, "missing func exported as: {module_name:?}::{func_name}",) } Self::GlobalNotFound { module_name, @@ -46,8 +42,7 @@ impl Display for TestError { } => { write!( f, - "missing global variable exported as: {:?}::{}", - module_name, global_name + "missing global variable exported as: {module_name:?}::{global_name}", ) } Self::Wasmi(wasmi_error) => Display::fmt(wasmi_error, f), diff --git a/crates/wasmi/tests/spec/run.rs b/crates/wasmi/tests/spec/run.rs index f32fc54b3e..6659b756c7 100644 --- a/crates/wasmi/tests/spec/run.rs +++ b/crates/wasmi/tests/spec/run.rs @@ -145,7 +145,7 @@ fn execute_directives(wast: Wast, test_context: &mut TestContext) -> Result<()> error ) }); - assert_results(&test_context, span, &results, &expected); + assert_results(test_context, span, &results, &expected); } WastDirective::AssertExhaustion { span, @@ -178,13 +178,12 @@ fn execute_directives(wast: Wast, test_context: &mut TestContext) -> Result<()> } WastDirective::AssertException { span, exec } => { test_context.profile().bump_assert_exception(); - match execute_wast_execute(test_context, span, exec) { - Ok(results) => panic!( + if let Ok(results) = execute_wast_execute(test_context, span, exec) { + panic!( "{}: expected to fail due to exception but succeeded with: {:?}", test_context.spanned(span), results - ), - Err(_) => {} + ) } } }