From 14acb0617984a2465a8179d46c1aca0ebdb9e16b Mon Sep 17 00:00:00 2001 From: Dalot Date: Tue, 17 Aug 2021 23:35:23 +0100 Subject: [PATCH 1/2] Resolved clippy warnings. Proposal of ifcfg crate. --- redisless/Cargo.toml | 1 + redisless/benches/benchmarks.rs | 2 +- redisless/src/cluster/node.rs | 11 +++++++---- redisless/src/cluster/peer.rs | 2 +- redisless/src/cluster/tests.rs | 9 +++++++-- redisless/src/cluster/util.rs | 11 +++++++---- redisless/src/command/command_error.rs | 2 +- redisless/src/command/mod.rs | 12 ++++++------ redisless/src/lib.rs | 9 ++++++++- redisless/src/protocol/response/mod.rs | 9 +++------ redisless/src/server/mod.rs | 4 ++-- redisless/src/server/tests/mod.rs | 18 +++++++++--------- redisless/src/server/util/mod.rs | 2 +- redisless/src/server/util/run_command.rs | 14 ++++++-------- redisless/src/storage/in_memory.rs | 10 ++++------ redisless/src/storage/tests/mod.rs | 14 +++++++------- redisless/src/tests/mod.rs | 2 +- 17 files changed, 72 insertions(+), 60 deletions(-) diff --git a/redisless/Cargo.toml b/redisless/Cargo.toml index f673324..176f039 100644 --- a/redisless/Cargo.toml +++ b/redisless/Cargo.toml @@ -25,6 +25,7 @@ ipnet = "2.3" chrono = "0.4" [dev-dependencies] +cfg-if = "1.0.0" redis = "0.20" serial_test = "0.5" criterion = "0.3" diff --git a/redisless/benches/benchmarks.rs b/redisless/benches/benchmarks.rs index 2723245..b3865ff 100644 --- a/redisless/benches/benchmarks.rs +++ b/redisless/benches/benchmarks.rs @@ -8,7 +8,7 @@ use redisless::storage::in_memory::InMemoryStorage; fn criterion_benchmarks(c: &mut Criterion) { let port = 3335; - let server = Server::new(InMemoryStorage::new(), port); + let server = Server::new(InMemoryStorage::default(), port); assert_eq!(server.start(), Some(ServerState::Started)); let mut stream = TcpStream::connect(format!("localhost:{}", port)).unwrap(); diff --git a/redisless/src/cluster/node.rs b/redisless/src/cluster/node.rs index a750ede..8cfcda3 100644 --- a/redisless/src/cluster/node.rs +++ b/redisless/src/cluster/node.rs @@ -5,11 +5,8 @@ use rand::rngs::OsRng; use raft::log::memory::InMemoryLog; use raft::node::Node; -use crate::cluster::peer::{Peer, Peers, PeersDiscovery, DEFAULT_NODE_LISTENING_PORT}; -use crate::cluster::util::{get_ip_addresses, get_local_network_ip_addresses, scan_ip_range}; +use crate::cluster::peer::{Peer, PeersDiscovery}; use crossbeam_channel::{unbounded, Receiver, Sender}; -use std::borrow::Borrow; -use std::collections::{HashSet, LinkedList}; use std::thread; use std::time::Duration; @@ -20,9 +17,15 @@ pub const GETINFO_RESPONSE: &[u8; 9] = b"redisless"; type RaftNode = Node; pub struct ClusterNode { + #[allow(dead_code)] node: RaftNode, + + #[allow(dead_code)] listening_socket_addr: SocketAddr, + + #[allow(dead_code)] peer_receiver: Receiver, + listener_started: bool, search_peers_started: bool, } diff --git a/redisless/src/cluster/peer.rs b/redisless/src/cluster/peer.rs index 10b99ba..c4cd0d7 100644 --- a/redisless/src/cluster/peer.rs +++ b/redisless/src/cluster/peer.rs @@ -83,7 +83,7 @@ fn search_peers(listening_port: u16) -> Peers { ports.insert(0, DEFAULT_NODE_LISTENING_PORT) } - let peers = scan_ip_range(local_ip_addresses.clone(), ports.clone()); + let peers = scan_ip_range(local_ip_addresses, ports); peers .into_iter() diff --git a/redisless/src/cluster/tests.rs b/redisless/src/cluster/tests.rs index fd24cc9..3adb2e4 100644 --- a/redisless/src/cluster/tests.rs +++ b/redisless/src/cluster/tests.rs @@ -1,11 +1,16 @@ +#[cfg(test)] +cfg_if::cfg_if! { + if #[cfg(test)] { use std::net::{IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4}; - use crate::cluster::node::ClusterNode; use crate::cluster::peer::{Peer, PeersDiscovery, DEFAULT_NODE_LISTENING_PORT}; use crate::cluster::util::{ get_ip_addresses, get_local_network_ip_addresses, get_range_from_ip_address, scan_ip_range, Range, }; +} else { + } +} #[test] fn start_cluster() { @@ -15,7 +20,7 @@ fn start_cluster() { #[test] fn list_ip_addresses() { let ip_addresses = get_ip_addresses(); - assert!(ip_addresses.len() > 0); + assert!(!ip_addresses.is_empty()); } #[test] diff --git a/redisless/src/cluster/util.rs b/redisless/src/cluster/util.rs index 1d829f2..1bb5e2f 100644 --- a/redisless/src/cluster/util.rs +++ b/redisless/src/cluster/util.rs @@ -1,4 +1,4 @@ -use std::io::{BufReader, Read, Write}; +use std::io::{Read, Write}; use std::net::{IpAddr, SocketAddr, TcpStream}; use std::thread; use std::time::Duration; @@ -27,7 +27,7 @@ pub fn get_local_network_ip_addresses(ip_addresses: Vec) -> Vec pub fn get_ip_addresses() -> Vec { let mut ip_addresses = vec![]; - for interfaces in get_if_addrs::get_if_addrs() { + if let Ok(interfaces) = get_if_addrs::get_if_addrs() { for interface in interfaces { ip_addresses.push(interface.ip()); } @@ -36,6 +36,7 @@ pub fn get_ip_addresses() -> Vec { ip_addresses } +#[allow(dead_code)] pub enum Range { Sixteen, TwentyFour, @@ -46,6 +47,7 @@ pub enum Range { /// - 10.0.0.0/8 /// - 172.16.0.0/12 /// - 192.168.0.0/16 +#[allow(dead_code)] pub fn get_range_from_ip_address(ip_address: IpAddr, range: Range) -> Vec { let ip_address = match ip_address { IpAddr::V4(ip_address) => ip_address, @@ -63,7 +65,7 @@ pub fn get_range_from_ip_address(ip_address: IpAddr, range: Range) -> Vec= 16 && b <= 31 => match range { + [172, b, c, _] if (16..=31).contains(&b) => match range { Range::Sixteen => Ipv4AddrRange::new( format!("172.{}.0.0", b).parse().unwrap(), format!("172.{}.255.255", b).parse().unwrap(), @@ -88,13 +90,14 @@ pub fn get_range_from_ip_address(ip_address: IpAddr, range: Range) -> Vec { Ok(T), Continue, + #[allow(dead_code)] End, } diff --git a/redisless/src/command/command_error.rs b/redisless/src/command/command_error.rs index a97be44..0a5f928 100644 --- a/redisless/src/command/command_error.rs +++ b/redisless/src/command/command_error.rs @@ -30,7 +30,7 @@ pub enum RedisCommandError { } impl RedisCommandError { - pub fn to_vec(self) -> Vec { + pub fn to_vec(&self) -> Vec { format!("-{}\r\n", self).as_bytes().to_vec() } } diff --git a/redisless/src/command/mod.rs b/redisless/src/command/mod.rs index 9a77da5..7e57586 100644 --- a/redisless/src/command/mod.rs +++ b/redisless/src/command/mod.rs @@ -115,8 +115,8 @@ impl Command { for pair in pairs.chunks_exact(chunk_size) { match pair { [key, value] => { - let key = get_bytes_vec(Some(&key))?; - let value = get_bytes_vec(Some(&value))?; + let key = get_bytes_vec(Some(key))?; + let value = get_bytes_vec(Some(value))?; items.push((key, value)); } _ => unreachable!(), // pairs has even length so each chunk will have len 2 @@ -136,8 +136,8 @@ impl Command { for pair in pairs.chunks_exact(chunk_size) { match pair { [key, value] => { - let key = get_bytes_vec(Some(&key))?; - let value = get_bytes_vec(Some(&value))?; + let key = get_bytes_vec(Some(key))?; + let value = get_bytes_vec(Some(value))?; items.push((key, value)); } _ => unreachable!(), @@ -203,8 +203,8 @@ impl Command { for pair in pairs.chunks_exact(chunk_size) { match pair { [key, value] => { - let key = get_bytes_vec(Some(&key))?; - let value = get_bytes_vec(Some(&value))?; + let key = get_bytes_vec(Some(key))?; + let value = get_bytes_vec(Some(value))?; items.push((key, value)); } _ => unreachable!(), diff --git a/redisless/src/lib.rs b/redisless/src/lib.rs index 04a38ce..f221841 100644 --- a/redisless/src/lib.rs +++ b/redisless/src/lib.rs @@ -2,6 +2,9 @@ #[macro_use] extern crate serial_test; +#[cfg(test)] +extern crate cfg_if; + use storage::in_memory::InMemoryStorage; use crate::server::{Server, ServerState}; @@ -16,16 +19,19 @@ mod protocol; pub mod server; pub mod storage; +#[allow(clippy::missing_safety_doc)] #[no_mangle] pub unsafe extern "C" fn redisless_server_new(port: u16) -> *mut Server { - Box::into_raw(Box::new(Server::new(InMemoryStorage::new(), port))) + Box::into_raw(Box::new(Server::new(InMemoryStorage::default(), port))) } +#[allow(clippy::missing_safety_doc)] #[no_mangle] pub unsafe extern "C" fn redisless_server_free(server: *mut Server) { let _ = Box::from_raw(server); } +#[allow(clippy::missing_safety_doc)] #[no_mangle] pub unsafe extern "C" fn redisless_server_start(server: *mut Server) -> bool { let server = match server.as_ref() { @@ -39,6 +45,7 @@ pub unsafe extern "C" fn redisless_server_start(server: *mut Server) -> bool { } } +#[allow(clippy::missing_safety_doc)] #[no_mangle] pub unsafe extern "C" fn redisless_server_stop(server: *mut Server) -> bool { let server = match server.as_ref() { diff --git a/redisless/src/protocol/response/mod.rs b/redisless/src/protocol/response/mod.rs index a2992ce..93f7f77 100644 --- a/redisless/src/protocol/response/mod.rs +++ b/redisless/src/protocol/response/mod.rs @@ -25,10 +25,10 @@ enum RedisResponseInner { impl RedisResponseType { // move out of the enum - fn to_vec(self) -> Vec { + fn to_vec(&self) -> Vec { use RedisResponseType::*; match self { - SimpleString(s) | BulkString(s) => s, + SimpleString(s) | BulkString(s) => s.clone(), Integer(num) => num.to_string().as_bytes().to_vec(), Nil => NIL.to_vec(), } @@ -75,10 +75,7 @@ impl RedisResponse { } } pub fn is_quit(&self) -> bool { - match self.responses { - RedisResponseInner::Quit => true, - _ => false, - } + matches!(self.responses, RedisResponseInner::Quit) } pub fn single(response: RedisResponseType) -> Self { diff --git a/redisless/src/server/mod.rs b/redisless/src/server/mod.rs index 971d0db..16fc04f 100644 --- a/redisless/src/server/mod.rs +++ b/redisless/src/server/mod.rs @@ -199,7 +199,7 @@ fn start_server( for stream in listener.incoming() { match stream { Ok(tcp_stream) => { - handle_tcp_stream(tcp_stream, &thread_pool, &state_send, &state_recv, &storage); + handle_tcp_stream(tcp_stream, &thread_pool, state_send, state_recv, storage); } Err(err) if err.kind() == ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(10)); @@ -209,7 +209,7 @@ fn start_server( } } - if stop_sig_received(&state_recv, &state_send) { + if stop_sig_received(state_recv, state_send) { // let's gracefully shutdown the server break; } diff --git a/redisless/src/server/tests/mod.rs b/redisless/src/server/tests/mod.rs index 0b55d89..dc65be0 100644 --- a/redisless/src/server/tests/mod.rs +++ b/redisless/src/server/tests/mod.rs @@ -6,7 +6,7 @@ use crate::storage::in_memory::InMemoryStorage; use crate::Server; fn get_redis_client_connection(port: u16) -> (Server, Connection) { - let server = Server::new(InMemoryStorage::new(), port); + let server = Server::new(InMemoryStorage::default(), port); assert_eq!(server.start(), Some(ServerState::Started)); let redis_client = redis::Client::open(format!("redis://127.0.0.1:{}/", port)).unwrap(); @@ -47,14 +47,14 @@ fn test_redis_implementation() { let _: () = con.set("key", "value").unwrap(); let exists: bool = con.exists("key").unwrap(); - assert_eq!(exists, true); + assert!(exists); let x: String = con.get("key").unwrap(); assert_eq!(x, "value"); let x: RedisResult = con.get("not-existing-key"); - assert_eq!(x.is_err(), true); + assert!(x.is_err()); let exists: bool = con.exists("non-existant-key").unwrap(); - assert_eq!(exists, false); + assert!(!exists); let x: u32 = con.del("key").unwrap(); assert_eq!(x, 1); @@ -315,7 +315,7 @@ fn llen() { let (server, mut con) = get_redis_client_connection(3377); let values = &["val1", "val2", "val3", "val4"]; - let x = con + let _ = con .rpush::<&'static str, &[&str], u32>("lkey", values) .unwrap(); @@ -412,7 +412,7 @@ fn rpop_lpop() { let y: String = con.lpop("listkey").unwrap(); assert_eq!(y, "val1"); let z: bool = con.exists("listkey").unwrap(); - assert_eq!(z, false); + assert!(!z); assert_eq!(server.stop(), Some(ServerState::Stopped)); } @@ -473,7 +473,7 @@ fn ltrim_lrem_rpoplpush() { let b: String = con.rpoplpush("src", "dest").unwrap(); assert_eq!(b, "val1"); let c: bool = con.exists("src").unwrap(); - assert_eq!(c, false); + assert!(!c); assert_eq!(server.stop(), Some(ServerState::Stopped)); } @@ -502,14 +502,14 @@ fn sadd_scard_srem() { #[test] #[serial] fn start_and_stop_server() { - let server = Server::new(InMemoryStorage::new(), 3340); + let server = Server::new(InMemoryStorage::default(), 3340); assert_eq!(server.start(), Some(ServerState::Started)); assert_eq!(server.stop(), Some(ServerState::Stopped)); } #[test] fn start_and_stop_server_multiple_times() { - let server = Server::new(InMemoryStorage::new(), 3341); + let server = Server::new(InMemoryStorage::default(), 3341); for _ in 0..9 { assert_eq!(server.start(), Some(ServerState::Started)); assert_eq!(server.stop(), Some(ServerState::Stopped)); diff --git a/redisless/src/server/util/mod.rs b/redisless/src/server/util/mod.rs index 58c0e97..1f314aa 100644 --- a/redisless/src/server/util/mod.rs +++ b/redisless/src/server/util/mod.rs @@ -87,7 +87,7 @@ pub fn handle_request( } let res = run_command_and_get_response(storage, &buf); - let quit = if res.is_quit() { true } else { false }; + let quit = res.is_quit(); let reply = res.reply(); //eprintln!("?{}", std::str::from_utf8(&reply).unwrap()); let _ = stream.write(&reply); diff --git a/redisless/src/server/util/run_command.rs b/redisless/src/server/util/run_command.rs index 1da775d..d5d7456 100644 --- a/redisless/src/server/util/run_command.rs +++ b/redisless/src/server/util/run_command.rs @@ -3,8 +3,6 @@ use std::{ sync::{Arc, Mutex}, }; -use chrono::format::format; - use crate::{ command::Command, protocol::response::{RedisResponse, RedisResponseType}, @@ -274,7 +272,7 @@ pub fn run_command_and_get_response( let values = storage.lread(&key).unwrap().to_vec(); let len = values.len() as i64; if index < 0 { - index = index + len; + index += len; } if index < 0 || index >= len { return RedisResponse::single(Nil); @@ -297,7 +295,7 @@ pub fn run_command_and_get_response( let mut values = storage.lread(&key).unwrap().to_vec(); let len = values.len() as i64; if index < 0 { - index = index + len; + index += len; } if index < 0 || index >= len { return RedisResponse::error(RedisCommandError::IndexOutOfRange); @@ -323,7 +321,7 @@ pub fn run_command_and_get_response( match index { Some(mut i) => { if place == b"AFTER" { - i = i + 1; + i += 1; } values.insert(i, value); let len = values.len(); @@ -347,10 +345,10 @@ pub fn run_command_and_get_response( let mut start = start; let mut stop = stop; if start < 0 { - start = start + len; + start += len; } if stop < 0 { - stop = stop + len; + stop += len; } if start < 0 { start = 0; @@ -493,7 +491,7 @@ pub fn run_command_and_get_response( let mut rem = 0; for v in values { if vals.remove(&v) { - rem = rem + 1; + rem += 1; } } storage.swrite(&key, vals); diff --git a/redisless/src/storage/in_memory.rs b/redisless/src/storage/in_memory.rs index 4eb95bf..202c228 100644 --- a/redisless/src/storage/in_memory.rs +++ b/redisless/src/storage/in_memory.rs @@ -13,8 +13,8 @@ pub struct InMemoryStorage { hash_store: HashMap, } -impl InMemoryStorage { - pub fn new() -> Self { +impl Default for InMemoryStorage { + fn default() -> Self { Self { data_mapper: HashMap::new(), string_store: HashMap::new(), @@ -149,8 +149,7 @@ impl Storage for InMemoryStorage { None } false => { - let values = self.list_store.get(key); - values + self.list_store.get(key) } } } else { @@ -172,8 +171,7 @@ impl Storage for InMemoryStorage { None } false => { - let values = self.set_store.get(key); - values + self.set_store.get(key) } } } else { diff --git a/redisless/src/storage/tests/mod.rs b/redisless/src/storage/tests/mod.rs index 9a44387..83303aa 100644 --- a/redisless/src/storage/tests/mod.rs +++ b/redisless/src/storage/tests/mod.rs @@ -5,7 +5,7 @@ use crate::storage::{in_memory::InMemoryStorage, models::Expiry}; #[test] fn test_in_memory_storage() { - let mut mem = InMemoryStorage::new(); + let mut mem = InMemoryStorage::default(); mem.write(b"key", b"xxx"); assert_eq!(mem.read(b"key"), Some(&b"xxx"[..])); assert_eq!(mem.remove(b"key"), 1); @@ -15,7 +15,7 @@ fn test_in_memory_storage() { #[test] fn test_dbsize() { - let mut mem = InMemoryStorage::new(); + let mut mem = InMemoryStorage::default(); mem.write(b"key", b"xxx"); assert_eq!(mem.size(), 1); assert_eq!(mem.remove(b"key"), 1); @@ -29,7 +29,7 @@ fn test_dbsize() { #[test] fn test_expire() { - let mut mem = InMemoryStorage::new(); + let mut mem = InMemoryStorage::default(); let duration: u64 = 4; mem.write(b"key", b"xxx"); @@ -54,17 +54,17 @@ fn test_expire() { #[test] fn contains() { - let mut mem = InMemoryStorage::new(); + let mut mem = InMemoryStorage::default(); mem.write(b"key1", b"value1"); let x = mem.contains(b"key1"); - assert_eq!(x, true); + assert!(x); let x = mem.contains(b"key2"); - assert_eq!(x, false); + assert!(!x); } #[test] fn extend() { - let mut mem = InMemoryStorage::new(); + let mut mem = InMemoryStorage::default(); mem.write(b"key1", b"val"); let len = mem.extend(b"key1", b"ue1"); assert_eq!(len, 6); diff --git a/redisless/src/tests/mod.rs b/redisless/src/tests/mod.rs index 37cc4e6..e28e578 100644 --- a/redisless/src/tests/mod.rs +++ b/redisless/src/tests/mod.rs @@ -8,7 +8,7 @@ use crate::{ #[test] #[serial] fn start_and_stop_server_from_c_binding() { - let port = 4444 as u16; + let port = 4444_u16; let server = unsafe { redisless_server_new(port) }; unsafe { From f3411bf2c409ab48423bc65e0f7614706753e14e Mon Sep 17 00:00:00 2001 From: Dalot Date: Wed, 18 Aug 2021 09:21:27 +0100 Subject: [PATCH 2/2] added cfg(test) to the tests module --- redisless/Cargo.toml | 1 - redisless/src/cluster/mod.rs | 1 + redisless/src/cluster/tests.rs | 7 +------ redisless/src/lib.rs | 3 --- 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/redisless/Cargo.toml b/redisless/Cargo.toml index 176f039..f673324 100644 --- a/redisless/Cargo.toml +++ b/redisless/Cargo.toml @@ -25,7 +25,6 @@ ipnet = "2.3" chrono = "0.4" [dev-dependencies] -cfg-if = "1.0.0" redis = "0.20" serial_test = "0.5" criterion = "0.3" diff --git a/redisless/src/cluster/mod.rs b/redisless/src/cluster/mod.rs index c85b4a4..df11e70 100644 --- a/redisless/src/cluster/mod.rs +++ b/redisless/src/cluster/mod.rs @@ -1,4 +1,5 @@ mod node; pub mod peer; +#[cfg(test)] mod tests; mod util; diff --git a/redisless/src/cluster/tests.rs b/redisless/src/cluster/tests.rs index 3adb2e4..e2a9bcf 100644 --- a/redisless/src/cluster/tests.rs +++ b/redisless/src/cluster/tests.rs @@ -1,6 +1,3 @@ -#[cfg(test)] -cfg_if::cfg_if! { - if #[cfg(test)] { use std::net::{IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4}; use crate::cluster::node::ClusterNode; use crate::cluster::peer::{Peer, PeersDiscovery, DEFAULT_NODE_LISTENING_PORT}; @@ -8,9 +5,7 @@ use crate::cluster::util::{ get_ip_addresses, get_local_network_ip_addresses, get_range_from_ip_address, scan_ip_range, Range, }; -} else { - } -} + #[test] fn start_cluster() { diff --git a/redisless/src/lib.rs b/redisless/src/lib.rs index f221841..f0fbbc7 100644 --- a/redisless/src/lib.rs +++ b/redisless/src/lib.rs @@ -2,9 +2,6 @@ #[macro_use] extern crate serial_test; -#[cfg(test)] -extern crate cfg_if; - use storage::in_memory::InMemoryStorage; use crate::server::{Server, ServerState};