From 880b00ef3f27016127a3b7df97252b7ee63a9cd7 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Mon, 16 Oct 2023 18:54:41 -0400 Subject: [PATCH] Apply more suggestions. --- src/compare/diff.rs | 17 +++---- src/compare/sexp.rs | 32 +++++------- src/compare/util.rs | 76 +++++++++++++--------------- src/context/context.rs | 28 ++++------ src/context/file_access_interface.rs | 2 +- src/context/list.rs | 2 +- src/context/mod.rs | 1 + 7 files changed, 66 insertions(+), 92 deletions(-) diff --git a/src/compare/diff.rs b/src/compare/diff.rs index 7ad080e2..88f61641 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -227,7 +227,7 @@ impl<'b, 's> DiffResult<'b, 's> { status_text = status_text, name = self.name, char_offset = preceding_text.chars().count() + 1, - message = self.message.as_ref().map(|m| m.as_str()).unwrap_or("") + message = self.message.as_deref().unwrap_or("") ); for child in self.children.iter() { child.print_indented(indentation + 1, original_document)?; @@ -330,8 +330,8 @@ pub(crate) fn artificial_diff_scope<'b, 's>( .into()) } -pub(crate) fn artificial_owned_diff_scope<'b, 's, 'x>( - name: &'x str, +pub(crate) fn artificial_owned_diff_scope<'b, 's>( + name: &str, children: Vec>, ) -> Result, Box> { Ok(DiffLayer { @@ -427,12 +427,9 @@ pub(crate) fn compare_ast_node<'b, 's>( // PlainText is a special case because upstream Org-Mode uses relative values for the bounds in plaintext rather than absolute so the below checks do not account for that. if let AstNode::PlainText(_) = rust { } else { - match compare_standard_properties(source, emacs, &rust) { - Err(err) => { - compare_result.status = DiffStatus::Bad; - compare_result.message = Some(err.to_string()) - } - Ok(_) => {} + if let Err(err) = compare_standard_properties(source, emacs, &rust) { + compare_result.status = DiffStatus::Bad; + compare_result.message = Some(err.to_string()) } } @@ -495,7 +492,7 @@ fn _compare_document<'b, 's>( .map(EmacsField::Required), ( EmacsField::Required(":path"), - |r| r.path.as_ref().map(|p| p.to_str()).flatten(), + |r| r.path.as_ref().and_then(|p| p.to_str()), compare_property_quoted_string ), ( diff --git a/src/compare/sexp.rs b/src/compare/sexp.rs index a6bd5777..85cab3f1 100644 --- a/src/compare/sexp.rs +++ b/src/compare/sexp.rs @@ -113,24 +113,21 @@ fn is_slice_of(parent: &str, child: &str) -> bool { /// Get a slice of the string that was consumed in a parser using the original input to the parser and the remaining input after the parser. fn get_consumed<'s>(input: &'s str, remaining: &'s str) -> &'s str { debug_assert!(is_slice_of(input, remaining)); - let source = { - let offset = remaining.as_ptr() as usize - input.as_ptr() as usize; - &input[..offset] - }; - source.into() + let offset = remaining.as_ptr() as usize - input.as_ptr() as usize; + &input[..offset] } pub(crate) fn unquote(text: &str) -> Result> { let mut out: Vec = Vec::with_capacity(text.len()); - if !text.starts_with(r#"""#) { + if !text.starts_with('"') { return Err("Quoted text does not start with quote.".into()); } - if !text.ends_with(r#"""#) { + if !text.ends_with('"') { return Err("Quoted text does not end with quote.".into()); } let interior_text = &text[1..(text.len() - 1)]; let mut state = ParseState::Normal; - for current_char in interior_text.bytes().into_iter() { + for current_char in interior_text.bytes() { // Check to see if octal finished state = match (state, current_char) { (ParseState::Octal(octal), b'0'..=b'7') if octal.len() < MAX_OCTAL_LENGTH => { @@ -229,11 +226,9 @@ fn atom<'s>(input: &'s str) -> Res<&'s str, Token<'s>> { #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] fn unquoted_atom<'s>(input: &'s str) -> Res<&'s str, Token<'s>> { - let (remaining, body) = take_till1(|c| match c { - ' ' | '\t' | '\r' | '\n' | ')' | ']' => true, - _ => false, - })(input)?; - Ok((remaining, Token::Atom(body.into()))) + let (remaining, body) = + take_till1(|c| matches!(c, ' ' | '\t' | '\r' | '\n' | ')' | ']'))(input)?; + Ok((remaining, Token::Atom(body))) } #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] @@ -264,22 +259,19 @@ fn quoted_atom<'s>(input: &'s str) -> Res<&'s str, Token<'s>> { } let (remaining, _) = tag(r#"""#)(remaining)?; let source = get_consumed(input, remaining); - Ok((remaining, Token::Atom(source.into()))) + Ok((remaining, Token::Atom(source))) } #[cfg_attr(feature = "tracing", tracing::instrument(ret, level = "debug"))] fn hash_notation<'s>(input: &'s str) -> Res<&'s str, Token<'s>> { let (remaining, _) = tag("#<")(input)?; - let (remaining, _body) = take_till1(|c| match c { - '>' => true, - _ => false, - })(remaining)?; + let (remaining, _body) = take_till1(|c| matches!(c, '>'))(remaining)?; let (remaining, _) = tag(">")(remaining)?; let source = get_consumed(input, remaining); - Ok((remaining, Token::Atom(source.into()))) + Ok((remaining, Token::Atom(source))) } -fn text_with_properties<'s>(input: &'s str) -> Res<&'s str, Token<'s>> { +fn text_with_properties(input: &str) -> Res<&str, Token<'_>> { let (remaining, _) = tag("#(")(input)?; let (remaining, (text, props)) = delimited( multispace0, diff --git a/src/compare/util.rs b/src/compare/util.rs index 1cb8a8fd..fd85938c 100644 --- a/src/compare/util.rs +++ b/src/compare/util.rs @@ -54,8 +54,8 @@ pub(crate) fn compare_standard_properties< Ok(()) } -pub(crate) fn assert_name<'b, 's, S: AsRef>( - emacs: &'b Token<'s>, +pub(crate) fn assert_name>( + emacs: &Token<'_>, name: S, ) -> Result<(), Box> { let name = name.as_ref(); @@ -90,9 +90,9 @@ pub(crate) fn assert_bounds<'b, 's, S: StandardProperties<'s> + ?Sized>( standard_properties.end.ok_or("Token should have an end.")?, ); let (rust_begin, rust_end) = get_rust_byte_offsets(original_document, rust); // 0-based - let rust_begin_char_offset = (&original_document[..rust_begin]).chars().count() + 1; // 1-based + let rust_begin_char_offset = original_document[..rust_begin].chars().count() + 1; // 1-based let rust_end_char_offset = - rust_begin_char_offset + (&original_document[rust_begin..rust_end]).chars().count(); // 1-based + rust_begin_char_offset + original_document[rust_begin..rust_end].chars().count(); // 1-based if rust_begin_char_offset != begin || rust_end_char_offset != end { Err(format!("Rust bounds (in chars) ({rust_begin}, {rust_end}) do not match emacs bounds ({emacs_begin}, {emacs_end})", rust_begin = rust_begin_char_offset, rust_end = rust_end_char_offset, emacs_begin=begin, emacs_end=end))?; } @@ -113,8 +113,8 @@ struct EmacsStandardProperties { post_blank: Option, } -fn get_emacs_standard_properties<'b, 's>( - emacs: &'b Token<'s>, +fn get_emacs_standard_properties( + emacs: &Token<'_>, ) -> Result> { let children = emacs.as_list()?; let attributes_child = children.get(1).ok_or("Should have an attributes child.")?; @@ -124,7 +124,7 @@ fn get_emacs_standard_properties<'b, 's>( let mut std_props = standard_properties .expect("if statement proves its Some") .as_vector()? - .into_iter(); + .iter(); let begin = maybe_token_to_usize(std_props.next())?; let post_affiliated = maybe_token_to_usize(std_props.next())?; let contents_begin = maybe_token_to_usize(std_props.next())?; @@ -140,16 +140,13 @@ fn get_emacs_standard_properties<'b, 's>( post_blank, } } else { - let begin = maybe_token_to_usize(attributes_map.get(":begin").map(|token| *token))?; - let end = maybe_token_to_usize(attributes_map.get(":end").map(|token| *token))?; - let contents_begin = - maybe_token_to_usize(attributes_map.get(":contents-begin").map(|token| *token))?; - let contents_end = - maybe_token_to_usize(attributes_map.get(":contents-end").map(|token| *token))?; - let post_blank = - maybe_token_to_usize(attributes_map.get(":post-blank").map(|token| *token))?; + let begin = maybe_token_to_usize(attributes_map.get(":begin").copied())?; + let end = maybe_token_to_usize(attributes_map.get(":end").copied())?; + let contents_begin = maybe_token_to_usize(attributes_map.get(":contents-begin").copied())?; + let contents_end = maybe_token_to_usize(attributes_map.get(":contents-end").copied())?; + let post_blank = maybe_token_to_usize(attributes_map.get(":post-blank").copied())?; let post_affiliated = - maybe_token_to_usize(attributes_map.get(":post-affiliated").map(|token| *token))?; + maybe_token_to_usize(attributes_map.get(":post-affiliated").copied())?; EmacsStandardProperties { begin, post_affiliated, @@ -167,62 +164,57 @@ fn maybe_token_to_usize( Ok(token .map(|token| token.as_atom()) .map_or(Ok(None), |r| r.map(Some))? - .map(|val| { + .and_then(|val| { if val == "nil" { None } else { Some(val.parse::()) } }) - .flatten() // Outer option is whether or not the param exists, inner option is whether or not it is nil .map_or(Ok(None), |r| r.map(Some))?) } /// Get a named property from the emacs token. /// /// Returns Ok(None) if value is nil or absent. -pub(crate) fn get_property<'b, 's, 'x>( +pub(crate) fn get_property<'b, 's>( emacs: &'b Token<'s>, - key: &'x str, + key: &str, ) -> Result>, Box> { let children = emacs.as_list()?; - let attributes_child = children - .iter() - .nth(1) - .ok_or("Should have an attributes child.")?; + let attributes_child = children.get(1).ok_or("Should have an attributes child.")?; let attributes_map = attributes_child.as_map()?; - let prop = attributes_map.get(key).map(|token| *token); - match prop.map(|token| token.as_atom()) { - Some(Ok("nil")) => return Ok(None), - _ => {} - }; + let prop = attributes_map.get(key).copied(); + if let Some(Ok("nil")) = prop.map(Token::as_atom) { + return Ok(None); + } Ok(prop) } /// Get a named property containing an unquoted atom from the emacs token. /// /// Returns None if key is not found. -pub(crate) fn get_property_unquoted_atom<'b, 's, 'x>( - emacs: &'b Token<'s>, - key: &'x str, +pub(crate) fn get_property_unquoted_atom<'s>( + emacs: &Token<'s>, + key: &str, ) -> Result, Box> { - Ok(get_property(emacs, key)? + get_property(emacs, key)? .map(Token::as_atom) - .map_or(Ok(None), |r| r.map(Some))?) + .map_or(Ok(None), |r| r.map(Some)) } /// Get a named property containing an quoted string from the emacs token. /// /// Returns None if key is not found. -pub(crate) fn get_property_quoted_string<'b, 's, 'x>( - emacs: &'b Token<'s>, - key: &'x str, +pub(crate) fn get_property_quoted_string( + emacs: &Token<'_>, + key: &str, ) -> Result, Box> { - Ok(get_property(emacs, key)? + get_property(emacs, key)? .map(Token::as_atom) .map_or(Ok(None), |r| r.map(Some))? .map(unquote) - .map_or(Ok(None), |r| r.map(Some))?) + .map_or(Ok(None), |r| r.map(Some)) } /// Get a named property containing an unquoted numeric value. @@ -299,8 +291,8 @@ where Ok(()) } -pub(crate) fn assert_no_children<'b, 's>( - emacs: &'b Token<'s>, +pub(crate) fn assert_no_children( + emacs: &Token<'_>, this_status: &mut DiffStatus, message: &mut Option, ) -> Result<(), Box> { @@ -329,7 +321,7 @@ where let rust_key = rust_key.as_ref(); let rust_value = rust_value.as_ref(); let emacs_value = get_property_quoted_string(emacs, rust_key)?; - if Some(rust_value) != emacs_value.as_ref().map(String::as_str) { + if Some(rust_value) != emacs_value.as_deref() { let this_status = DiffStatus::Bad; let message = Some(format!( "{} mismatch (emacs != rust) {:?} != {:?}", diff --git a/src/context/context.rs b/src/context/context.rs index 01c3d461..bff8fb3f 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -112,23 +112,18 @@ impl<'g, 'r, 's> Context<'g, 'r, 's> { let mut current_class_filter = ExitClass::Gamma; for current_node in self.iter_context() { let context_element = current_node.get_data(); - match context_element { - ContextElement::ExitMatcherNode(exit_matcher) => { - if exit_matcher.class as u32 <= current_class_filter as u32 { - current_class_filter = exit_matcher.class; - let local_result = (exit_matcher.exit_matcher)(¤t_node, i); - if local_result.is_ok() { - return local_result; - } + if let ContextElement::ExitMatcherNode(exit_matcher) = context_element { + if exit_matcher.class as u32 <= current_class_filter as u32 { + current_class_filter = exit_matcher.class; + let local_result = (exit_matcher.exit_matcher)(¤t_node, i); + if local_result.is_ok() { + return local_result; } } - _ => {} - }; + } } // TODO: Make this a specific error instead of just a generic MyError - return Err(nom::Err::Error(CustomError::MyError(MyError( - "NoExit".into(), - )))); + return Err(nom::Err::Error(CustomError::MyError(MyError("NoExit")))); } /// Indicates if elements should consume the whitespace after them. @@ -140,11 +135,8 @@ impl<'g, 'r, 's> Context<'g, 'r, 's> { fn _should_consume_trailing_whitespace(&self) -> Option { for current_node in self.iter() { - match current_node { - ContextElement::ConsumeTrailingWhitespace(should) => { - return Some(*should); - } - _ => {} + if let ContextElement::ConsumeTrailingWhitespace(should) = current_node { + return Some(*should); } } None diff --git a/src/context/file_access_interface.rs b/src/context/file_access_interface.rs index ebc72696..88ee09b1 100644 --- a/src/context/file_access_interface.rs +++ b/src/context/file_access_interface.rs @@ -23,6 +23,6 @@ impl FileAccessInterface for LocalFileAccessInterface { .as_deref() .map(|pb| pb.join(path)) .unwrap_or_else(|| PathBuf::from(path)); - Ok(std::fs::read_to_string(final_path)?) + std::fs::read_to_string(final_path) } } diff --git a/src/context/list.rs b/src/context/list.rs index f3191b31..46486fba 100644 --- a/src/context/list.rs +++ b/src/context/list.rs @@ -50,7 +50,7 @@ impl<'a, T> Iterator for Iter<'a, T> { fn next(&mut self) -> Option { let ret = self.next.map(|link| link.get_data()); - self.next = self.next.map(|link| link.get_parent()).flatten(); + self.next = self.next.and_then(|link| link.get_parent()); ret } } diff --git a/src/context/mod.rs b/src/context/mod.rs index 321eb37c..a82b5a3a 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -2,6 +2,7 @@ use crate::error::Res; use crate::parser::OrgSource; mod constants; +#[allow(clippy::module_inception)] mod context; mod exiting; mod file_access_interface;