From 10ff304473835fa2b769ec52ef555c196786a83f Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Tue, 22 Apr 2025 21:15:16 +0100 Subject: [PATCH] Deny the clippy::string_slice lint --- src/lib.rs | 2 +- src/metadata/metadata_document.rs | 63 +++++++++++++++++++------------ src/metadata/plugin_metadata.rs | 51 ++++++++++++------------- src/plugin/mod.rs | 13 ++----- 4 files changed, 67 insertions(+), 62 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2e1be808..d86a83c4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,7 +79,7 @@ clippy::rest_pat_in_fully_bound_structs, clippy::str_to_string, clippy::string_lit_chars_any, - // clippy::string_slice, + clippy::string_slice, clippy::string_to_string, clippy::suspicious_xor_used_as_pow, clippy::tests_outside_test_module, diff --git a/src/metadata/metadata_document.rs b/src/metadata/metadata_document.rs index ea39b3d9..88074188 100644 --- a/src/metadata/metadata_document.rs +++ b/src/metadata/metadata_document.rs @@ -336,10 +336,10 @@ impl std::default::Default for MetadataDocument { fn replace_prelude(masterlist: String, prelude: &str) -> String { let line_ending = detect_line_ending(&masterlist); - if let Some((start, end)) = find_prelude_bounds(&masterlist) { + if let Some((start, end)) = split_on_prelude(&masterlist) { let prelude = indent_prelude(prelude, line_ending); - masterlist[..start].to_string() + &prelude + &masterlist[end..] + format!("{start}{prelude}{end}") } else { masterlist } @@ -359,34 +359,49 @@ fn detect_line_ending(masterlist: &str) -> &'static str { } } -fn find_prelude_bounds(masterlist: &str) -> Option<(usize, usize)> { - let prelude_on_first_line = "prelude:"; - let prelude_on_new_line = "\nprelude:"; +fn split_on_prelude(masterlist: &str) -> Option<(&str, &str)> { + let (prefix, remainder) = split_on_prelude_start(masterlist)?; - let start = if masterlist.starts_with(prelude_on_first_line) { - prelude_on_first_line.len() - } else if let Some(pos) = masterlist.find(prelude_on_new_line) { - pos + prelude_on_new_line.len() - } else { - return None; - }; - - let mut pos = start; - while let Some(next_line_break_pos) = masterlist[pos..].find('\n') { - if next_line_break_pos == masterlist.len() - 1 { - break; + let mut iter = remainder.bytes().enumerate().peekable(); + while let Some((index, byte)) = iter.next() { + if byte != b'\n' { + continue; } - pos += next_line_break_pos + 1; - - if let Some(c) = masterlist.as_bytes().get(pos) { - if !matches!(*c, b' ' | b'#' | b'\n' | b'\r') { - return Some((start, pos - 1)); + if let Some((_, next_byte)) = iter.peek() { + if !matches!(next_byte, b' ' | b'#' | b'\n' | b'\r') { + // Slicing at index should never fail, but we can't prove that, + // and we don't want to risk panicking. + if let Some(suffix) = remainder.get(index..) { + return Some((prefix, suffix)); + } } } } - Some((start, masterlist.len())) + Some((prefix, "")) +} + +fn split_on_prelude_start(masterlist: &str) -> Option<(&str, &str)> { + let prelude_on_first_line = "prelude:"; + let prelude_on_new_line = "\nprelude:"; + + if let Some(remainder) = masterlist.strip_prefix(prelude_on_first_line) { + Some((prelude_on_first_line, remainder)) + } else { + if let Some(pos) = masterlist.find(prelude_on_new_line) { + let index = pos + prelude_on_new_line.len(); + // A checked split shouldn't be necessary, but there's no + // split_inclusive_once() method, so we need to find and split in + // two steps and there's always the risk of a bug being introduced + // in the middle. + if let Some((prefix, remainder)) = masterlist.split_at_checked(index) { + return Some((prefix, remainder)) + } + } + None + } + } fn indent_prelude(prelude: &str, line_ending: &str) -> String { @@ -394,7 +409,7 @@ fn indent_prelude(prelude: &str, line_ending: &str) -> String { .replace(&format!(" {line_ending}"), line_ending); if prelude.ends_with("\n ") { - prelude[..prelude.len() - 2].to_string() + prelude.trim_end_matches(' ').to_owned() } else { prelude } diff --git a/src/metadata/plugin_metadata.rs b/src/metadata/plugin_metadata.rs index 9b5f8b3f..71719fa4 100644 --- a/src/metadata/plugin_metadata.rs +++ b/src/metadata/plugin_metadata.rs @@ -318,8 +318,13 @@ impl std::hash::Hash for PluginName { } pub(crate) fn trim_dot_ghost(string: &str) -> &str { - if iends_with_ascii(string, GHOST_FILE_EXTENSION) { - &string[..(string.len() - 6)] + let suffix_start_index = string.len().saturating_sub(GHOST_FILE_EXTENSION.len()); + if let Some((first, last)) = string.split_at_checked(suffix_start_index) { + if last.eq_ignore_ascii_case(GHOST_FILE_EXTENSION) { + first + } else { + string + } } else { string } @@ -352,38 +357,30 @@ fn merge_slices(target: &mut Box<[T]>, source: &[T]) { } fn replace_capturing_groups(regex_string: &str) -> Cow<'_, str> { - let mut output = String::new(); - let mut prefix_length = 0; - let mut remainder = regex_string; - while let Some(pos) = remainder.find('(') { - let Some((before, after)) = remainder.split_at_checked(pos + 1) else { + let mut iter = regex_string.split_inclusive('(').peekable(); + + let mut parts = Vec::new(); + + while let Some(before) = iter.next() { + let Some(after) = iter.peek() else { + parts.push(before); break; }; if after.starts_with('?') || (before.ends_with("\\(") && !before.ends_with("\\\\(")) { - if output.is_empty() { - // No need to copy the string yet. - prefix_length += before.len(); - } else { - output.push_str(before); - } + parts.push(before); } else { - if output.is_empty() { - output.push_str(®ex_string[..prefix_length]); - } - - output.push_str(before); - output.push_str("?:"); + parts.push(before); + parts.push("?:"); } - - remainder = after; } - if output.is_empty() { + let new_length: usize = parts.iter().map(|s| s.len()).sum(); + + if new_length == regex_string.len() { Cow::Borrowed(regex_string) } else { - output.push_str(remainder); - Cow::Owned(output) + Cow::Owned(parts.into_iter().collect()) } } @@ -1237,7 +1234,7 @@ mod tests { match replace_capturing_groups(input) { Cow::Borrowed(output) => assert_eq!(input, output), - Cow::Owned(output) => panic!("Expected borrowed output, got {output}"), + Cow::Owned(output) => panic!("Expected borrowed output, got \"{output}\""), } } @@ -1247,14 +1244,14 @@ mod tests { match replace_capturing_groups(input) { Cow::Borrowed(output) => assert_eq!(input, output), - Cow::Owned(output) => panic!("Expected borrowed output, got {output}"), + Cow::Owned(output) => panic!("Expected borrowed output, got \"{output}\""), } let input = "no paren(?:th(?:e)s)es"; match replace_capturing_groups(input) { Cow::Borrowed(output) => assert_eq!(input, output), - Cow::Owned(output) => panic!("Expected borrowed output, got {output}"), + Cow::Owned(output) => panic!("Expected borrowed output, got \"{output}\""), } } diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index a877647a..d98da511 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -388,16 +388,9 @@ fn calculate_crc(path: &Path) -> std::io::Result { } fn extract_bash_tags(description: &str) -> Vec { - let bash_tags_opener = "{{BASH:"; - - if let Some(mut start_pos) = description.find(bash_tags_opener) { - start_pos += bash_tags_opener.len(); - - if let Some(end_pos) = description[start_pos..].find("}}") { - return description[start_pos..start_pos + end_pos] - .split(',') - .map(|s| s.trim().to_owned()) - .collect(); + if let Some((_, bash_tags)) = description.split_once("{{BASH:") { + if let Some((bash_tags, _)) = bash_tags.split_once("}}") { + return bash_tags.split(',').map(|s| s.trim().to_owned()).collect(); } } Vec::new()