diff options
| -rw-r--r-- | clippy_lints/src/enum_variants.rs | 124 | ||||
| -rw-r--r-- | clippy_utils/src/str_utils.rs | 91 | ||||
| -rw-r--r-- | tests/ui/enum_variants.rs | 6 | ||||
| -rw-r--r-- | tests/ui/enum_variants.stderr | 38 |
4 files changed, 187 insertions, 72 deletions
diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index fc3a35efaf8..6e1fa836b8f 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -2,8 +2,8 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; use clippy_utils::source::is_present_in_source; -use clippy_utils::str_utils::{self, count_match_end, count_match_start}; -use rustc_hir::{EnumDef, Item, ItemKind}; +use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start}; +use rustc_hir::{EnumDef, Item, ItemKind, Variant}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; @@ -18,6 +18,12 @@ declare_clippy_lint! { /// Enumeration variant names should specify their variant, /// not repeat the enumeration name. /// + /// ### Limitations + /// Characters with no casing will be considered when comparing prefixes/suffixes + /// This applies to numbers and non-ascii characters without casing + /// e.g. `Foo1` and `Foo2` is considered to have different prefixes + /// (the prefixes are `Foo1` and `Foo2` respectively), as also `Bar螃`, `Bar蟹` + /// /// ### Example /// ```rust /// enum Cake { @@ -120,72 +126,73 @@ impl_lint_pass!(EnumVariantNames => [ MODULE_INCEPTION ]); -fn check_variant( - cx: &LateContext<'_>, - threshold: u64, - def: &EnumDef<'_>, - item_name: &str, - item_name_chars: usize, - span: Span, -) { +fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) { + let name = variant.ident.name.as_str(); + let item_name_chars = item_name.chars().count(); + + if count_match_start(item_name, &name).char_count == item_name_chars + && name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase()) + && name.chars().nth(item_name_chars + 1).map_or(false, |c| !c.is_numeric()) + { + span_lint( + cx, + ENUM_VARIANT_NAMES, + variant.span, + "variant name starts with the enum's name", + ); + } +} + +fn check_enum_end(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) { + let name = variant.ident.name.as_str(); + let item_name_chars = item_name.chars().count(); + + if count_match_end(item_name, &name).char_count == item_name_chars { + span_lint( + cx, + ENUM_VARIANT_NAMES, + variant.span, + "variant name ends with the enum's name", + ); + } +} + +fn check_variant(cx: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_name: &str, span: Span) { if (def.variants.len() as u64) < threshold { return; } - for var in def.variants { - let name = var.ident.name.as_str(); - if count_match_start(item_name, &name).char_count == item_name_chars - && name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase()) - && name.chars().nth(item_name_chars + 1).map_or(false, |c| !c.is_numeric()) - { - span_lint( - cx, - ENUM_VARIANT_NAMES, - var.span, - "variant name starts with the enum's name", - ); - } - if count_match_end(item_name, &name).char_count == item_name_chars { - span_lint( - cx, - ENUM_VARIANT_NAMES, - var.span, - "variant name ends with the enum's name", - ); - } - } + let first = &def.variants[0].ident.name.as_str(); - let mut pre = &first[..str_utils::camel_case_until(&*first).byte_index]; - let mut post = &first[str_utils::camel_case_start(&*first).byte_index..]; + let mut pre = camel_case_split(first); + let mut post = pre.clone(); + post.reverse(); for var in def.variants { + check_enum_start(cx, item_name, var); + check_enum_end(cx, item_name, var); let name = var.ident.name.as_str(); - let pre_match = count_match_start(pre, &name).byte_count; - pre = &pre[..pre_match]; - let pre_camel = str_utils::camel_case_until(pre).byte_index; - pre = &pre[..pre_camel]; - while let Some((next, last)) = name[pre.len()..].chars().zip(pre.chars().rev()).next() { - if next.is_numeric() { - return; - } - if next.is_lowercase() { - let last = pre.len() - last.len_utf8(); - let last_camel = str_utils::camel_case_until(&pre[..last]); - pre = &pre[..last_camel.byte_index]; - } else { - break; - } - } + let variant_split = camel_case_split(&name); - let post_match = count_match_end(post, &name); - let post_end = post.len() - post_match.byte_count; - post = &post[post_end..]; - let post_camel = str_utils::camel_case_start(post); - post = &post[post_camel.byte_index..]; + pre = pre + .iter() + .zip(variant_split.iter()) + .take_while(|(a, b)| a == b) + .map(|e| *e.0) + .collect(); + post = post + .iter() + .zip(variant_split.iter().rev()) + .take_while(|(a, b)| a == b) + .map(|e| *e.0) + .collect(); } let (what, value) = match (pre.is_empty(), post.is_empty()) { (true, true) => return, - (false, _) => ("pre", pre), - (true, false) => ("post", post), + (false, _) => ("pre", pre.join("")), + (true, false) => { + post.reverse(); + ("post", post.join("")) + }, }; span_lint_and_help( cx, @@ -233,7 +240,6 @@ impl LateLintPass<'_> for EnumVariantNames { #[allow(clippy::similar_names)] fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { let item_name = item.ident.name.as_str(); - let item_name_chars = item_name.chars().count(); let item_camel = to_camel_case(&item_name); if !item.span.from_expansion() && is_present_in_source(cx, item.span) { if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() { @@ -283,7 +289,7 @@ impl LateLintPass<'_> for EnumVariantNames { } if let ItemKind::Enum(ref def, _) = item.kind { if !(self.avoid_breaking_exported_api && cx.access_levels.is_exported(item.def_id)) { - check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span); + check_variant(cx, self.threshold, def, &item_name, item.span); } } self.modules.push((item.ident.name, item_camel)); diff --git a/clippy_utils/src/str_utils.rs b/clippy_utils/src/str_utils.rs index 6dc52bf2325..03a9d3c25fd 100644 --- a/clippy_utils/src/str_utils.rs +++ b/clippy_utils/src/str_utils.rs @@ -68,6 +68,20 @@ pub fn camel_case_until(s: &str) -> StrIndex { /// ``` #[must_use] pub fn camel_case_start(s: &str) -> StrIndex { + camel_case_start_from_idx(s, 0) +} + +/// Returns `StrIndex` of the last camel-case component of `s[idx..]`. +/// +/// ``` +/// # use clippy_utils::str_utils::{camel_case_start_from_idx, StrIndex}; +/// assert_eq!(camel_case_start_from_idx("AbcDef", 0), StrIndex::new(0, 0)); +/// assert_eq!(camel_case_start_from_idx("AbcDef", 1), StrIndex::new(3, 3)); +/// assert_eq!(camel_case_start_from_idx("AbcDefGhi", 0), StrIndex::new(0, 0)); +/// assert_eq!(camel_case_start_from_idx("AbcDefGhi", 1), StrIndex::new(3, 3)); +/// assert_eq!(camel_case_start_from_idx("Abcdefg", 1), StrIndex::new(7, 7)); +/// ``` +pub fn camel_case_start_from_idx(s: &str, start_idx: usize) -> StrIndex { let char_count = s.chars().count(); let range = 0..char_count; let mut iter = range.rev().zip(s.char_indices().rev()); @@ -78,9 +92,13 @@ pub fn camel_case_start(s: &str) -> StrIndex { } else { return StrIndex::new(char_count, s.len()); } + let mut down = true; let mut last_index = StrIndex::new(char_count, s.len()); for (char_index, (byte_index, c)) in iter { + if byte_index < start_idx { + break; + } if down { if c.is_uppercase() { down = false; @@ -98,9 +116,55 @@ pub fn camel_case_start(s: &str) -> StrIndex { return last_index; } } + last_index } +/// Get the indexes of camel case components of a string `s` +/// +/// ``` +/// # use clippy_utils::str_utils::{camel_case_indices, StrIndex}; +/// assert_eq!( +/// camel_case_indices("AbcDef"), +/// vec![StrIndex::new(0, 0), StrIndex::new(3, 3), StrIndex::new(6, 6)] +/// ); +/// assert_eq!( +/// camel_case_indices("abcDef"), +/// vec![StrIndex::new(3, 3), StrIndex::new(6, 6)] +/// ); +/// ``` +pub fn camel_case_indices(s: &str) -> Vec<StrIndex> { + let mut result = Vec::new(); + let mut str_idx = camel_case_start(s); + + while str_idx.byte_index < s.len() { + let next_idx = str_idx.byte_index + 1; + result.push(str_idx); + str_idx = camel_case_start_from_idx(s, next_idx); + } + result.push(str_idx); + + result +} + +/// Split camel case string into a vector of its components +/// +/// ``` +/// # use clippy_utils::str_utils::{camel_case_split, StrIndex}; +/// assert_eq!(camel_case_split("AbcDef"), vec!["Abc", "Def"]); +/// ``` +pub fn camel_case_split(s: &str) -> Vec<&str> { + let mut offsets = camel_case_indices(s) + .iter() + .map(|e| e.byte_index) + .collect::<Vec<usize>>(); + if offsets[0] != 0 { + offsets.insert(0, 0); + } + + offsets.windows(2).map(|w| &s[w[0]..w[1]]).collect() +} + /// Dealing with sting comparison can be complicated, this struct ensures that both the /// character and byte count are provided for correct indexing. #[derive(Debug, Default, PartialEq, Eq)] @@ -231,4 +295,31 @@ mod test { fn until_caps() { assert_eq!(camel_case_until("ABCD"), StrIndex::new(0, 0)); } + + #[test] + fn camel_case_start_from_idx_full() { + assert_eq!(camel_case_start_from_idx("AbcDef", 0), StrIndex::new(0, 0)); + assert_eq!(camel_case_start_from_idx("AbcDef", 1), StrIndex::new(3, 3)); + assert_eq!(camel_case_start_from_idx("AbcDef", 4), StrIndex::new(6, 6)); + assert_eq!(camel_case_start_from_idx("AbcDefGhi", 0), StrIndex::new(0, 0)); + assert_eq!(camel_case_start_from_idx("AbcDefGhi", 1), StrIndex::new(3, 3)); + assert_eq!(camel_case_start_from_idx("Abcdefg", 1), StrIndex::new(7, 7)); + } + + #[test] + fn camel_case_indices_full() { + assert_eq!(camel_case_indices("Abc\u{f6}\u{f6}DD"), vec![StrIndex::new(7, 9)]); + } + + #[test] + fn camel_case_split_full() { + assert_eq!(camel_case_split("A"), vec!["A"]); + assert_eq!(camel_case_split("AbcDef"), vec!["Abc", "Def"]); + assert_eq!(camel_case_split("Abc"), vec!["Abc"]); + assert_eq!(camel_case_split("abcDef"), vec!["abc", "Def"]); + assert_eq!( + camel_case_split("\u{f6}\u{f6}AabABcd"), + vec!["\u{f6}\u{f6}", "Aab", "A", "Bcd"] + ); + } } diff --git a/tests/ui/enum_variants.rs b/tests/ui/enum_variants.rs index 083f5143e6e..d3662a0a213 100644 --- a/tests/ui/enum_variants.rs +++ b/tests/ui/enum_variants.rs @@ -145,4 +145,10 @@ enum HIDataRequest { DeleteUnpubHIData(String), } +enum North { + Normal, + NoLeft, + NoRight, +} + fn main() {} diff --git a/tests/ui/enum_variants.stderr b/tests/ui/enum_variants.stderr index add8a91e26b..8a3265086e8 100644 --- a/tests/ui/enum_variants.stderr +++ b/tests/ui/enum_variants.stderr @@ -6,6 +6,18 @@ LL | cFoo, | = note: `-D clippy::enum-variant-names` implied by `-D warnings` +error: all variants have the same prefix: `c` + --> $DIR/enum_variants.rs:14:1 + | +LL | / enum Foo { +LL | | cFoo, +LL | | cBar, +LL | | cBaz, +LL | | } + | |_^ + | + = help: remove the prefixes and use full paths to the variants instead of glob imports + error: variant name starts with the enum's name --> $DIR/enum_variants.rs:26:5 | @@ -60,25 +72,25 @@ LL | | } | = help: remove the prefixes and use full paths to the variants instead of glob imports -error: all variants have the same prefix: `WithOut` - --> $DIR/enum_variants.rs:81:1 +error: all variants have the same prefix: `C` + --> $DIR/enum_variants.rs:59:1 | -LL | / enum Seallll { -LL | | WithOutCake, -LL | | WithOutTea, -LL | | WithOut, +LL | / enum Something { +LL | | CCall, +LL | | CCreate, +LL | | CCryogenize, LL | | } | |_^ | = help: remove the prefixes and use full paths to the variants instead of glob imports -error: all variants have the same prefix: `Prefix` - --> $DIR/enum_variants.rs:87:1 +error: all variants have the same prefix: `WithOut` + --> $DIR/enum_variants.rs:81:1 | -LL | / enum NonCaps { -LL | | Prefix的, -LL | | PrefixTea, -LL | | PrefixCake, +LL | / enum Seallll { +LL | | WithOutCake, +LL | | WithOutTea, +LL | | WithOut, LL | | } | |_^ | @@ -108,5 +120,5 @@ LL | | } | = help: remove the postfixes and use full paths to the variants instead of glob imports -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors |