Skip to content

Commit

Permalink
more pedantic and nursery clippy (#222)
Browse files Browse the repository at this point in the history
* more pedantic and nursery clippy

* mark methods as must_use or ignore if has side effect

* format code
  • Loading branch information
gkorland authored Feb 14, 2022
1 parent 52b4e2a commit d6aef18
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 42 deletions.
22 changes: 9 additions & 13 deletions examples/data_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static MY_REDIS_TYPE: RedisType = RedisType::new(
);

unsafe extern "C" fn free(value: *mut c_void) {
Box::from_raw(value as *mut MyType);
Box::from_raw(value.cast::<MyType>());
}

fn alloc_set(ctx: &Context, args: Vec<RedisString>) -> RedisResult {
Expand All @@ -49,19 +49,15 @@ fn alloc_set(ctx: &Context, args: Vec<RedisString>) -> RedisResult {

let key = ctx.open_key_writable(&key);

match key.get_value::<MyType>(&MY_REDIS_TYPE)? {
Some(value) => {
value.data = "B".repeat(size as usize);
}
None => {
let value = MyType {
data: "A".repeat(size as usize),
};

key.set_value(&MY_REDIS_TYPE, value)?;
}
}
if let Some(value) = key.get_value::<MyType>(&MY_REDIS_TYPE)? {
value.data = "B".repeat(size as usize);
} else {
let value = MyType {
data: "A".repeat(size as usize),
};

key.set_value(&MY_REDIS_TYPE, value)?;
}
Ok(size.into())
}

Expand Down
2 changes: 1 addition & 1 deletion examples/hello.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn hello_mul(_: &Context, args: Vec<RedisString>) -> RedisResult {
}

fn hello_err(ctx: &Context, args: Vec<RedisString>) -> RedisResult {
if args.len() < 1 {
if args.is_empty() {
return Err(RedisError::WrongArity);
}

Expand Down
1 change: 1 addition & 0 deletions src/context/blocked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ impl Drop for BlockedClient {
}

impl Context {
#[must_use]
pub fn block_client(&self) -> BlockedClient {
let blocked_client = unsafe {
raw::RedisModule_BlockClient.unwrap()(
Expand Down
1 change: 1 addition & 0 deletions src/context/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ impl ServerInfo {
}

impl Context {
#[must_use]
pub fn server_info(&self, section: &str) -> ServerInfo {
let section = CString::new(section).unwrap();
let server_info = unsafe {
Expand Down
36 changes: 25 additions & 11 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ use crate::{RedisError, RedisResult, RedisString, RedisValue};
mod timer;

#[cfg(feature = "experimental-api")]
pub(crate) mod thread_safe;
pub mod thread_safe;

#[cfg(feature = "experimental-api")]
pub(crate) mod blocked;
pub mod blocked;

pub(crate) mod info;
pub mod info;

/// `Context` is a structure that's designed to give us a high-level interface to
/// the Redis module API by abstracting away the raw C FFI calls.
Expand All @@ -26,11 +26,12 @@ pub struct Context {
}

impl Context {
pub fn new(ctx: *mut raw::RedisModuleCtx) -> Self {
pub const fn new(ctx: *mut raw::RedisModuleCtx) -> Self {
Self { ctx }
}

pub fn dummy() -> Self {
#[must_use]
pub const fn dummy() -> Self {
Self {
ctx: ptr::null_mut(),
}
Expand Down Expand Up @@ -68,6 +69,7 @@ impl Context {
/// # Panics
///
/// Will panic if `RedisModule_IsKeysPositionRequest` is missing in redismodule.h
#[must_use]
pub fn is_keys_position_request(&self) -> bool {
// We want this to be available in tests where we don't have an actual Redis to call
if cfg!(feature = "test") {
Expand Down Expand Up @@ -127,7 +129,7 @@ impl Context {
for i in 0..length {
vec.push(Self::parse_call_reply(raw::call_reply_array_element(
reply, i,
))?)
))?);
}
Ok(RedisValue::Array(vec))
}
Expand All @@ -137,6 +139,7 @@ impl Context {
}
}

#[must_use]
pub fn str_as_legal_resp_string(s: &str) -> CString {
CString::new(
s.chars()
Expand All @@ -149,11 +152,13 @@ impl Context {
.unwrap()
}

#[allow(clippy::must_use_candidate)]
pub fn reply_simple_string(&self, s: &str) -> raw::Status {
let msg = Self::str_as_legal_resp_string(s);
unsafe { raw::RedisModule_ReplyWithSimpleString.unwrap()(self.ctx, msg.as_ptr()).into() }
}

#[allow(clippy::must_use_candidate)]
pub fn reply_error_string(&self, s: &str) -> raw::Status {
let msg = Self::str_as_legal_resp_string(s);
unsafe { raw::RedisModule_ReplyWithError.unwrap()(self.ctx, msg.as_ptr()).into() }
Expand All @@ -162,6 +167,7 @@ impl Context {
/// # Panics
///
/// Will panic if methods used are missing in redismodule.h
#[allow(clippy::must_use_candidate)]
pub fn reply(&self, r: RedisResult) -> raw::Status {
match r {
Ok(RedisValue::Integer(v)) => unsafe {
Expand All @@ -185,7 +191,7 @@ impl Context {
Ok(RedisValue::BulkString(s)) => unsafe {
raw::RedisModule_ReplyWithStringBuffer.unwrap()(
self.ctx,
s.as_ptr() as *const c_char,
s.as_ptr().cast::<c_char>(),
s.len() as usize,
)
.into()
Expand All @@ -198,7 +204,7 @@ impl Context {
Ok(RedisValue::StringBuffer(s)) => unsafe {
raw::RedisModule_ReplyWithStringBuffer.unwrap()(
self.ctx,
s.as_ptr() as *const c_char,
s.as_ptr().cast::<c_char>(),
s.len() as usize,
)
.into()
Expand Down Expand Up @@ -243,10 +249,12 @@ impl Context {
}
}

#[must_use]
pub fn open_key(&self, key: &RedisString) -> RedisKey {
RedisKey::open(self.ctx, key)
}

#[must_use]
pub fn open_key_writable(&self, key: &RedisString) -> RedisKeyWritable {
RedisKeyWritable::open(self.ctx, key)
}
Expand All @@ -255,11 +263,13 @@ impl Context {
raw::replicate_verbatim(self.ctx);
}

#[must_use]
pub fn create_string(&self, s: &str) -> RedisString {
RedisString::create(self.ctx, s)
}

pub fn get_raw(&self) -> *mut raw::RedisModuleCtx {
#[must_use]
pub const fn get_raw(&self) -> *mut raw::RedisModuleCtx {
self.ctx
}

Expand All @@ -274,6 +284,7 @@ impl Context {
}

#[cfg(feature = "experimental-api")]
#[allow(clippy::must_use_candidate)]
pub fn notify_keyspace_event(
&self,
event_type: raw::NotifyEvent,
Expand All @@ -283,7 +294,7 @@ impl Context {
unsafe { raw::notify_keyspace_event(self.ctx, event_type, event, keyname) }
}

/// Returns the redis version either by calling RedisModule_GetServerVersion API,
/// Returns the redis version either by calling ``RedisModule_GetServerVersion`` API,
/// Or if it is not available, by calling "info server" API and parsing the reply
pub fn get_redis_version(&self) -> Result<Version, RedisError> {
self.get_redis_version_internal(false)
Expand Down Expand Up @@ -332,18 +343,21 @@ pub struct InfoContext {
}

impl InfoContext {
pub fn new(ctx: *mut raw::RedisModuleInfoCtx) -> Self {
pub const fn new(ctx: *mut raw::RedisModuleInfoCtx) -> Self {
Self { ctx }
}

#[allow(clippy::must_use_candidate)]
pub fn add_info_section(&self, name: Option<&str>) -> Status {
add_info_section(self.ctx, name)
}

#[allow(clippy::must_use_candidate)]
pub fn add_info_field_str(&self, name: &str, content: &str) -> Status {
add_info_field_str(self.ctx, name, content)
}

#[allow(clippy::must_use_candidate)]
pub fn add_info_field_long_long(&self, name: &str, value: c_longlong) -> Status {
add_info_field_long_long(self.ctx, name, value)
}
Expand Down
9 changes: 6 additions & 3 deletions src/context/thread_safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl Deref for ContextGuard {
}
}

/// A ThreadSafeContext can either be bound to a blocked client, or detached from any client.
/// A ``ThreadSafeContext`` can either be bound to a blocked client, or detached from any client.
pub struct DetachedFromClient;

pub struct ThreadSafeContext<B: Send> {
Expand All @@ -37,9 +37,10 @@ unsafe impl<B: Send> Send for ThreadSafeContext<B> {}
unsafe impl<B: Send> Sync for ThreadSafeContext<B> {}

impl ThreadSafeContext<DetachedFromClient> {
#[must_use]
pub fn new() -> Self {
let ctx = unsafe { raw::RedisModule_GetThreadSafeContext.unwrap()(ptr::null_mut()) };
ThreadSafeContext {
Self {
ctx,
blocked_client: DetachedFromClient,
}
Expand All @@ -53,16 +54,18 @@ impl Default for ThreadSafeContext<DetachedFromClient> {
}

impl ThreadSafeContext<BlockedClient> {
#[must_use]
pub fn with_blocked_client(blocked_client: BlockedClient) -> Self {
let ctx = unsafe { raw::RedisModule_GetThreadSafeContext.unwrap()(blocked_client.inner) };
ThreadSafeContext {
Self {
ctx,
blocked_client,
}
}

/// The Redis modules API does not require locking for `Reply` functions,
/// so we pass through its functionality directly.
#[allow(clippy::must_use_candidate)]
pub fn reply(&self, r: RedisResult) -> raw::Status {
let ctx = Context::new(self.ctx);
ctx.reply(r)
Expand Down
8 changes: 4 additions & 4 deletions src/context/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl Context {
data: T,
) -> RedisModuleTimerID
where
F: FnOnce(&Context, T),
F: FnOnce(&Self, T),
{
let cb_data = CallbackData { data, callback };

Expand All @@ -46,7 +46,7 @@ impl Context {
.try_into()
.expect("Value must fit in 64 bits"),
Some(raw_callback::<F, T>),
data as *mut c_void,
data.cast::<c_void>(),
)
}
}
Expand Down Expand Up @@ -98,7 +98,7 @@ impl Context {
}

// Cast the *mut c_void supplied by the Redis API to a raw pointer of our custom type.
let data = data as *mut T;
let data = data.cast::<T>();

// Dereference the raw pointer (we know this is safe, since Redis should return our
// original pointer which we know to be good) and turn it into a safe reference
Expand All @@ -110,7 +110,7 @@ impl Context {

fn take_data<T>(data: *mut c_void) -> T {
// Cast the *mut c_void supplied by the Redis API to a raw pointer of our custom type.
let data = data as *mut T;
let data = data.cast::<T>();

// Take back ownership of the original boxed data, so we can unbox it safely.
// If we don't do this, the data's memory will be leaked.
Expand Down
2 changes: 2 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub enum Error {
}

impl Error {
#[must_use]
pub fn generic(message: &str) -> Self {
Self::Generic(GenericError::new(message))
}
Expand Down Expand Up @@ -66,6 +67,7 @@ pub struct GenericError {
}

impl GenericError {
#[must_use]
pub fn new(message: &str) -> Self {
Self {
message: String::from(message),
Expand Down
6 changes: 6 additions & 0 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,12 @@ impl RedisKeyWritable {
Ok(Some(read_key(self.key_inner)?))
}

#[allow(clippy::must_use_candidate)]
pub fn hash_set(&self, field: &str, value: RedisString) -> raw::Status {
raw::hash_set(self.key_inner, field, value.inner)
}

#[allow(clippy::must_use_candidate)]
pub fn hash_del(&self, field: &str) -> raw::Status {
raw::hash_del(self.key_inner, field)
}
Expand Down Expand Up @@ -191,11 +193,13 @@ impl RedisKeyWritable {
}

// `list_push_head` inserts the specified element at the head of the list stored at this key.
#[allow(clippy::must_use_candidate)]
pub fn list_push_head(&self, element: RedisString) -> raw::Status {
raw::list_push(self.key_inner, raw::Where::ListHead, element.inner)
}

// `list_push_tail` inserts the specified element at the tail of the list stored at this key.
#[allow(clippy::must_use_candidate)]
pub fn list_push_tail(&self, element: RedisString) -> raw::Status {
raw::list_push(self.key_inner, raw::Where::ListTail, element.inner)
}
Expand All @@ -204,6 +208,7 @@ impl RedisKeyWritable {
// Returns None when:
// 1. The list is empty.
// 2. The key is not a list.
#[allow(clippy::must_use_candidate)]
pub fn list_pop_head(&self) -> Option<RedisString> {
let ptr = raw::list_pop(self.key_inner, raw::Where::ListHead);

Expand Down Expand Up @@ -267,6 +272,7 @@ impl RedisKeyWritable {
/// # Panics
///
/// Will panic if `RedisModule_KeyType` is missing in redismodule.h
#[must_use]
pub fn key_type(&self) -> raw::KeyType {
unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }.into()
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub use crate::redismodule::*;
use backtrace::Backtrace;

/// Ideally this would be `#[cfg(not(test))]`, but that doesn't work:
/// https://github.com/rust-lang/rust/issues/59168#issuecomment-472653680
/// [59168#issuecomment-472653680](https://github.com/rust-lang/rust/issues/59168#issuecomment-472653680)
/// The workaround is to use the `test` feature instead.
#[cfg(not(feature = "test"))]
#[global_allocator]
Expand Down
1 change: 1 addition & 0 deletions src/native_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub struct RedisType {
unsafe impl Sync for RedisType {}

impl RedisType {
#[must_use]
pub const fn new(
name: &'static str,
version: i32,
Expand Down
3 changes: 2 additions & 1 deletion src/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,7 @@ pub unsafe fn notify_keyspace_event(
}

#[cfg(feature = "experimental-api")]
#[must_use]
pub fn get_keyspace_events() -> NotifyEvent {
unsafe {
let events = RedisModule_GetNotifyKeyspaceEvents.unwrap()();
Expand All @@ -648,7 +649,7 @@ pub struct Version {
impl From<c_int> for Version {
fn from(ver: c_int) -> Self {
// Expected format: 0x00MMmmpp for Major, minor, patch
Version {
Self {
major: (ver & 0x00FF_0000) >> 16,
minor: (ver & 0x0000_FF00) >> 8,
patch: (ver & 0x0000_00FF),
Expand Down
Loading

0 comments on commit d6aef18

Please sign in to comment.