From c184ab3926948a97910bcb568b85b0f6b733405e Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 1 Dec 2020 14:55:19 +0800 Subject: [PATCH] ci: add cargo clippy --- .github/workflows/lint.yaml | 6 +++++- napi-derive/src/lib.rs | 4 ++-- napi/src/async_work.rs | 4 ++-- napi/src/call_context.rs | 8 +++----- napi/src/cleanup_env.rs | 2 +- napi/src/env.rs | 18 +++++++++--------- napi/src/js_values/de.rs | 4 ++-- napi/src/js_values/escapable_handle_scope.rs | 1 - napi/src/js_values/function.rs | 6 ++---- napi/src/js_values/mod.rs | 3 +++ napi/src/js_values/string/latin1.rs | 5 +++++ napi/src/js_values/string/utf16.rs | 5 +++++ napi/src/js_values/string/utf8.rs | 7 ++++++- napi/src/js_values/value_ref.rs | 1 - napi/src/lib.rs | 2 ++ test_module/src/napi4/tsfn.rs | 2 +- test_module/src/tokio_rt/read_file.rs | 4 ++-- 17 files changed, 50 insertions(+), 32 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 59f91853..aaff0f0c 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -25,6 +25,7 @@ jobs: toolchain: stable profile: default override: true + components: rustfmt, clippy - name: Generate Cargo.lock uses: actions-rs/cargo@v1 @@ -55,9 +56,12 @@ jobs: - name: 'Lint JS/TS' run: yarn lint - - name: 'Lint rust' + - name: Cargo fmt run: cargo fmt -- --check + - name: Clippy + run: cargo clippy + - name: Clear the cargo caches run: | cargo install cargo-cache --no-default-features --features ci-autoclean diff --git a/napi-derive/src/lib.rs b/napi-derive/src/lib.rs index f7b8760c..bb194dfa 100644 --- a/napi-derive/src/lib.rs +++ b/napi-derive/src/lib.rs @@ -18,8 +18,8 @@ impl Parse for ArgLength { Ok(ArgLength { length: vars .first() - .map(|i| i.clone()) - .unwrap_or(Literal::usize_unsuffixed(0)), + .cloned() + .unwrap_or_else(|| Literal::usize_unsuffixed(0)), }) } } diff --git a/napi/src/async_work.rs b/napi/src/async_work.rs index a8aa8537..ff7bb4c4 100644 --- a/napi/src/async_work.rs +++ b/napi/src/async_work.rs @@ -32,7 +32,7 @@ impl<'env> AsyncWorkPromise<'env> { } #[inline] -pub fn run<'env, T: Task>(env: &'env Env, task: T) -> Result> { +pub fn run(env: &Env, task: T) -> Result> { let mut raw_resource = ptr::null_mut(); check_status!(unsafe { sys::napi_create_object(env.0, &mut raw_resource) })?; let mut raw_promise = ptr::null_mut(); @@ -86,7 +86,7 @@ unsafe extern "C" fn execute(_env: sys::napi_env, data: *mut c_void) { let mut work = Box::from_raw(data as *mut AsyncWork); let _ = mem::replace( &mut work.value, - work.inner_task.compute().map(|v| mem::MaybeUninit::new(v)), + work.inner_task.compute().map(mem::MaybeUninit::new), ); Box::leak(work); } diff --git a/napi/src/call_context.rs b/napi/src/call_context.rs index 9c37ce32..cff33ad2 100644 --- a/napi/src/call_context.rs +++ b/napi/src/call_context.rs @@ -53,12 +53,10 @@ impl<'env> CallContext<'env> { status: Status::GenericFailure, reason: "Arguments index out of range".to_owned(), }) + } else if index < self.length { + unsafe { ArgType::from_raw(self.env.0, self.args[index]) }.map(Either::A) } else { - if index < self.length { - unsafe { ArgType::from_raw(self.env.0, self.args[index]) }.map(Either::A) - } else { - self.env.get_undefined().map(Either::B) - } + self.env.get_undefined().map(Either::B) } } diff --git a/napi/src/cleanup_env.rs b/napi/src/cleanup_env.rs index 37aac59e..26fee47d 100644 --- a/napi/src/cleanup_env.rs +++ b/napi/src/cleanup_env.rs @@ -1,6 +1,6 @@ pub(crate) struct CleanupEnvHookData { pub(crate) data: T, - pub(crate) hook: Box ()>, + pub(crate) hook: Box, } #[derive(Clone, Copy)] diff --git a/napi/src/env.rs b/napi/src/env.rs index 5bfe8da1..6e7b7677 100644 --- a/napi/src/env.rs +++ b/napi/src/env.rs @@ -1,7 +1,6 @@ use std::any::TypeId; use std::convert::TryInto; use std::ffi::CString; -use std::mem; use std::os::raw::{c_char, c_void}; use std::ptr; @@ -37,6 +36,7 @@ pub struct Env(pub(crate) sys::napi_env); impl Env { #[inline] + #[allow(clippy::missing_safety_doc)] pub unsafe fn from_raw(env: sys::napi_env) -> Self { Env(env) } @@ -461,9 +461,9 @@ impl Env { &mut unknown_tagged_object, ))?; - let type_id: *const TypeId = mem::transmute(unknown_tagged_object); + let type_id = unknown_tagged_object as *const TypeId; if *type_id == TypeId::of::() { - let tagged_object: *mut TaggedObject = mem::transmute(unknown_tagged_object); + let tagged_object = unknown_tagged_object as *mut TaggedObject; (*tagged_object).object.as_mut().ok_or(Error { status: Status::InvalidArg, reason: "Invalid argument, nothing attach to js_object".to_owned(), @@ -487,9 +487,9 @@ impl Env { &mut unknown_tagged_object, ))?; - let type_id: *const TypeId = mem::transmute(unknown_tagged_object); + let type_id = unknown_tagged_object as *const TypeId; if *type_id == TypeId::of::() { - let tagged_object: *mut TaggedObject = mem::transmute(unknown_tagged_object); + let tagged_object = unknown_tagged_object as *mut TaggedObject; (*tagged_object).object = None; Ok(()) } else { @@ -527,9 +527,9 @@ impl Env { &mut unknown_tagged_object, ))?; - let type_id: *const TypeId = mem::transmute(unknown_tagged_object); + let type_id = unknown_tagged_object as *const TypeId; if *type_id == TypeId::of::() { - let tagged_object: *mut TaggedObject = mem::transmute(unknown_tagged_object); + let tagged_object = unknown_tagged_object as *mut TaggedObject; (*tagged_object).object.as_mut().ok_or(Error { status: Status::InvalidArg, reason: "nothing attach to js_external".to_owned(), @@ -610,7 +610,7 @@ impl Env { ) -> Result> where T: 'static, - F: 'static + FnOnce(T) -> (), + F: 'static + FnOnce(T), { let hook = CleanupEnvHookData { data: cleanup_data, @@ -793,7 +793,7 @@ unsafe extern "C" fn raw_finalize( finalize_data: *mut c_void, _finalize_hint: *mut c_void, ) { - let tagged_object: *mut TaggedObject = mem::transmute(finalize_data); + let tagged_object = finalize_data as *mut TaggedObject; Box::from_raw(tagged_object); } diff --git a/napi/src/js_values/de.rs b/napi/src/js_values/de.rs index 9b55d341..2dac524b 100644 --- a/napi/src/js_values/de.rs +++ b/napi/src/js_values/de.rs @@ -109,7 +109,7 @@ impl<'x, 'de, 'env> serde::de::Deserializer<'x> for &'de mut De<'env> { ValueType::String => visitor.visit_enum(JsEnumAccess::new( unsafe { JsString::from_raw_unchecked(self.0.env, self.0.value) } .into_utf8()? - .to_owned()?, + .into_owned()?, None, )), ValueType::Object => { @@ -128,7 +128,7 @@ impl<'x, 'de, 'env> serde::de::Deserializer<'x> for &'de mut De<'env> { let key = properties.get_element::(0)?; let value: JsUnknown = js_object.get_property(&key)?; visitor.visit_enum(JsEnumAccess::new( - key.into_utf8()?.to_owned()?, + key.into_utf8()?.into_owned()?, Some(&value.0), )) } diff --git a/napi/src/js_values/escapable_handle_scope.rs b/napi/src/js_values/escapable_handle_scope.rs index 734fbe46..b345dc2c 100644 --- a/napi/src/js_values/escapable_handle_scope.rs +++ b/napi/src/js_values/escapable_handle_scope.rs @@ -24,7 +24,6 @@ impl EscapableHandleScope { }) } - #[must_use] pub fn close(self, env: Env) -> Result<()> { check_status!(unsafe { sys::napi_close_escapable_handle_scope(env.0, self.handle_scope) }) } diff --git a/napi/src/js_values/function.rs b/napi/src/js_values/function.rs index 3e2b1c53..b395f395 100644 --- a/napi/src/js_values/function.rs +++ b/napi/src/js_values/function.rs @@ -32,10 +32,7 @@ impl JsFunction { .ok() .map(|u| unsafe { u.raw() }) }) - .ok_or(Error::new( - Status::GenericFailure, - "Get raw this failed".to_owned(), - ))?; + .ok_or_else(|| Error::new(Status::GenericFailure, "Get raw this failed".to_owned()))?; let raw_args = args .iter() .map(|arg| arg.0.value) @@ -57,6 +54,7 @@ impl JsFunction { /// https://nodejs.org/api/n-api.html#n_api_napi_new_instance /// This method is used to instantiate a new `JavaScript` value using a given `JsFunction` that represents the constructor for the object. + #[allow(clippy::new_ret_no_self)] #[inline] pub fn new(&self, args: &[V]) -> Result where diff --git a/napi/src/js_values/mod.rs b/napi/src/js_values/mod.rs index 82e5c937..1120be69 100644 --- a/napi/src/js_values/mod.rs +++ b/napi/src/js_values/mod.rs @@ -516,10 +516,13 @@ macro_rules! impl_object_methods { } pub trait NapiValue: Sized { + #[allow(clippy::missing_safety_doc)] unsafe fn from_raw(env: sys::napi_env, value: sys::napi_value) -> Result; + #[allow(clippy::missing_safety_doc)] unsafe fn from_raw_unchecked(env: sys::napi_env, value: sys::napi_value) -> Self; + #[allow(clippy::missing_safety_doc)] unsafe fn raw(&self) -> sys::napi_value; } diff --git a/napi/src/js_values/string/latin1.rs b/napi/src/js_values/string/latin1.rs index 29496923..2f6e7c43 100644 --- a/napi/src/js_values/string/latin1.rs +++ b/napi/src/js_values/string/latin1.rs @@ -24,6 +24,11 @@ impl JsStringLatin1 { self.buf.len() } + #[inline] + pub fn is_empty(&self) -> bool { + self.buf.is_empty() + } + #[inline] pub fn take(self) -> Vec { self.as_slice().to_vec() diff --git a/napi/src/js_values/string/utf16.rs b/napi/src/js_values/string/utf16.rs index 485d1241..4ae5b71f 100644 --- a/napi/src/js_values/string/utf16.rs +++ b/napi/src/js_values/string/utf16.rs @@ -26,6 +26,11 @@ impl JsStringUtf16 { self.buf.len() } + #[inline] + pub fn is_empty(&self) -> bool { + self.buf.is_empty() + } + #[inline] pub fn into_value(self) -> JsString { self.inner diff --git a/napi/src/js_values/string/utf8.rs b/napi/src/js_values/string/utf8.rs index fbe777c1..fb8c662d 100644 --- a/napi/src/js_values/string/utf8.rs +++ b/napi/src/js_values/string/utf8.rs @@ -27,7 +27,12 @@ impl JsStringUtf8 { } #[inline] - pub fn to_owned(self) -> Result { + pub fn is_empty(&self) -> bool { + self.buf.is_empty() + } + + #[inline] + pub fn into_owned(self) -> Result { Ok(self.as_str()?.to_owned()) } diff --git a/napi/src/js_values/value_ref.rs b/napi/src/js_values/value_ref.rs index bfb37977..90bb4c24 100644 --- a/napi/src/js_values/value_ref.rs +++ b/napi/src/js_values/value_ref.rs @@ -39,7 +39,6 @@ impl Ref { Ok(self.count) } - #[must_use] #[inline] pub fn unref(mut self, env: Env) -> Result { check_status!(unsafe { sys::napi_reference_unref(env.0, self.raw_ref, &mut self.count) })?; diff --git a/napi/src/lib.rs b/napi/src/lib.rs index 2dc5ec8d..f9e441ad 100644 --- a/napi/src/lib.rs +++ b/napi/src/lib.rs @@ -1,3 +1,5 @@ +#![deny(clippy::all)] + //! High level NodeJS [N-API](https://nodejs.org/api/n-api.html) binding //! //! **napi-rs** provides minimal overhead to write N-API modules in `Rust`. diff --git a/test_module/src/napi4/tsfn.rs b/test_module/src/napi4/tsfn.rs index 5508f0fd..7097157d 100644 --- a/test_module/src/napi4/tsfn.rs +++ b/test_module/src/napi4/tsfn.rs @@ -127,7 +127,7 @@ async fn read_file_content(filepath: &Path) -> Result> { pub fn test_tokio_readfile(ctx: CallContext) -> Result { let js_filepath = ctx.get::(0)?; let js_func = ctx.get::(1)?; - let path_str = js_filepath.into_utf8()?.to_owned()?; + let path_str = js_filepath.into_utf8()?.into_owned()?; let tsfn = ctx diff --git a/test_module/src/tokio_rt/read_file.rs b/test_module/src/tokio_rt/read_file.rs index 4a943e57..847305ef 100644 --- a/test_module/src/tokio_rt/read_file.rs +++ b/test_module/src/tokio_rt/read_file.rs @@ -5,7 +5,7 @@ use tokio; #[js_function(1)] pub fn test_execute_tokio_readfile(ctx: CallContext) -> Result { let js_filepath = ctx.get::(0)?; - let path_str = js_filepath.into_utf8()?.to_owned()?; + let path_str = js_filepath.into_utf8()?.into_owned()?; ctx.env.execute_tokio_future( tokio::fs::read(path_str).map(|v| { v.map_err(|e| { @@ -22,7 +22,7 @@ pub fn test_execute_tokio_readfile(ctx: CallContext) -> Result { #[js_function(1)] pub fn error_from_tokio_future(ctx: CallContext) -> Result { let js_filepath = ctx.get::(0)?; - let path_str = js_filepath.into_utf8()?.to_owned()?; + let path_str = js_filepath.into_utf8()?.into_owned()?; ctx.env.execute_tokio_future( tokio::fs::read(path_str) .map_err(Error::from)