From fbedc3794ab3662133058ef0d4459010de3d091a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E6=9E=97=E4=BC=9F?= Date: Sun, 29 Oct 2023 00:45:14 +0800 Subject: [PATCH] Enable clippy in ci (#55) * Improve build * Fix ci * Enable clippy workflow * Fix conflict * Install protoc before clippy * Fix lints * Fix lints * fmt * Fix lints --- .github/workflows/ci.yml | 19 ++++------ src/bin/cli.rs | 2 +- src/kernel/lsm/compactor.rs | 6 +-- src/kernel/lsm/iterator/level_iter.rs | 6 +-- src/kernel/lsm/log.rs | 5 +-- src/kernel/lsm/mem_table.rs | 12 +++--- src/kernel/lsm/mvcc.rs | 41 +++++++++------------ src/kernel/lsm/table/loader.rs | 8 ++-- src/kernel/lsm/table/ss_table/block.rs | 10 ++--- src/kernel/lsm/table/ss_table/block_iter.rs | 8 ++-- src/kernel/lsm/table/ss_table/iter.rs | 6 +-- src/kernel/lsm/table/ss_table/mod.rs | 16 +++----- src/kernel/lsm/version/mod.rs | 1 + src/kernel/rocksdb_storage.rs | 2 +- src/kernel/utils/lru_cache.rs | 1 + src/server/mod.rs | 1 + 16 files changed, 65 insertions(+), 79 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f01fec1..09947bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,26 +9,21 @@ env: CARGO_REGISTRIES_MY_REGISTRY_INDEX: https://github.com/rust-lang/crates.io-index jobs: - fmt: - runs-on: ubuntu-20.04 + lint: + runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1 + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@nightly with: - profile: minimal - toolchain: nightly components: rustfmt, clippy - name: Check code format uses: actions-rs/cargo@v1 with: command: fmt args: --all -- --check -# TODO follow up pr to fix clippy lints -# - name: Run cargo clippy -# uses: actions-rs/cargo@v1 -# with: -# command: clippy -# args: -- -D warnings + - name: Install Protoc + uses: arduino/setup-protoc@v2 + - run: cargo clippy --all-targets --all-features -- -D warnings build: diff --git a/src/bin/cli.rs b/src/bin/cli.rs index 6382dbc..a822e9b 100644 --- a/src/bin/cli.rs +++ b/src/bin/cli.rs @@ -83,7 +83,7 @@ async fn main() -> ConnectionResult<()> { .batch_get(keys) .await? .into_iter() - .map(|value| decode(value)) + .map(decode) .collect_vec() ) } diff --git a/src/kernel/lsm/compactor.rs b/src/kernel/lsm/compactor.rs index dfb0ff4..7d9ecd2 100644 --- a/src/kernel/lsm/compactor.rs +++ b/src/kernel/lsm/compactor.rs @@ -386,7 +386,7 @@ mod tests { || !level_slice[6].is_empty() ); - for (level, slice) in level_slice.into_iter().enumerate() { + for (level, slice) in level_slice.iter().enumerate() { if !slice.is_empty() && level != LEVEL_0 { let mut tmp_scope: Option<&Scope> = None; @@ -402,8 +402,8 @@ mod tests { assert_eq!(kv_store.len().await?, times); let start = Instant::now(); - for i in 0..times { - assert_eq!(kv_store.get(&vec_kv[i].0).await?, Some(vec_kv[i].1.clone())); + for kv in vec_kv.iter().take(times) { + assert_eq!(kv_store.get(&kv.0).await?, Some(kv.1.clone())); } println!("[get_for][Time: {:?}]", start.elapsed()); kv_store.flush().await?; diff --git a/src/kernel/lsm/iterator/level_iter.rs b/src/kernel/lsm/iterator/level_iter.rs index 48f87da..1569c22 100644 --- a/src/kernel/lsm/iterator/level_iter.rs +++ b/src/kernel/lsm/iterator/level_iter.rs @@ -136,7 +136,7 @@ mod tests { ver_status .loader() .create(2, slice_2.to_vec(), 1, TableType::Skip)?; - let fusion_meta = TableMeta::fusion(&vec![meta_1, meta_2]); + let fusion_meta = TableMeta::fusion(&[meta_1, meta_2]); let vec_edit = vec![ // Tips: 由于level 0只是用于测试seek是否发生错误，因此可以忽略此处重复使用 @@ -156,8 +156,8 @@ mod tests { let version = ver_status.current().await; let mut iterator = LevelIter::new(&version, 1)?; - for i in 0..times { - assert_eq!(iterator.try_next()?.unwrap(), vec_data[i]); + for kv in vec_data.iter().take(times) { + assert_eq!(iterator.try_next()?.unwrap(), kv.clone()); } assert_eq!( diff --git a/src/kernel/lsm/log.rs b/src/kernel/lsm/log.rs index a7dfe6c..529f9b0 100644 --- a/src/kernel/lsm/log.rs +++ b/src/kernel/lsm/log.rs @@ -298,8 +298,7 @@ mod tests { "and my third", ]; - let mut dst = Vec::new(); - dst.resize(1024, 0u8); + let mut dst = vec![0; 1024]; { let mut lw = LogWriter::new(Cursor::new(dst.as_mut_slice())); @@ -355,7 +354,7 @@ mod tests { loop { let r = lr.read(&mut dst); - if !r.is_ok() { + if r.is_err() { panic!("{}", r.unwrap_err()); } else if r.unwrap() == 0 { break; diff --git a/src/kernel/lsm/mem_table.rs b/src/kernel/lsm/mem_table.rs index 45573be..a96ab3b 100644 --- a/src/kernel/lsm/mem_table.rs +++ b/src/kernel/lsm/mem_table.rs @@ -346,7 +346,7 @@ impl MemTable { ._immut .as_ref() .map(|mem_map| Self::_range_scan(mem_map, min, max, option_seq)) - .unwrap_or(vec![]) + .unwrap_or_default() .into_iter() .chain(Self::_range_scan(&inner._mem, min, max, option_seq)) .rev() @@ -451,26 +451,26 @@ mod tests { let old_seq_id = Sequence::create(); assert_eq!( - mem_table.find(&vec![b'k']), + mem_table.find(&[b'k']), Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'1'])))) ); let _ = mem_table.insert_data(data_2)?; assert_eq!( - mem_table.find(&vec![b'k']), + mem_table.find(&[b'k']), Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'2'])))) ); assert_eq!( - mem_table.find_with_sequence_id(&vec![b'k'], old_seq_id), + mem_table.find_with_sequence_id(&[b'k'], old_seq_id), Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'1'])))) ); let new_seq_id = Sequence::create(); assert_eq!( - mem_table.find_with_sequence_id(&vec![b'k'], new_seq_id), + mem_table.find_with_sequence_id(&[b'k'], new_seq_id), Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'2'])))) ); @@ -632,7 +632,7 @@ mod tests { assert_eq!(iter.try_next()?, None); assert_eq!( - iter.seek(Seek::Backward(&vec![b'3']))?, + iter.seek(Seek::Backward(&[b'3']))?, Some((key_4_2.key.clone(), None)) ); diff --git a/src/kernel/lsm/mvcc.rs b/src/kernel/lsm/mvcc.rs index c3d7a8d..5b9a562 100644 --- a/src/kernel/lsm/mvcc.rs +++ b/src/kernel/lsm/mvcc.rs @@ -40,7 +40,7 @@ pub struct Transaction { impl Transaction { fn write_buf_or_init(&mut self) -> &mut SkipMap> { - self.write_buf.get_or_insert_with(|| SkipMap::new()) + self.write_buf.get_or_insert_with(SkipMap::new) } /// 通过Key获取对应的Value @@ -118,7 +118,8 @@ impl Transaction { } fn _mem_range(&self, min: Bound<&[u8]>, max: Bound<&[u8]>) -> Option { - if let Some(buf) = &self.write_buf { + #[allow(clippy::option_map_or_none)] + self.write_buf.as_ref().map_or(None, |buf| { Some( buf.range( min.map(Bytes::copy_from_slice).as_ref(), @@ -126,9 +127,7 @@ impl Transaction { ) .map(|(key, value)| (key.clone(), value.clone())), ) - } else { - None - } + }) } fn mem_table(&self) -> &MemTable { @@ -325,38 +324,34 @@ mod tests { } // 模拟数据分布在MemTable以及SSTable中 - for i in 0..50 { - kv_store - .set(vec_kv[i].0.clone(), vec_kv[i].1.clone()) - .await?; + for kv in vec_kv.iter().take(50) { + kv_store.set(kv.0.clone(), kv.1.clone()).await?; } kv_store.flush().await?; - for i in 50..100 { - kv_store - .set(vec_kv[i].0.clone(), vec_kv[i].1.clone()) - .await?; + for kv in vec_kv.iter().take(100).skip(50) { + kv_store.set(kv.0.clone(), kv.1.clone()).await?; } let mut tx_1 = kv_store.new_transaction().await; - for i in 100..times { - tx_1.set(vec_kv[i].0.clone(), vec_kv[i].1.clone()); + for kv in vec_kv.iter().take(times).skip(100) { + tx_1.set(kv.0.clone(), kv.1.clone()); } tx_1.remove(&vec_kv[times - 1].0)?; // 事务在提交前事务可以读取到自身以及Store已写入的数据 - for i in 0..times - 1 { - assert_eq!(tx_1.get(&vec_kv[i].0)?, Some(vec_kv[i].1.clone())); + for kv in vec_kv.iter().take(times - 1) { + assert_eq!(tx_1.get(&kv.0)?, Some(kv.1.clone())); } assert_eq!(tx_1.get(&vec_kv[times - 1].0)?, None); // 事务在提交前Store不应该读取到事务中的数据 - for i in 100..times { - assert_eq!(kv_store.get(&vec_kv[i].0).await?, None); + for kv in vec_kv.iter().take(times).skip(100) { + assert_eq!(kv_store.get(&kv.0).await?, None); } let vec_test = vec_kv[25..] @@ -368,17 +363,17 @@ mod tests { let mut iter = tx_1.iter(Bound::Included(&vec_kv[25].0), Bound::Unbounded)?; // -1是因为最后一个元素在之前tx中删除了，因此为None - for i in 0..vec_test.len() - 1 { + for kv in vec_test.iter().take(vec_test.len() - 1) { // 元素太多，因此这里就单个对比，否则会导致报错时日志过多 - assert_eq!(iter.try_next()?.unwrap(), vec_test[i]); + assert_eq!(iter.try_next()?.unwrap(), kv.clone()); } drop(iter); tx_1.commit().await?; - for i in 0..times - 1 { - assert_eq!(kv_store.get(&vec_kv[i].0).await?, Some(vec_kv[i].1.clone())); + for kv in vec_kv.iter().take(times - 1) { + assert_eq!(kv_store.get(&kv.0).await?, Some(kv.1.clone())); } Ok(()) diff --git a/src/kernel/lsm/table/loader.rs b/src/kernel/lsm/table/loader.rs index 949c6d2..35d3a1d 100644 --- a/src/kernel/lsm/table/loader.rs +++ b/src/kernel/lsm/table/loader.rs @@ -209,9 +209,9 @@ mod tests { ss_table_loaded.query(&repeat_data.0)?, Some(repeat_data.clone()) ); - for i in 1..times { + for kv in vec_data.iter().take(times).skip(1) { assert_eq!( - ss_table_loaded.query(&vec_data[i].0)?.unwrap().1, + ss_table_loaded.query(&kv.0)?.unwrap().1, Some(value.clone()) ) } @@ -228,9 +228,9 @@ mod tests { ss_table_backup.query(&repeat_data.0)?, Some(repeat_data.clone()) ); - for i in 1..times { + for kv in vec_data.iter().take(times).skip(1) { assert_eq!( - ss_table_backup.query(&vec_data[i].0)?.unwrap().1, + ss_table_backup.query(&kv.0)?.unwrap().1, Some(value.clone()) ) } diff --git a/src/kernel/lsm/table/ss_table/block.rs b/src/kernel/lsm/table/ss_table/block.rs index 1dddc02..2196f07 100644 --- a/src/kernel/lsm/table/ss_table/block.rs +++ b/src/kernel/lsm/table/ss_table/block.rs @@ -735,8 +735,8 @@ mod tests { let mut cache = LruCache::new(5)?; - for i in 0..times { - let key = &vec_data[i].0; + for kv in vec_data.iter().take(times) { + let key = &kv.0; let data_block = cache.get_or_insert(index_block.find_with_upper(key), |index| { let &Index { offset, len } = index; let target_block = Block::::decode( @@ -779,8 +779,8 @@ mod tests { let rand_max_idx: usize = rng.gen_range(2..all_item.len() - 1); let rand_min_idx: usize = rng.gen_range(0..rand_max_idx - 1); - let min_key = full_key(&block, &block.vec_entry, rand_min_idx); - let max_key = full_key(&block, &block.vec_entry, rand_max_idx); + let min_key = full_key(block, &block.vec_entry, rand_min_idx); + let max_key = full_key(block, &block.vec_entry, rand_max_idx); let excluded_vec = block.range(Bound::Excluded(&min_key), Bound::Excluded(&max_key)); @@ -803,7 +803,7 @@ mod tests { fn full_key( block: &Block, - all_entry: &Vec<(usize, Entry)>, + all_entry: &[(usize, Entry)], index: usize, ) -> Vec { let entry = &all_entry[index].1; diff --git a/src/kernel/lsm/table/ss_table/block_iter.rs b/src/kernel/lsm/table/ss_table/block_iter.rs index 59da158..577318a 100644 --- a/src/kernel/lsm/table/ss_table/block_iter.rs +++ b/src/kernel/lsm/table/ss_table/block_iter.rs @@ -171,7 +171,7 @@ mod tests { ); assert_eq!( - iterator.seek(Seek::Backward(&vec![b'2']))?, + iterator.seek(Seek::Backward(&[b'2']))?, Some(( Bytes::from(vec![b'2']), Value::from(Some(Bytes::from(vec![b'0']))) @@ -179,7 +179,7 @@ mod tests { ); assert_eq!( - iterator.seek(Seek::Backward(&vec![b'3']))?, + iterator.seek(Seek::Backward(&[b'3']))?, Some((Bytes::from(vec![b'4']), Value::from(None))) ); @@ -204,8 +204,8 @@ mod tests { tokio_test::block_on(async move { let mut iterator = BlockIter::new(&block); - for i in 0..times { - assert_eq!(iterator.try_next()?.unwrap(), vec_data[i]); + for kv in vec_data.iter().take(times) { + assert_eq!(iterator.try_next()?.unwrap(), kv.clone()); } for i in (0..times - 1).rev() { diff --git a/src/kernel/lsm/table/ss_table/iter.rs b/src/kernel/lsm/table/ss_table/iter.rs index 85ee4e4..dd234d2 100644 --- a/src/kernel/lsm/table/ss_table/iter.rs +++ b/src/kernel/lsm/table/ss_table/iter.rs @@ -32,7 +32,7 @@ impl<'a> SSTableIter<'a> { .cache .get_or_insert((ss_table.gen(), Some(index)), |(_, index)| { let index = (*index).ok_or_else(|| KernelError::DataEmpty)?; - Ok(ss_table.data_block(index)?) + ss_table.data_block(index) }) .map(|block_type| match block_type { BlockType::Data(data_block) => Some(data_block), @@ -154,8 +154,8 @@ mod tests { let mut iterator = SSTableIter::new(&ss_table)?; - for i in 0..times { - assert_eq!(iterator.try_next()?.unwrap(), vec_data[i]); + for kv in vec_data.iter().take(times) { + assert_eq!(iterator.try_next()?.unwrap(), kv.clone()); } for i in (0..times - 1).rev() { diff --git a/src/kernel/lsm/table/ss_table/mod.rs b/src/kernel/lsm/table/ss_table/mod.rs index ed11e3c..c9225d4 100644 --- a/src/kernel/lsm/table/ss_table/mod.rs +++ b/src/kernel/lsm/table/ss_table/mod.rs @@ -205,7 +205,7 @@ impl Table for SSTable { (self.gen(), Some(index_block.find_with_upper(key))), |(_, index)| { let index = (*index).ok_or_else(|| KernelError::DataEmpty)?; - Ok(Self::data_block(self, index)?) + Self::data_block(self, index) }, )? { if let (value, true) = data_block.find(key) { @@ -291,20 +291,14 @@ mod tests { let ss_table = sst_loader.get(1).unwrap(); - for i in 0..times { - assert_eq!( - ss_table.query(&vec_data[i].0)?.unwrap().1, - Some(value.clone()) - ) + for kv in vec_data.iter().take(times) { + assert_eq!(ss_table.query(&kv.0)?.unwrap().1, Some(value.clone())) } let cache = ShardingLruCache::new(config.table_cache_size, 16, RandomState::default())?; let ss_table = SSTable::load_from_file(sst_factory.reader(1, IoType::Direct)?, Arc::new(cache))?; - for i in 0..times { - assert_eq!( - ss_table.query(&vec_data[i].0)?.unwrap().1, - Some(value.clone()) - ) + for kv in vec_data.iter().take(times) { + assert_eq!(ss_table.query(&kv.0)?.unwrap().1, Some(value.clone())) } Ok(()) diff --git a/src/kernel/lsm/version/mod.rs b/src/kernel/lsm/version/mod.rs index f49c714..3aee049 100644 --- a/src/kernel/lsm/version/mod.rs +++ b/src/kernel/lsm/version/mod.rs @@ -178,6 +178,7 @@ impl Version { } } + #[allow(clippy::filter_map_bool_then)] self.level_slice .iter() .enumerate() diff --git a/src/kernel/rocksdb_storage.rs b/src/kernel/rocksdb_storage.rs index 9ac73a3..54ae505 100644 --- a/src/kernel/rocksdb_storage.rs +++ b/src/kernel/rocksdb_storage.rs @@ -35,7 +35,7 @@ impl Storage for RocksdbStorage { #[inline] async fn set(&self, key: Bytes, value: Bytes) -> crate::kernel::KernelResult<()> { - let _ignore = self.data_base.put(key.as_slice(), value.to_vec())?; + let _ignore = self.data_base.put(key.as_slice(), &value)?; Ok(()) } diff --git a/src/kernel/utils/lru_cache.rs b/src/kernel/utils/lru_cache.rs index 7b9d45d..a3d070b 100644 --- a/src/kernel/utils/lru_cache.rs +++ b/src/kernel/utils/lru_cache.rs @@ -177,6 +177,7 @@ impl ShardingLruCache { fn shard(&self, key: &K) -> Arc>> { let mut hasher = self.hasher.build_hasher(); key.hash(&mut hasher); + #[allow(clippy::manual_hash_one)] Arc::clone(&self.sharding_vec[hasher.finish() as usize % self.sharding_size()]) } } diff --git a/src/server/mod.rs b/src/server/mod.rs index c07f47e..9637303 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -1,2 +1,3 @@ pub mod client; +#[allow(clippy::module_inception)] pub mod server;