Fix cargo clippy errors

Dont apply clippy for tests for now

Fix clippy warnings of filter-parser package

parent 8352febd646ec4bcf56a44161e5c4dce0e55111f
author unvalley <38400669+unvalley@users.noreply.github.com> 1666325847 +0900
committer unvalley <kirohi.code@gmail.com> 1666791316 +0900

Update .github/workflows/rust.yml

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>

Allow clippy lint too_many_argments

Allow clippy lint needless_collect

Allow clippy lint too_many_arguments and type_complexity

Fix for clippy warnings comparison_chains

Fix for clippy warnings vec_init_then_push

Allow clippy lint should_implement_trait

Allow clippy lint drop_non_drop

Fix lifetime clipy warnings in filter-paprser

Execute cargo fmt

Fix clippy remaining warnings

Fix clippy remaining warnings again and allow lint on each place

.github/workflows/rust.yml

@@ -65,7 +65,6 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: clippy
-          args: --all-targets
 
   fmt:
     name: Run Rustfmt

filter-parser/src/condition.rs

@@ -48,17 +48,14 @@ pub fn parse_condition(input: Span) -> IResult<FilterCondition> {
 pub fn parse_exists(input: Span) -> IResult<FilterCondition> {
     let (input, key) = terminated(parse_value, tag("EXISTS"))(input)?;
 
-    Ok((input, FilterCondition::Condition { fid: key.into(), op: Exists }))
+    Ok((input, FilterCondition::Condition { fid: key, op: Exists }))
 }
 /// exist          = value "NOT" WS+ "EXISTS"
 pub fn parse_not_exists(input: Span) -> IResult<FilterCondition> {
     let (input, key) = parse_value(input)?;
 
     let (input, _) = tuple((tag("NOT"), multispace1, tag("EXISTS")))(input)?;
-    Ok((
+    Ok((input, FilterCondition::Not(Box::new(FilterCondition::Condition { fid: key, op: Exists }))))
-        input,
-        FilterCondition::Not(Box::new(FilterCondition::Condition { fid: key.into(), op: Exists })),
-    ))
 }
 
 /// to             = value value "TO" WS+ value

filter-parser/src/lib.rs

@@ -168,7 +168,7 @@ fn ws<'a, O>(inner: impl FnMut(Span<'a>) -> IResult<O>) -> impl FnMut(Span<'a>)
 }
 
 /// value_list = (value ("," value)* ","?)?
-fn parse_value_list<'a>(input: Span<'a>) -> IResult<Vec<Token<'a>>> {
+fn parse_value_list(input: Span) -> IResult<Vec<Token>> {
     let (input, first_value) = opt(parse_value)(input)?;
     if let Some(first_value) = first_value {
         let value_list_el_parser = preceded(ws(tag(",")), parse_value);
@@ -335,17 +335,17 @@ fn parse_error_reserved_keyword(input: Span) -> IResult<FilterCondition> {
         Ok(result) => Ok(result),
         Err(nom::Err::Error(inner) | nom::Err::Failure(inner)) => match inner.kind() {
             ErrorKind::ExpectedValue(ExpectedValueKind::ReservedKeyword) => {
-                return Err(nom::Err::Failure(inner));
+                Err(nom::Err::Failure(inner))
             }
-            _ => return Err(nom::Err::Error(inner)),
+            _ => Err(nom::Err::Error(inner)),
         },
-        Err(e) => {
+        Err(e) => Err(e),
-            return Err(e);
-        }
     }
 }
 
-/// primary        = (WS* "(" WS* expression WS* ")" WS*) | geoRadius | condition | exists | not_exists | to
+/**
+primary        = (WS* "(" WS* expression WS* ")" WS*) | geoRadius | condition | exists | not_exists | to
+*/
 fn parse_primary(input: Span, depth: usize) -> IResult<FilterCondition> {
     if depth > MAX_FILTER_DEPTH {
         return Err(nom::Err::Error(Error::new_from_kind(input, ErrorKind::DepthLimitReached)));

filter-parser/src/value.rs

@@ -78,7 +78,7 @@ pub fn word_exact<'a, 'b: 'a>(tag: &'b str) -> impl Fn(Span<'a>) -> IResult<'a,
 }
 
 /// value          = WS* ( word | singleQuoted | doubleQuoted) WS+
-pub fn parse_value<'a>(input: Span<'a>) -> IResult<Token<'a>> {
+pub fn parse_value(input: Span) -> IResult<Token> {
     // to get better diagnostic message we are going to strip the left whitespaces from the input right now
     let (input, _) = take_while(char::is_whitespace)(input)?;
 

milli/src/lib.rs

@@ -1,6 +1,4 @@
 #![cfg_attr(all(test, fuzzing), feature(no_coverage))]
-#![allow(clippy::reversed_empty_ranges)]
-#![allow(clippy::too_many_arguments)]
 #[macro_use]
 pub mod documents;
 

milli/src/search/criteria/asc_desc.rs

@@ -242,6 +242,7 @@ fn iterative_facet_number_ordered_iter<'t>(
     // The itertools GroupBy iterator doesn't provide an owned version, we are therefore
     // required to collect the result into an owned collection (a Vec).
     // https://github.com/rust-itertools/itertools/issues/499
+    #[allow(clippy::needless_collect)]
     let vec: Vec<_> = iter
         .group_by(|(_, v)| *v)
         .into_iter()
@@ -284,6 +285,7 @@ fn iterative_facet_string_ordered_iter<'t>(
     // The itertools GroupBy iterator doesn't provide an owned version, we are therefore
     // required to collect the result into an owned collection (a Vec).
     // https://github.com/rust-itertools/itertools/issues/499
+    #[allow(clippy::needless_collect)]
     let vec: Vec<_> = iter
         .group_by(|(_, v)| *v)
         .into_iter()

milli/src/search/criteria/attribute.rs

@@ -179,6 +179,7 @@ impl<'t> Criterion for Attribute<'t> {
 /// QueryPositionIterator is an Iterator over positions of a Query,
 /// It contains iterators over words positions.
 struct QueryPositionIterator<'t> {
+    #[allow(clippy::type_complexity)]
     inner:
         Vec<Peekable<Box<dyn Iterator<Item = heed::Result<((&'t str, u32), RoaringBitmap)>> + 't>>>,
 }

milli/src/search/criteria/mod.rs

@@ -96,6 +96,7 @@ pub trait Context<'c> {
         &self,
         docid: DocumentId,
     ) -> heed::Result<HashMap<String, RoaringBitmap>>;
+    #[allow(clippy::type_complexity)]
     fn word_position_iterator(
         &self,
         word: &str,
@@ -610,11 +611,7 @@ fn query_pair_proximity_docids(
         }
         (QueryKind::Exact { word: left, .. }, QueryKind::Tolerant { typo, word: right }) => {
             let r_words = word_derivations(right, prefix, *typo, ctx.words_fst(), wdcache)?;
-<<<<<<< HEAD
             all_word_pair_overall_proximity_docids(ctx, &[(left, 0)], r_words, proximity)
-=======
-            all_word_pair_proximity_docids(ctx, &[(left, 0)], r_words, proximity)
->>>>>>> 08fe530b (Execute cargo clippy --fix)
         }
         (
             QueryKind::Tolerant { typo: l_typo, word: left },

milli/src/search/distinct/facet_distinct.rs

@@ -123,6 +123,7 @@ impl<'a> FacetDistinctIter<'a> {
     }
 }
 
+#[allow(clippy::drop_non_drop)]
 fn facet_values_prefix_key(distinct: FieldId, id: DocumentId) -> [u8; FID_SIZE + DOCID_SIZE] {
     concat_arrays!(distinct.to_be_bytes(), id.to_be_bytes())
 }

milli/src/search/facet/filter.rs

@@ -100,10 +100,10 @@ impl<'a> Filter<'a> {
                         }
                     }
 
-                    if ors.len() > 1 {
+                    match ors.len() {
-                        ands.push(FilterCondition::Or(ors));
+                        1 => ands.push(ors.pop().unwrap()),
-                    } else if ors.len() == 1 {
+                        n if n > 1 => ands.push(FilterCondition::Or(ors)),
-                        ands.push(ors.pop().unwrap());
+                        _ => (),
                     }
                 }
                 Either::Right(rule) => {
@@ -128,6 +128,7 @@ impl<'a> Filter<'a> {
         Ok(Some(Self { condition: and }))
     }
 
+    #[allow(clippy::should_implement_trait)]
     pub fn from_str(expression: &'a str) -> Result<Option<Self>> {
         let condition = match FilterCondition::parse(expression) {
             Ok(Some(fc)) => Ok(fc),

milli/src/search/matches/mod.rs

@@ -125,10 +125,7 @@ impl<'t, A: AsRef<[u8]>> Matcher<'t, '_, A> {
             words_positions: &mut impl Iterator<Item = (usize, usize, &'a Token<'a>)>,
             matches: &mut Vec<Match>,
         ) -> bool {
-            let mut potential_matches = Vec::new();
+            let mut potential_matches = vec![(token_position, word_position, partial.char_len())];
-
-            // Add first match to potential matches.
-            potential_matches.push((token_position, word_position, partial.char_len()));
 
             for (token_position, word_position, word) in words_positions {
                 partial = match partial.match_token(word) {

milli/src/snapshot_tests.rs

@@ -314,7 +314,7 @@ pub fn snap_field_id_docid_facet_strings(index: &Index) -> String {
 pub fn snap_documents_ids(index: &Index) -> String {
     let rtxn = index.read_txn().unwrap();
     let documents_ids = index.documents_ids(&rtxn).unwrap();
-    
+
     display_bitmap(&documents_ids)
 }
 pub fn snap_stop_words(index: &Index) -> String {
@@ -326,7 +326,7 @@ pub fn snap_stop_words(index: &Index) -> String {
 pub fn snap_soft_deleted_documents_ids(index: &Index) -> String {
     let rtxn = index.read_txn().unwrap();
     let soft_deleted_documents_ids = index.soft_deleted_documents_ids(&rtxn).unwrap();
-    
+
     display_bitmap(&soft_deleted_documents_ids)
 }
 pub fn snap_field_distributions(index: &Index) -> String {
@@ -350,7 +350,7 @@ pub fn snap_fields_ids_map(index: &Index) -> String {
 pub fn snap_geo_faceted_documents_ids(index: &Index) -> String {
     let rtxn = index.read_txn().unwrap();
     let geo_faceted_documents_ids = index.geo_faceted_documents_ids(&rtxn).unwrap();
-    
+
     display_bitmap(&geo_faceted_documents_ids)
 }
 pub fn snap_external_documents_ids(index: &Index) -> String {

milli/src/update/available_documents_ids.rs

@@ -21,6 +21,7 @@ impl AvailableDocumentsIds {
 
                 let iter = match last_id.checked_add(1) {
                     Some(id) => id..=u32::max_value(),
+                    #[allow(clippy::reversed_empty_ranges)]
                     None => 1..=0, // empty range iterator
                 };
 

milli/src/update/index_documents/extract/extract_geo_points.rs

@@ -51,6 +51,7 @@ pub fn extract_geo_points<R: io::Read + io::Seek>(
             )
             .map_err(|lng| GeoError::BadLongitude { document_id: document_id(), value: lng })?;
 
+            #[allow(clippy::drop_non_drop)]
             let bytes: [u8; 16] = concat_arrays![lat.to_ne_bytes(), lng.to_ne_bytes()];
             writer.insert(docid_bytes, bytes)?;
         } else if lat.is_none() && lng.is_some() {

milli/src/update/index_documents/extract/mod.rs

@@ -33,6 +33,7 @@ use crate::{FieldId, Result};
 
 /// Extract data for each databases from obkv documents in parallel.
 /// Send data in grenad file over provided Sender.
+#[allow(clippy::too_many_arguments)]
 pub(crate) fn data_from_obkv_documents(
     original_obkv_chunks: impl Iterator<Item = Result<grenad::Reader<File>>> + Send,
     flattened_obkv_chunks: impl Iterator<Item = Result<grenad::Reader<File>>> + Send,
@@ -53,6 +54,7 @@ pub(crate) fn data_from_obkv_documents(
         })
         .collect::<Result<()>>()?;
 
+    #[allow(clippy::type_complexity)]
     let result: Result<(Vec<_>, (Vec<_>, (Vec<_>, Vec<_>)))> = flattened_obkv_chunks
         .par_bridge()
         .map(|flattened_obkv_chunks| {
@@ -217,6 +219,8 @@ fn send_original_documents_data(
 /// - docid_fid_facet_numbers
 /// - docid_fid_facet_strings
 /// - docid_fid_facet_exists
+#[allow(clippy::too_many_arguments)]
+#[allow(clippy::type_complexity)]
 fn send_and_extract_flattened_documents_data(
     flattened_documents_chunk: Result<grenad::Reader<File>>,
     indexer: GrenadParameters,

milli/src/update/index_documents/mod.rs

@@ -598,6 +598,7 @@ where
 }
 
 /// Run the word prefix docids update operation.
+#[allow(clippy::too_many_arguments)]
 fn execute_word_prefix_docids(
     txn: &mut heed::RwTxn,
     reader: grenad::Reader<Cursor<ClonableMmap>>,

milli/src/update/prefix_word_pairs/prefix_word.rs

@@ -12,6 +12,7 @@ use crate::update::prefix_word_pairs::{
 };
 use crate::{CboRoaringBitmapCodec, Result, U8StrStrCodec, UncheckedU8StrStrCodec};
 
+#[allow(clippy::too_many_arguments)]
 #[logging_timer::time]
 pub fn index_prefix_word_database(
     wtxn: &mut heed::RwTxn,
@@ -38,8 +39,7 @@ pub fn index_prefix_word_database(
 
     for proximity in 1..max_proximity {
         for prefix in common_prefixes.iter() {
-            let mut prefix_key = vec![];
+            let mut prefix_key = vec![proximity];
-            prefix_key.push(proximity);
             prefix_key.extend_from_slice(prefix.as_bytes());
             let mut cursor = new_word_pair_proximity_docids.clone().into_prefix_iter(prefix_key)?;
             // This is the core of the algorithm
@@ -84,8 +84,7 @@ pub fn index_prefix_word_database(
 
     for proximity in 1..max_proximity {
         for prefix in new_prefixes.iter() {
-            let mut prefix_key = vec![];
+            let mut prefix_key = vec![proximity];
-            prefix_key.push(proximity);
             prefix_key.extend_from_slice(prefix.as_bytes());
             let mut db_iter = word_pair_proximity_docids
                 .as_polymorph()

milli/src/update/prefix_word_pairs/word_prefix.rs

@@ -176,6 +176,7 @@ use crate::update::prefix_word_pairs::{
 };
 use crate::{CboRoaringBitmapCodec, Result, U8StrStrCodec, UncheckedU8StrStrCodec};
 
+#[allow(clippy::too_many_arguments)]
 #[logging_timer::time]
 pub fn index_word_prefix_database(
     wtxn: &mut heed::RwTxn,
@@ -385,6 +386,7 @@ can be inserted into the database in sorted order. When it is flushed, it calls
 struct PrefixAndProximityBatch {
     proximity: u8,
     word1: Vec<u8>,
+    #[allow(clippy::type_complexity)]
     batch: Vec<(Vec<u8>, Vec<Cow<'static, [u8]>>)>,
 }
 

milli/tests/search/query_criteria.rs

@@ -200,12 +200,14 @@ test_criterion!(
 #[test]
 fn criteria_mixup() {
     use Criterion::*;
-    let index = search::setup_search_index_with_criteria(&[Words,
+    let index = search::setup_search_index_with_criteria(&[
+        Words,
         Attribute,
         Desc(S("asc_desc_rank")),
         Exactness,
         Proximity,
-        Typo]);
+        Typo,
+    ]);
 
     #[rustfmt::skip]
     let criteria_mix = {